Details

Reviewers

Bastien Montagne (mont29)
Dalai Felinto (dfelinto)

Commits

rB659ccd9cd2ee: Fix T66154: viewlayer hide/exclude settings getting lost for linked collections

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

Branch

arcpatch-D4416 (branched from master)

Build Status

Buildable 4998
Build 4998: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) created this revision.Feb 26 2019, 8:41 PM

Harbormaster completed remote builds in B3012: Diff 13879.Feb 26 2019, 8:41 PM

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

Brecht Van Lommel (brecht) mentioned this in T66154: Flags of linked-in nested collections are determined by the top-level collection.Jun 26 2019, 5:20 PM

Dalai Felinto (dfelinto) added a project: BF Blender: After Release.Jun 26 2019, 6:07 PM

Rebase to latest master.

Harbormaster completed remote builds in B4974: Diff 18264.Sep 16 2019, 3:04 PM

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.

Harbormaster completed remote builds in B4979: Diff 18270.Sep 16 2019, 5:24 PM

Brecht Van Lommel (brecht) removed reviewers: Sergey Sharybin (sergey), Campbell Barton (campbellbarton).Sep 16 2019, 5:42 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?

Harbormaster completed remote builds in B4998: Diff 18302.Sep 17 2019, 6:36 PM

In D4416#133881, @Brecht Van Lommel (brecht) wrote:

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.

Harbormaster completed remote builds in B5004: Diff 18314.Sep 18 2019, 11:30 AM

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

Dalai Felinto (dfelinto) mentioned this in T70046: Linked Instanced collections are not being displayed in the viewport with Local collections enabled.Sep 19 2019, 2:09 AM

Satish Goda (satishgoda) added a subscriber: Satish Goda (satishgoda).Sep 19 2019, 2:39 AM

Closed by commit rB659ccd9cd2ee: Fix T66154: viewlayer hide/exclude settings getting lost for linked collections (authored by Brecht Van Lommel (brecht)). · Explain WhySep 19 2019, 11:38 AM

This revision was automatically updated to reflect the committed changes.

Brecht Van Lommel (brecht) added a commit: rB659ccd9cd2ee: Fix T66154: viewlayer hide/exclude settings getting lost for linked collections.

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

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 18302

source/blender/blenkernel/BKE_library.h

source/blender/blenkernel/intern/layer.c

source/blender/blenkernel/intern/library.c

source/blender/blenkernel/intern/library_remap.c

source/blender/blenloader/intern/readfile.c

source/blender/blenloader/intern/writefile.c

source/blender/makesdna/DNA_ID.h

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

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 18302

source/blender/blenkernel/BKE_library.h

source/blender/blenkernel/intern/layer.c

source/blender/blenkernel/intern/library.c

source/blender/blenkernel/intern/library_remap.c

source/blender/blenloader/intern/readfile.c

source/blender/blenloader/intern/writefile.c

source/blender/makesdna/DNA_ID.h

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

Revision Contents
Changeset List