Outliner, Library Overrides: List child objects under parents
ClosedPublic
Actions

Authored by Julian Eisel (Severin) on Jul 1 2022, 1:54 PM.

Details

Reviewers

Bastien Montagne (mont29)

Commits

rBf9a805164a94: Outliner, Library Overrides: List child objects under parents

Summary

Because children point to, or "use" their parent, the Library Overrides Hierarchies mode in the Outliner would show parents contained in children, not children contained in a parent. Or explained in pictures:

Old	New

(Not quite sure why the parent is listed twice in the old one. The new one should be more correct anyway.)

In production files this would make the rig listed under all its children, so it would appear many times in the tree. Now it appears once and the children are collected under it.

Old	New

Bastien and I decided to refactor the way we build the tree for this. Instead of using BKE_library_foreach_ID_link(), it now uses the ID relations mapping in MainIDRelations.

Diff Detail

Repository: rB Blender

Event Timeline

Julian Eisel (Severin) requested review of this revision.Jul 1 2022, 1:54 PM

Julian Eisel (Severin) created this revision.

Harbormaster completed remote builds in B22821: Diff 53161.Jul 1 2022, 1:54 PM

Julian Eisel (Severin) added projects: Pipeline, Assets & I/O, Overrides.Jul 1 2022, 1:56 PM

Julian Eisel (Severin) edited the summary of this revision. (Show Details)

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Jul 1 2022, 1:58 PM

Julian Eisel (Severin) retitled this revision from Outliner, Library Overrides: List parents objects above children to Outliner, Library Overrides: List child objects under parents.Jul 1 2022, 2:00 PM

Generally looks fine to me.

source/blender/editors/space_outliner/tree/tree_display_override_library_hierarchies.cc
288–303	I would rather systematically assume that `real_override_id = &relationship_parent_id;` is valid (it should be!), and ideally add an assert like that: const IDTypeInfo id_type = BKE_idtype_get_info_from_id(real_override_id); BLI_assert( (id_type->owner_get != nullptr); / The #IDTypeInfo::owner_get callback should not modify the arguments, so casting away const * is okay. / BLI_assert(real_override_id == id_type->owner_get(const_cast<Main >(bmain), const_cast<ID >(id))); But since you do not have a `bmain` here, and call to `owner_get` is expensive currently in some cases, you could just do something like that for the time being: #ifndef NDEBUG if (GS(id.name) == ID_KE) { const Key key = (Key *)&id; BLI_assert(real_override_id == key->from); } else { BLI_assert((id.flag & LIB_EMBEDDED_DATA) != 0); } #endif NOTE: Ideally one would make `lib_override_get` public and use it here, but again, usage of `owner_get` would be way too expensive here currently. :(

This revision now requires changes to proceed.Jul 5 2022, 6:21 PM

Changes requested in review

Harbormaster completed remote builds in B22882: Diff 53358.Jul 7 2022, 4:18 PM

LGTM now.

source/blender/editors/space_outliner/tree/tree_display_override_library_hierarchies.cc

289

Would add a comment here explaining a bit, something like:

/* This assumes that the parent ID is always the owner of the 'embedded' one, I.e. that no other ID directly uses the embedded one. Should be true, but the debug code adds some checks to validate this assumption. */

This revision is now accepted and ready to land.Jul 7 2022, 4:41 PM

Closed by commit rBf9a805164a94: Outliner, Library Overrides: List child objects under parents (authored by Julian Eisel (Severin)). · Explain WhyJul 7 2022, 5:20 PM

This revision was automatically updated to reflect the committed changes.

Julian Eisel (Severin) added a commit: rBf9a805164a94: Outliner, Library Overrides: List child objects under parents.