Page MenuHome

Fix T73051: Multiple IK chains influencing the same bone don't work
ClosedPublic

Authored by Sybren A. Stüvel (sybren) on Jan 28 2020, 2:13 PM.

Details

Summary

This patch fixes T73051: Multiple IK chains influencing the same bone don't work. The cause of the issue was the absence of relations in the depsgraph between IK solvers of overlapping chains.

For example, this rig has two overlapping chains:

Before this patch, the Thumb IK constraint was not updated when the Wrist IK target moved. This patch adds a dependency to do this update.

Each POSE_IK_SOLVER depsgraph operation node depends on the chain bones' BONE_READY nodes, and is a dependency of those bones' BONE_DONE nodes. This patch adds a relation between the POSE_IK_SOLVER of one chain and the chain root of any overlapping chain higher up in the bone hierarchy. This ensures proper updates and fixes T73051.

There could be an alternative approach to solve this. The root_map object contains the set of IK chain roots for each bone. For each such set that has more than one root, we could add this relation to the depsgraph. However, this would not only require additional code for the RootPChanMap class, it would also require inspecting the hierarchy to determine the hierarchical order of those bones. As such it would produce more & more complex code than the current approach, which is why I didn't implement this.

This is a test file that adds a Spine IK chain to the example file of T73051, to ensure that this works as well:

This is a simpler case with three bones and two overlapping IK chains:

Diff Detail

Repository
rB Blender
Branch
wip-sybren-T73051-multiple-ik-chains (branched from master)
Build Status
Buildable 6415
Build 6415: arc lint + arc unit

Event Timeline

Sybren A. Stüvel (sybren) added inline comments.
source/blender/depsgraph/intern/builder/deg_builder_pchanmap.h
42

The addition of const was needed to allow build_inter_ik_chains(…, const RootPChanMap *root_map) below. I can commit this as a separate cleanup commit if desired.

Conceptually this seems fine.

I'm not familiar with this IK dependency building code, so unless Sergey wants me to review it as well I'll leave it to him.

Think it's fine.

As for solution, keep it simple, at least for now. Can always go more complicated code if things appears in a profiler. There are already a lot of queries to rootmap in constraints code, one extra lookup will not hurt in a real files.

Probably open some production files, make sure they work and commit to 2.82 branch as a bugfix.

This revision is now accepted and ready to land.Jan 28 2020, 5:28 PM