Fix T75810: Child bone frozen when both Auto IK and X-Axis mirror are used
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Apr 25 2020, 9:15 AM.

Details

Reviewers

Sybren A. Stüvel (sybren)
Sebastian Parborg (zeddb)
Germano Cavalcante (mano-wii)

Maniphest Tasks

T75810: Child bone frozen when both Auto IK and X-Axis mirror are used

Commits

rB08048f7cceb8: Fix T75810: Child bone frozen when both Auto IK and X-Axis mirror are

Summary

Caused by rBa6a9a12e8f32: Fix T75142: No autokeying with pose mode X-Mirror

Other relevant commits:
rBb8ca806b7798e2f8dd6effca8f0d081b3cd8c23f
rBde530a95dc7b482dc22c933b9b8b2a98c79b5663

The issue is caused by some leftover BONE_TRANSFORM_MIRROR flags on a bone from previous runs (file in the report had the flag still on forearm.R).
With these false leftover flags still set, pose_grab_with_ik() cannot work correctly.
Culprit commit above removed the early clearing of this flag on all bones, this should be restored [this happened in`count_set_pose_transflags()`].

This should only be done in the beginning of the transform process, so now still clear the flags early in 'createTransPose()' [but dont restore this in 'count_set_pose_transflags()' -- this will be called from special_aftertrans_update again, so placing the clearance here only complicates things (autokeyframe_pose() still needs to work as well)...]

Diff Detail

Repository: rB Blender

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Apr 25 2020, 9:15 AM

Harbormaster completed remote builds in B7727: Diff 24064.Apr 25 2020, 9:15 AM

Philipp Oeser (lichtwerk) requested review of this revision.Apr 25 2020, 9:15 AM

Philipp Oeser (lichtwerk) edited the summary of this revision. (Show Details)Apr 25 2020, 9:17 AM

Philipp Oeser (lichtwerk) added a project: Animation & Rigging.

As much as this solution may work, it seems to make things more difficult to understand and maintain.
The count_set_pose_transflags apparently sets the bone transform flags, but why doesn't it set the BONE_TRANSFORM_MIRROR as well?
Should it be renamed to count_and_set_pose_transflags_except_mirror_but_optionally_clean_them?

I know that for a fix, doing this type of refactor is not really necessary, but I think it would be good to consider.
At least think of a different name for this function.

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

In D7527#181367, @Germano Cavalcante (mano-wii) wrote:

As much as this solution may work, it seems to make things more difficult to understand and maintain.
The count_set_pose_transflags apparently sets the bone transform flags, but why doesn't it set the BONE_TRANSFORM_MIRROR as well?
Should it be renamed to count_and_set_pose_transflags_except_mirror_but_optionally_clean_them?

I know that for a fix, doing this type of refactor is not really necessary, but I think it would be good to consider.
At least think of a different name for this function.

Yeah, OK, this really seems a bit convoluted.
So instead of even doing this in count_set_pose_transflags, or try to set the flag in a central place in this commit as well, lets just clear those false flags once in the beginning and be done with it...
Will update in a bit.