Page MenuHome

Fix viewlayer hide/exclude settings getting lost for linked collections.
ClosedPublic

Authored by Brecht Van Lommel (brecht) on Feb 26 2019, 8:41 PM.

Details

Summary

The problem was that the object and collection pointers in Base and
LayerCollection would get lost of file read. Normally such ID pointers would
be resolved by pointing to an ID_ID placeholder which has the datablock name,
and then replacing it will the real datablock. However ID_ID is only written
for directly linked datablocks.

This adds the concept of an indirectly linked datablock with a weak reference
to it. For this we write an ID_ID_WEAK_REF code, which is a reference that
will only be resolved if the datablock was read for another reason.

I'm not really happy with this, but I couldn't find a better solution. For
the studio this is an important feature, as it allows them to link in
collections to shots and then change their view layer visibility as if it
was local data. Except that without this fix it would not be saved.

Depends on D4415.

Diff Detail

Repository
rB Blender

Event Timeline

Harbormaster completed remote builds in B3012: Diff 13879.

Hmmpff… Took me some time to wrap my head around both the issue and the solution here… :/

Am not really happy with this either (it's adding even more complexity to something that is already rather confusing), but cannot think of any better solution. Aside from making all those linked objects and collections directly linked, which is obviously not acceptable.

So unless other devs have a brilliant idea, this is OK for me.

Did you check that this is also working fine in recursive linking (i.e. linking a scene which is linking some collections, are the view settings of linked scene's viewlayer preserved)?

source/blender/blenkernel/intern/idcode.c
94 ↗(On Diff #13879)

… and ID_WEAK_LINK_PLACEHOLDER!

This revision is now accepted and ready to land.Mar 6 2019, 9:05 AM

Rebase to latest master.

Fix crash loading .blend files in older Blender versions.

Don't use a separate ID code anymore, since this causes loading in older versions
to try to load it as a regular ID which crashes.

Bastien Montagne (mont29) requested changes to this revision.Sep 17 2019, 5:18 PM

Generally fine, approach remain globally the same as in previous version, and not having to define a new block type in .blend file is probably a good thing.

The only thing that bothers me here is the usage of three new bitflags… Especially since, afaict, LIB_TAG_INDIRECT_WEAK_LINK in tags and LIB_WEAK_LINK_PLACEHOLDER in flags mean exactly the same thing? Also, this kind of twists the clear separation between tag and flag (the former being runtime only, the later being saved in .blend files). And to me it seems the tag one is only here to help with status checking (i.e. not having to checks flags in two different places in read/write code)? I’d rather see that info stay in flag all the time…

This revision now requires changes to proceed.Sep 17 2019, 5:18 PM

Remove LIB_TAG_INDIRECT_WEAK_LINK. Is that what you had in mind?

Bastien Montagne (mont29) requested changes to this revision.Sep 17 2019, 10:12 PM

Remove LIB_TAG_INDIRECT_WEAK_LINK. Is that what you had in mind?

Well, yes… But the one you removed here is LIB_TAG_ID_WEAK_LINK_PLACEHOLDER (temp one used during reading of .blend file)! Nice if we can also get rid of that one btw.

My point being, in current patch, LIB_WEAK_LINK_PLACEHOLDER flag and LIB_TAG_INDIRECT_WEAK_LINK tag have exactly the same meaning, unless I am missing something. So I do not see the point of this switching between tag during most of runtime, and flag for saving into .blend file. We should also probably rename LIB_WEAK_LINK_PLACEHOLDER flag to LIB_INDIRECT_WEAK_LINK, in fact.

This revision now requires changes to proceed.Sep 17 2019, 10:12 PM

Remove LIB_TAG_INDIRECT_WEAK_LINK.

Am still a bit puzzled by the two blocks clearing the flag in readfile.c… Feels odd to clear that flag, and then set it again, but I guess those are needed. And maybe it's better that way, to ensure we only mark as week link the IDs that need it (since a linked collection's content can change e.g.).

/* Convert any previously read weak link to regular link
 * to signal that we want to read this datablock. */
id->flag &= ~LIB_INDIRECT_WEAK_LINK;

Anyway, patch LGTM now.

This revision is now accepted and ready to land.Sep 18 2019, 8:22 PM