Paths

Table of Contentst

Differential D10533

Fix T85981: Undo on linked rig with overrides loses custom shapes
AbandonedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Feb 25 2021, 4:14 PM.

Details

Reviewers

Bastien Montagne (mont29)

Maniphest Tasks

T85981: Undo on linked rig with overrides loses custom shapes

Summary

Caused by rB4c3d51326efa: LibOverride: Refactor 'make override' 3DView operator.

Custom bone shapes are actually getting an override now [which is
unwanted].

For custom bone shapes, the tag for making an Override is removed in
lib_override_linked_group_tag after
lib_override_linked_group_tag_recursive.
But in above commit, an additional
make_override_hierarchy_recursive_tag (now
lib_override_hierarchy_dependencies_recursive_tag) was introduced that
could tag the custom bone shapes again afterwards.

Not 100% sure where the best place for tag removal now is, but propose
to move it to lib_override_library_create_do (after
lib_override_linked_group_tag /
lib_override_hierarchy_dependencies_recursive_tag have done their
thing).

Diff Detail

Repository

rB Blender

Branch

T85981 (branched from master)

Build Status

Buildable 13115
Build 13115: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) requested review of this revision.Feb 25 2021, 4:14 PM

Philipp Oeser (lichtwerk) created this revision.

Harbormaster completed remote builds in B13115: Diff 34420.Feb 25 2021, 4:15 PM

Philipp Oeser (lichtwerk) added a project: Core.Feb 25 2021, 4:15 PM

Thanks for the patch, but this cannot be a proper solution, if the bone shape object is added back in the override set by lib_override_hierarchy_dependencies_recursive_tag, it's that it actually needs to be overridden to satisfy other relationships.

What would be interesting is to know what relation causes that re-tagging ?

But in nay case, it does not explain why this custom shape is lost on undo.

This revision now requires changes to proceed.Feb 26 2021, 9:31 AM

What would be interesting is to know what relation causes that re-tagging ?

will check

But in nay case, it does not explain why this custom shape is lost on undo.

will check

The relation is to the parent (in the example file, the custom bone shape Widget is parented to Wgt_grp -- and the bug only occurs if Widget is actually parented, without parenting, this is fine)
So an MainIDRelationsEntry is created for that, with its i`d_pointer.to` set to Wgt_grp.
Happens from BKE_main_relations_create / object_foreach_id / BKE_LIB_FOREACHID_PROCESS(data, object->parent and there seems nothing wrong with that.

Then we clear the custom bone shape object from the tagged things (but its parent still hangs around)
Once the parent is still tagged, the custom bone shape itself will return to be tagged as well through lib_override_hierarchy_dependencies_recursive_tag.

So not sure, we could also untag the parent right away (but that might have its own justification to be tagged?)

P1996 T85981_untag_parent_as_well

1
2
3	diff --git a/source/blender/blenkernel/intern/lib_override.c b/source/blender/blenkernel/intern/lib_override.c
4	index 602c560cedd..79824ec6c8e 100644
5	--- a/source/blender/blenkernel/intern/lib_override.c
6	+++ b/source/blender/blenkernel/intern/lib_override.c
7	@@ -509,6 +509,9 @@ static void lib_override_linked_group_tag(LibOverrideGroupTagData *data)
8	for (bPoseChannel *pchan = ob->pose->chanbase.first; pchan != NULL; pchan = pchan->next) {
9	if (pchan->custom != NULL) {
10	pchan->custom->id.tag &= ~(data->tag \| data->missing_tag);
11	+ /* Also untag the parent? (but that might have its own justification to be tagged). */
12	+ Object ob_custom = (Object )&pchan->custom->id;
13	+ ob_custom->parent->id.tag &= ~(data->tag \| data->missing_tag);
14	}
15	}
16	}

And (in theroy), shouldnt all things that object_foreach_id does for the custom boneshape be "reverted" (not just the parent, there might be other stuff tagged that shouldnt?)

Will check what goes wrong with UNDO next, but @Bastien Montagne (mont29), feel free to interfere and take over if you have a vision where this should be going right away...

Not sure why custom shape object needs to be parented to anything? That does not look like the right thing to do to me in a decent rig, would expect those bone shape objects to remain isolated in a hidden sub-collection or something like that...

But once again, undo bug is the main issue here for me.

If people create dependencies on their bone shape objects, then those should be overridden, there is nothing wrong here.

If people create dependencies on their bone shape objects, then those should be overridden, there is nothing wrong here.

If this includes the bone shape objects themselves, then this contradicts the explicit removal of tagging, right? Also: "custom_shape" should not be PROPOVERRIDE_NO_COMPARISON then, right?

Regarding Undo, I have looked a bit and I would love to spend some more time in memfile undo code, but I assume this is not the most effective way to get rid of this issue since you are much more into this than I am @Bastien Montagne (mont29).
Sure, those investgations are never lost or wasted, but the tracker is too loaded atm. to justify looking much longer.
Mind taking over?

OK will do

Philipp Oeser (lichtwerk) abandoned this revision.Mar 1 2021, 7:15 PM

Revision Contents
Changeset List

Path

Size

source/

blender/

blenkernel/

intern/

lib_override.c

25 lines

Diff 34420

View Options

source/blender/blenkernel/intern/lib_override.c

Fix T85981: Undo on linked rig with overrides loses custom shapesAbandonedPublicActions