Page MenuHome

Fixes T83657: Resetting pose transforms gets mirrored when X-Mirror is enabled in Pose Mode.
ClosedPublic

Authored by Karthik Rangasai Sivaraman (rangasai) on Dec 28 2020, 3:42 PM.

Details

Summary

This commit fixes T83657 where the reset transforms were not being applied onto the other side as well when animating and posing with X-Mirror toggled on.
The pose channel clear functions were not conditioned on the X Mirror toggle and I have added it to mirror the transform resets.

Diff Detail

Repository
rB Blender

Event Timeline

Sybren A. Stüvel (sybren) requested changes to this revision.Jan 8 2021, 3:52 PM

Thanks for the patch @Karthik Rangasai Sivaraman (rangasai). Functionally it seems to do exactly what's needed. I do have a few small remarks about the code itself, which should be easy to address.

I'm not a fan of "helper" functions. IMO functions should be named after their purpose, and not after what they are vaguely related to. How about this approach?

  • Keep pchan_clear_XXX() functions named as they are, and
  • name the new functions pchan_clear_XXX_with_mirrored().
source/blender/editors/armature/pose_transform.c
937–938

Comments are certainly welcome, but I think it can be clarified and shortened a bit. How about something like "Clear the scale. When X-mirror is enabled, also clear the scale of the mirrored pose channel." ?

941–942

This can be optimized a bit. If mirror editing is disabled, there is no need to look up the mirrored pose channel:

if (pose->flag & POSE_MIRROR_EDIT) {
  bPoseChannel *pchan_mirror = BKE_pose_channel_get_mirrored(pose, pchan->name);
  if (pchan_mirror != NULL) {
    pchan_clear_scale_helper(pchan_mirror);
  }
}
This revision now requires changes to proceed.Jan 8 2021, 3:52 PM

Cleanup: Updating functions names, comments and taking care of code optimizations.

Summary:
The necessary changes in the function names have been made and the code optimisation is also done during the retrieval of pchan_mirror when X-mirror is enabled.

Karthik Rangasai Sivaraman (rangasai) marked 2 inline comments as done.

Cleanup: Updating Comments.

Summary:
Minor changes in the comments for the functions.

Sybren A. Stüvel (sybren) requested changes to this revision.Jan 11 2021, 12:45 PM

The function names are now incorrect, they should be switched. As it is now, pchan_clear_scale_with_mirrored() clears the scale of a single pose channel only, whereas pchan_clear_scale() includes the mirrored channel.

This revision now requires changes to proceed.Jan 11 2021, 12:45 PM

Oh sorry, I thought the changes you mentioned were with respect to the changes I made in the previous revision. I will push the changes with the swapped function names immediately.

Cleanup: Updating functions names.

Thanks for the patch! I'll commit it to master for you.

This revision is now accepted and ready to land.Jan 18 2021, 11:53 AM

Thanks for the patch! I'll commit it to master for you.

Thank you @Sybren A. Stüvel (sybren)