Details

Reviewers

Sebastian Parborg (zeddb)
Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

Maniphest Tasks

T65671: Armature X-Mirror inconsistencies

Commits

rBf041d2f1163c: Fix T65671: Armature X-Mirror inconsistencies

Summary

This is my first patch, be nice.
This is only for fixing edit mode X-Mirror. I'd also like to fix pose mode X-mirror, but I didn't want my first patch to be too big. Therefore this is only a partial fix for T65671.

I also don't know for sure if I'm adding all the appropriate update functions in all the appropriate circumstances using RNA_def_property_update(). But everything seems to work fine when I'm playing with it.

Thanks to @Jacques Lucke (JacquesLucke) for his video! :)

Slightly long video showing the fix in action:

Diff Detail

Repository

rB Blender

Build Status

Buildable 4272
Build 4272: arc lint + arc unit

Event Timeline

Demeter Dzadik (Mets) created this revision.Aug 3 2019, 12:13 PM

Harbormaster completed remote builds in B4272: Diff 16815.Aug 3 2019, 12:13 PM

Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Aug 3 2019, 12:14 PM

Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)

Demeter Dzadik (Mets) added a task: T65671: Armature X-Mirror inconsistencies.Aug 3 2019, 12:18 PM

Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Aug 3 2019, 12:22 PM

Un-removed check_select

I understand now what check_select was for :) Without it, the entire armature would get mirrored when a transform operator was used(eg. from the 3d viewport) And also the tail would be mirrored when only the head is moved, and vice versa.

There are still some issues in this area, namely that when a value is changed via the UI value sliders or python, the entire bone gets mirrored no matter what, instead of just that value, and also cancelling a slider drag will not reset the mirrored bone correctly. This is not a regression, it's a separate bug that always existed.

Harbormaster completed remote builds in B4274: Diff 16817.Aug 3 2019, 5:51 PM

Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Aug 3 2019, 8:07 PM

Demeter Dzadik (Mets) edited the summary of this revision. (Show Details)Aug 4 2019, 11:24 AM

Bone roll was not being mirrored when head or tail is moved. It needs to always be mirrored.
+ Some commenting and cleanup

Harbormaster completed remote builds in B4275: Diff 16821.Aug 4 2019, 12:20 PM

There is a non-obvious thing here. Changing the bone segments requires a dependency graph rebuild in pose mode (rna_Armature_dependency_update), but it does not in edit mode so this is ok as is.

source/blender/editors/armature/armature_utils.c
385–387	Follow comment style: https://wiki.blender.org/wiki/Style_Guide/C_Cpp#Comments
442	This is redundant.

This revision now requires changes to proceed.Aug 12 2019, 10:15 AM

Cleaned up commenting, hope it adheres to convention now, but let me know if it doesn't.
Removed redundant if statement
Fixed head radius mirroring was not being cancelled correctly. (This was always like this)

Harbormaster completed remote builds in B4393: Diff 17060.Aug 13 2019, 1:16 AM

Demeter Dzadik (Mets) marked 2 inline comments as done.Aug 13 2019, 1:22 AM

Brecht Van Lommel (brecht) accepted this revision.Aug 13 2019, 5:59 AM

This revision is now accepted and ready to land.Aug 13 2019, 5:59 AM

@Sebastian Parborg (zeddb) can you help getting this committed?

Closed by commit rBf041d2f1163c: Fix T65671: Armature X-Mirror inconsistencies (authored by Demeter Dzadik (Mets), committed by Sebastian Parborg (zeddb)). · Explain WhyAug 21 2019, 10:01 AM

This revision was automatically updated to reflect the committed changes.

Sebastian Parborg (zeddb) added a commit: rBf041d2f1163c: Fix T65671: Armature X-Mirror inconsistencies.