Fix T83050: Crash dragging collection in two scenes to scene collection
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Dec 8 2020, 3:58 PM.

Details

Reviewers

Bastien Montagne (mont29)

Maniphest Tasks

T83050: Dragging a sub-collection out of a scene-linked collection into the "Scene Collection" crashes reliably

Commits

rB47159f1609c2: Fix T83050: Crash dragging shared collection to master collection
rB97651f428b6a: Fix T83050: Crash dragging shared collection to master collection

Summary

The flag syncing code expects to find collection flags in same view layer
before and after the move, it even has an assert for it. However, there is
one case where this doesn't happen, when dragging a collection that exists
in two scenes to the scene collection. Because the scene collection is
always local to each scene, its sub-heirarchy doesn't copy between scenes
created with "Linked Copy", so the dragged collection doesn't exist in the
second scene, and the corresponding CollectionFlag isn't found.

This patch removes this assert, frees the temporary flag structs
separately, and updates some comments with this information.

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Dec 8 2020, 3:58 PM

Hans Goudey (HooglyBoogly) created this revision.

Harbormaster completed remote builds in B11618: Diff 31736.Dec 8 2020, 3:58 PM

Hans Goudey (HooglyBoogly) added a task: T83050: Dragging a sub-collection out of a scene-linked collection into the "Scene Collection" crashes reliably.Dec 8 2020, 3:59 PM

Hans Goudey (HooglyBoogly) added projects: Pipeline, Assets & I/O, BF Blender (2.91).

Besides the notes on the comment, LGTM.

source/blender/blenkernel/intern/collection.c
1825–1833	`is in the` I guess?
1825–1833	restored
1825–1833	`is still in the scene,`
1826	-> `master`
1826	its
1826–1828	Do not focus on `linked copy` here, there are many other ways to end up sharing a same collection in several scenes, and all those would trigger this special case when moved to the master one. In fact, it can even happen when a sub-collection is moved to anther regular collection in one scene, that does not exist in the other scene that shared it previously. `However, if we move a sub-collection shared by several scenes, it may effectively remove it from some non-active scenes hierarchy (e.g. if moved into current scene's master collection). Those non-active scene view layers won't contain a matching layer collection anymore then.` ?
1828	-> `master`
1893	`Restore the original layer collection flags and free their temp storage.`

This revision now requires changes to proceed.Dec 9 2020, 10:23 AM

Improved the wording and correctness of comments

Thanks for the comments on the wording, it's definitely an improvement,
but the structure of it (which was meant to be "normally it works like this, however
sometimes it doesn't" didn't really make sense after your changes, so I changed it a bit
more. Anyway, it's just a comment and I think it makes sense now and contains the
important information.

Harbormaster completed remote builds in B11685: Diff 31848.Dec 11 2020, 12:06 AM

Comment looks perfectly clear to me now indeed

This revision is now accepted and ready to land.Dec 11 2020, 7:26 AM

Closed by commit rB97651f428b6a: Fix T83050: Crash dragging shared collection to master collection (authored by Hans Goudey (HooglyBoogly)). · Explain WhyDec 11 2020, 7:16 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rB97651f428b6a: Fix T83050: Crash dragging shared collection to master collection.

Jeroen Bakker (jbakker) added a commit: rB47159f1609c2: Fix T83050: Crash dragging shared collection to master collection.Jan 13 2021, 3:10 PM