Transform: Pose: Partial support for Auto IK + X-Mirror
ClosedPublic
Actions

Authored by Germano Cavalcante (mano-wii) on Sep 20 2019, 5:00 PM.

Details

Reviewers

Campbell Barton (campbellbarton)
Sybren A. Stüvel (sybren)

Maniphest Tasks

T69572: Auto IK fails with X-Axis Mirror

Commits

rBde530a95dc7b: Transform: Pose: Partial support for Auto IK + X-Mirror

Summary

Fix T69572

This patch proposes to use Auto-IK for mirrored bones.
It would take a lot of changes to support Relative-Mirror as well, so I added a comment indicating this feature as TODO.

Diff Detail

Repository: rB Blender

Event Timeline

Germano Cavalcante (mano-wii) created this revision.Sep 20 2019, 5:00 PM

Harbormaster completed remote builds in B5050: Diff 18390.Sep 20 2019, 5:00 PM

Revert unnecessary change.

Harbormaster completed remote builds in B5051: Diff 18391.Sep 20 2019, 5:06 PM

Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)Nov 6 2019, 9:51 PM

Germano Cavalcante (mano-wii) edited the summary of this revision. (Show Details)

Thanks for the fix, it works! I just added myself as a reviewer as I was looking into T69572.

It would take a lot of changes to support Relative-Mirror as well, so I added a comment indicating this feature as TODO.

In that case we probably want to disable Relative Mirror when Auto IK is enabled, and vice versa. IMO it's better to have such restrictions explicit in the UI (and the manual, of course). What do you think @Germano Cavalcante (mano-wii) ?

source/blender/editors/transform/transform_generics.c
789	The fact that this doesn't happen when Auto IK is enabled should be documented here.
801	Changing this to `if (pchan == NULL) continue;` will cause another unindent of the code below. IMO it's also much easier to read, as it's immediately clear that nothing will happen when a mirrored pose channel cannot be found.
821	Comments should be a sentence, so start with a captial letter and end with a period.
824	`if (data == NULL) continue;` is clearer here IMO.
827	TODOs should have a name, indicating who to ask for more info about the TODO.

Improve comments
Improve code readability by using 'continue'
Clear 'BONE_TRANSFORM_MIRROR'
UI: Disable Relative Mirror when Auto IK is enabled

Harbormaster completed remote builds in B6147: Diff 20586.Jan 2 2020, 4:07 PM

In D5862#152826, @Sybren A. Stüvel (sybren) wrote:

... In that case we probably want to disable Relative Mirror when Auto IK is enabled, and vice versa. IMO it's better to have such restrictions explicit in the UI (and the manual, of course)...

Good point, I made the change.
But remember that this limitation is only for bones affected with Auto-IK.
So if the user selects a bone with parent and another bone without parent Relative Mirror still works on one of them.

I informed the limitation in the rna description, is it still necessary to update the manual? (Someday this feature may be added).

The RNA description is nice. Given that things can get a bit more complex than a single-bit "usable/not usable" I think it's good as-is.