Fix T67232: Multiples targetless IKs in a chain gives weird behaviour (known as FakeIK for FK posing)
ClosedPublic
Actions

Authored by Sebastian Parborg (zeddb) on Apr 8 2020, 3:40 PM.

Details

Reviewers

Sergey Sharybin (sergey)
Sybren A. Stüvel (sybren)
Brecht Van Lommel (brecht)

Commits

rB9a7f5f1bb422: Fix T67232: Multiples targetless IKs in a chain gives weird behaviour (known as…

Summary

The issue was that the deps graph relation builder assumed that all bones that had a IK constraint on them would be evaluated.
However for targetless IK bones, only the active bone would receive updates and the others would be skipped (as those would be treated as if the IK constraint was disabled).

I didn't see an easy way to solve this from the depsgraph side of things.

Instead I came up with a solution that I feel is quite strait forward and reflects what is actually supposed to happen under the hood.

Now all targetless IK constraints are treated as disabled and will not be added to any relations in the depsgraph.

Instead, a temporary IK constraint will be created when the bone in question is transformed.
This is basically activating "Auto IK" for the bone while transforming.

Diff Detail

Repository: rB Blender

Event Timeline

Sebastian Parborg (zeddb) created this revision.Apr 8 2020, 3:40 PM

Sebastian Parborg (zeddb) edited the summary of this revision. (Show Details)

You can't ignore disabled constraints during dependency graph build: similarly to modifiers enabled state, constraint enabled flag does not trigger relations update (due to performance issues mainly i would imagine).

Is it possible to construct temporary object/depsgraph for the auto-ik?

This revision now requires changes to proceed.Apr 8 2020, 3:46 PM

Hold on.
I got confused between CONSTRAINT_DISABLE and CONSTRAINT_OFF.

The previous comment applies to CONSTRAINT_OFF.

The CONSTRAINT_DISABLE I am not sure where it is set in the evaluation flow. You wouldn't be able to use this flag in builder if evaluation changes this flag. If this flag is something intrinsic and is maintained by operators then you should be able to use it.

It is set in: blender/editors/object/object_constraint.c line 304

The constraint is set as disabled as long as it is targetless (the target has to be valid of course).
So I think that this would trigger a rebuild because the only way to have it not disabled anymore, is to change the target settings.
If the target settings change, then the depsgraph must be rebuilt to get the correct relations.

It is set in: blender/editors/object/object_constraint.c line 304

Right, but is there any code path which goes there from depsgraph evaluation?

Doesn't seem like it.

All places where this is called from tags depsgraph for update afterwards.
And they are when adding, modifying or removing constraints it seems.

I think it's fine.

@Brecht Van Lommel (brecht), do you have time/energy to have second pair of eyes?

This revision is now accepted and ready to land.Apr 8 2020, 4:19 PM

Brecht Van Lommel (brecht) requested changes to this revision.Apr 9 2020, 3:34 PM

Brecht Van Lommel (brecht) added inline comments.

source/blender/editors/transform/transform_convert_armature.c
234–250	Is this code also needed in `pose_transform_mirror_update` which also sets `CONSTRAINT_IK_AUTO`? Can this code be deduplicated with similar code in `pose_grab_with_ik_add`? Can we call `BIK_clear_data` and `DEG_relations_tag_update` only once, after the constraints have been added for all bones?

This revision now requires changes to proceed.Apr 9 2020, 3:34 PM

Updated with the latest feedback

Brecht Van Lommel (brecht) accepted this revision.Apr 10 2020, 1:47 PM

This revision is now accepted and ready to land.Apr 10 2020, 1:47 PM

Sebastian Parborg (zeddb) marked an inline comment as done.Apr 10 2020, 1:48 PM

Sebastian Parborg (zeddb) added inline comments.

source/blender/editors/transform/transform_convert_armature.c
234–250	For the deg tagging, I guess that I could modify the functions to keep track if they added temporary ik constraints. I don't know how much this will help in practice though as I suspect that only one targetless (or two if in mirror mode) would be active at the same time for most use cases. Do you want me to add extra logic for this?