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.
Details
- Reviewers
Sybren A. Stüvel (sybren) - Group Reviewers
Animation & Rigging - Maniphest Tasks
- T83657: Pose Mode: Clearing transform doesn't respect Mirror X
- Commits
- rBc85317e66940: Fix T83657: Pose Mode: Clearing transform doesn't respect Mirror X
rBf7a5695676dc: Fix T83657: Pose Mode: Clearing transform doesn't respect Mirror X
Diff Detail
- Repository
- rB Blender
Event Timeline
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);
}
} | |
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.
Cleanup: Updating Comments.
Summary:
Minor changes in the comments for the functions.
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.
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.