Page MenuHome
Differential D4765

Implement mirroring in pose mode (absolute and relative).
ClosedPublic
Actions

Authored by Sebastian Parborg (zeddb) on Apr 30 2019, 5:20 PM.

Details

Reviewers
Brecht Van Lommel (brecht)
Alexander Gavrilov (angavrilov)
Joshua Leung (aligorith)
Campbell Barton (campbellbarton)
Commits
rBS09181540b5aa: Implement mirroring in pose mode (absolute and relative)
rB09181540b5aa: Implement mirroring in pose mode (absolute and relative)
Summary

This is a request from the animators at BI.

They wanted to be able to use the X-axis mirror in pose mode. With an option to use relative positioning or absolute positioning of the mirror.
Here is my initial take on it.

As noted in the code, I wanted to have ED_armature_pose_transform_mirror_update_relative in armature_utils.c. However, that function depends on the TransData type which is only available in editors/transform/transform.h. I'm wondering if there is some nice way we could solve this issue without too may hacks.

Diff Detail

Repository
rB Blender
Branch
arcpatch-D4765 (branched from master)
Build Status
Buildable 3481
Build 3481: arc lint + arc unit

Event Timeline

Sebastian Parborg (zeddb) created this revision.Apr 30 2019, 5:20 PM
Sybren A. Stüvel (sybren) mentioned this in T63901: Animation & Rigging Module.Apr 30 2019, 5:51 PM
Campbell Barton (campbellbarton) added a subscriber: Campbell Barton (campbellbarton).May 1 2019, 5:40 AM

Is there any reason not to use matrices instead of having to handle each mode separately?

Using matrices could simplify logic since it's then not necessary to have two functions:
ED_armature_pose_transform_mirror_update_relative and ED_armature_pose_transform_mirror_update. PoseInitData could store an offset rotation (3x3 matrix for eg). Which is optionally applied to the flipped rotation.

  • Storing all rotations as eulers is going to give issues when cancelling rotation, original values should be stored.
  • The case when pose bones use different rotation modes isn't accounted for, if it's not supported there should be a check for this case (since it's an obscure use-case), or handled by converting the rotation into a matrix, flipping, then applying: BKE_pchan_rot_to_mat3, then BKE_pchan_mat3_to_rot.
source/blender/blenloader/intern/versioning_280.c
3012

The flag should still be cleared.

source/blender/editors/transform/transform.h
386

Since we already store init data for pose bones this name is confusing, could be called PoseInitMirrorData since it's only when mirroring.

393

The original rotation values should be stored since the values in the euler can be different when converted back into quat/axis-angle.

Could store original values based on the bones type in a union.

union {
  float eul[3];
  float quat[4];
  float axis_angle[4];
}
source/blender/editors/transform/transform_generics.c
831–850

Converting to eulers seems strange here, rotation_between_quats_to_quat.

source/blender/makesdna/DNA_armature_types.h
147

Would call ARM_MIRROR_RELATIVE

source/blender/makesrna/intern/rna_armature.c
1362

No reason to abbreviate: use_mirror_relative

Campbell Barton (campbellbarton) requested changes to this revision.May 1 2019, 5:41 AM
This revision now requires changes to proceed.May 1 2019, 5:41 AM
Sebastian Parborg (zeddb) updated this revision to Diff 15037.May 1 2019, 4:07 PM
Sebastian Parborg (zeddb) marked 5 inline comments as done.

I've updated the code with your input.

Campbell Barton (campbellbarton) updated this revision to Diff 15051.May 2 2019, 4:35 AM
Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Sync with master, minor changes

Harbormaster completed remote builds in B3481: Diff 15051.May 2 2019, 4:35 AM
Campbell Barton (campbellbarton) added a comment.May 2 2019, 4:40 AM

Mirror relative isn't working properly, this is a simple example file.

mirror_example.blend672 KBDownload

source/blender/editors/transform/transform_generics.c
814

Don't think this is right, all that should be needed is the matrix from pchan and a delta matrix, calculated initially, the current matrix of the mirror bone shouldn't need to be taken into account.

Campbell Barton (campbellbarton) requested changes to this revision.May 2 2019, 4:41 AM
This revision now requires changes to proceed.May 2 2019, 4:41 AM
Sebastian Parborg (zeddb) updated this revision to Diff 15054.May 2 2019, 10:01 AM

The relative mirroring didn't work because you make a mistake when you updated the code. (Thanks for updating it to the current master!)

It wasn't and should not be storing the inverted rotation matrix of the mirrored bone. It was storing the inverted rotation matrix of the original bone (not the mirror).

I have changed that back and it is working correctly as before now.

I'm unsure what you mean with storing the delta matrix. When that information is calculated, the bones have not been transformed yet. So it can only store the initial data. To get the delta matrix, we need to have the rotation matrix of the current transformation and the inverted rotation matrix of the initial transformation.

Am I overthinking this?

Sebastian Parborg (zeddb) marked an inline comment as done.May 2 2019, 10:04 AM
Sebastian Parborg (zeddb) added inline comments.
source/blender/editors/transform/transform_generics.c
814

I need the matrix of the mirror bone to be able to apply the delta transformations to it. (?)

Campbell Barton (campbellbarton) added a comment.May 3 2019, 10:52 AM

Ack, sorry for the botched update.

I've just looked into simplifying the code,

  • Add support for scale.
  • Use one function, where relative transform just applies an extra matrix multiply (calculated at once on initialization).
  • Use 4x4 matrices so loc/scale/rotation is all handled at once.

P964 Update to D4765
1diff --git a/release/scripts/startup/bl_ui/space_view3d_toolbar.py b/release/scripts/startup/bl_ui/space_view3d_toolbar.py
2index 81804f6a399..d672984ca49 100644
3--- a/release/scripts/startup/bl_ui/space_view3d_toolbar.py
4+++ b/release/scripts/startup/bl_ui/space_view3d_toolbar.py
5@@ -192,9 +192,13 @@ class VIEW3D_PT_tools_posemode_options(View3DPanel, Panel):
6
7 def draw(self, context):
8 arm = context.active_object.data
9+ layout = self.layout
10
11- self.layout.prop(arm, "use_auto_ik")
12- self.layout.prop(arm, "use_mirror_x")
13+ layout.prop(arm, "use_auto_ik")
14+ layout.prop(arm, "use_mirror_x")
15+ col = layout.column()
16+ col.active = arm.use_mirror_x
17+ col.prop(arm, "use_mirror_relative")
18
19 # ********** default tools for paint modes ****************
20
21diff --git a/source/blender/blenloader/intern/versioning_280.c b/source/blender/blenloader/intern/versioning_280.c
22index d8d100db022..afbb512fe2f 100644
23--- a/source/blender/blenloader/intern/versioning_280.c
24+++ b/source/blender/blenloader/intern/versioning_280.c
25@@ -3009,7 +3009,7 @@ void blo_do_versions_280(FileData *fd, Library *UNUSED(lib), Main *bmain)
26 }
27
28 LISTBASE_FOREACH (bArmature *, arm, &bmain->armatures) {
29- arm->flag &= ~(ARM_FLAG_UNUSED_1 | ARM_FLAG_UNUSED_5 | ARM_FLAG_UNUSED_7 |
30+ arm->flag &= ~(ARM_FLAG_UNUSED_1 | ARM_FLAG_UNUSED_5 | ARM_MIRROR_RELATIVE |
31 ARM_FLAG_UNUSED_12);
32 }
33
34diff --git a/source/blender/editors/transform/transform.h b/source/blender/editors/transform/transform.h
35index 50fc1a276b9..3e24b833357 100644
36--- a/source/blender/editors/transform/transform.h
37+++ b/source/blender/editors/transform/transform.h
38@@ -383,6 +383,30 @@ typedef struct BoneInitData {
39 float zwidth;
40 } BoneInitData;
41
42+typedef struct PoseInitData_Mirror {
43+ /** Points to the bone which this info is initialized & restored to.
44+ * A NULL value is used to terminate the array. */
45+ struct bPoseChannel *pchan;
46+ struct {
47+ float loc[3];
48+ float size[3];
49+ union {
50+ float eul[3];
51+ float quat[4];
52+ float axis_angle[4];
53+ };
54+ float curve_in_x;
55+ float curve_out_x;
56+ float roll1;
57+ float roll2;
58+ } orig;
59+ /**
60+ * An extra offset to apply after mirroring.
61+ * Use with #ARM_MIRROR_RELATIVE.
62+ */
63+ float offset_mtx[4][4];
64+} PoseInitData_Mirror;
65+
66 typedef struct TransData {
67 /** Distance needed to affect element (for Proportionnal Editing). */
68 float dist;
69@@ -892,6 +916,7 @@ void flushTransSeq(TransInfo *t);
70 void flushTransTracking(TransInfo *t);
71 void flushTransMasking(TransInfo *t);
72 void flushTransPaintCurve(TransInfo *t);
73+void restoreMirrorPoseBones(TransDataContainer *tc);
74 void restoreBones(TransDataContainer *tc);
75
76 /*********************** transform_gizmo.c ********** */
77diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c
78index 6c1da5ae825..ba565972689 100644
79--- a/source/blender/editors/transform/transform_conversions.c
80+++ b/source/blender/editors/transform/transform_conversions.c
81@@ -1201,6 +1201,74 @@ static short pose_grab_with_ik(Main *bmain, Object *ob)
82 return (tot_ik) ? 1 : 0;
83 }
84
85+static void pose_mirror_info_init(PoseInitData_Mirror *pid,
86+ bPoseChannel *pchan,
87+ bPoseChannel *pchan_orig,
88+ bool is_mirror_relative)
89+{
90+ pid->pchan = pchan;
91+ copy_v3_v3(pid->orig.loc, pchan->loc);
92+ copy_v3_v3(pid->orig.size, pchan->size);
93+ pid->orig.curve_in_x = pchan->curve_in_x;
94+ pid->orig.curve_out_x = pchan->curve_out_x;
95+ pid->orig.roll1 = pchan->roll1;
96+ pid->orig.roll2 = pchan->roll2;
97+
98+ if (pchan->rotmode > 0) {
99+ copy_v3_v3(pid->orig.eul, pchan->eul);
100+ }
101+ else if (pchan->rotmode == ROT_MODE_AXISANGLE) {
102+ copy_v3_v3(pid->orig.axis_angle, pchan->rotAxis);
103+ pid->orig.axis_angle[3] = pchan->rotAngle;
104+ }
105+ else {
106+ copy_qt_qt(pid->orig.quat, pchan->quat);
107+ }
108+
109+ if (is_mirror_relative) {
110+ float pchan_mtx[4][4];
111+ float pchan_mtx_mirror[4][4];
112+
113+ float flip_mtx[4][4];
114+ unit_m4(flip_mtx);
115+ flip_mtx[0][0] = -1;
116+
117+ BKE_pchan_to_mat4(pchan_orig, pchan_mtx_mirror);
118+ BKE_pchan_to_mat4(pchan, pchan_mtx);
119+
120+ mul_m4_m4m4(pchan_mtx_mirror, pchan_mtx_mirror, flip_mtx);
121+ mul_m4_m4m4(pchan_mtx_mirror, flip_mtx, pchan_mtx_mirror);
122+
123+ invert_m4(pchan_mtx_mirror);
124+ mul_m4_m4m4(pid->offset_mtx, pchan_mtx, pchan_mtx_mirror);
125+ }
126+ else {
127+ unit_m4(pid->offset_mtx);
128+ }
129+}
130+
131+static void pose_mirror_info_restore(const PoseInitData_Mirror *pid)
132+{
133+ bPoseChannel *pchan = pid->pchan;
134+ copy_v3_v3(pchan->loc, pid->orig.loc);
135+ copy_v3_v3(pchan->size, pid->orig.size);
136+ pchan->curve_in_x = pid->orig.curve_in_x;
137+ pchan->curve_out_x = pid->orig.curve_out_x;
138+ pchan->roll1 = pid->orig.roll1;
139+ pchan->roll2 = pid->orig.roll2;
140+
141+ if (pchan->rotmode > 0) {
142+ copy_v3_v3(pchan->eul, pid->orig.eul);
143+ }
144+ else if (pchan->rotmode == ROT_MODE_AXISANGLE) {
145+ copy_v3_v3(pchan->rotAxis, pid->orig.axis_angle);
146+ pchan->rotAngle = pid->orig.axis_angle[3];
147+ }
148+ else {
149+ copy_qt_qt(pchan->quat, pid->orig.quat);
150+ }
151+}
152+
153 /**
154 * When objects array is NULL, use 't->data_container' as is.
155 */
156@@ -1225,6 +1293,8 @@ static void createTransPose(TransInfo *t)
157 continue;
158 }
159
160+ const bool mirror = ((arm->flag & ARM_MIRROR_EDIT) != 0);
161+
162 /* set flags and count total */
163 tc->data_len = count_set_pose_transflags(ob, t->mode, t->around, has_translate_rotate);
164 if (tc->data_len == 0) {
165@@ -1247,6 +1317,25 @@ static void createTransPose(TransInfo *t)
166 has_translate_rotate[0] = true;
167 }
168 }
169+
170+ if (mirror) {
171+ int total_mirrored = 0;
172+ for (bPoseChannel *pchan = ob->pose->chanbase.first; pchan; pchan = pchan->next) {
173+ if ((pchan->bone->flag & BONE_TRANSFORM) &&
174+ BKE_pose_channel_get_mirrored(ob->pose, pchan->name)) {
175+ total_mirrored++;
176+ }
177+ }
178+
179+ PoseInitData_Mirror *pid = MEM_mallocN((total_mirrored + 1) * sizeof(PoseInitData_Mirror),
180+ "PoseInitData_Mirror");
181+
182+ /* Trick to terminate iteration. */
183+ pid[total_mirrored].pchan = NULL;
184+
185+ tc->custom.type.data = pid;
186+ tc->custom.type.use_free = true;
187+ }
188 }
189
190 /* if there are no translatable bones, do rotation */
191@@ -1269,6 +1358,19 @@ static void createTransPose(TransInfo *t)
192 short ik_on = 0;
193 int i;
194
195+ PoseInitData_Mirror *pid = tc->custom.type.data;
196+ int pid_index = 0;
197+ bArmature *arm;
198+
199+ /* check validity of state */
200+ arm = BKE_armature_from_object(tc->poseobj);
201+ if ((arm == NULL) || (ob->pose == NULL)) {
202+ continue;
203+ }
204+
205+ const bool mirror = ((arm->flag & ARM_MIRROR_EDIT) != 0);
206+ const bool is_mirror_relative = ((arm->flag & ARM_MIRROR_RELATIVE) != 0);
207+
208 tc->poseobj = ob; /* we also allow non-active objects to be transformed, in weightpaint */
209
210 /* init trans data */
211@@ -1285,6 +1387,15 @@ static void createTransPose(TransInfo *t)
212 for (bPoseChannel *pchan = ob->pose->chanbase.first; pchan; pchan = pchan->next) {
213 if (pchan->bone->flag & BONE_TRANSFORM) {
214 add_pose_transdata(t, pchan, ob, tc, td);
215+
216+ if (mirror) {
217+ bPoseChannel *pchan_mirror = BKE_pose_channel_get_mirrored(ob->pose, pchan->name);
218+ if (pchan_mirror) {
219+ pose_mirror_info_init(&pid[pid_index], pchan_mirror, pchan, is_mirror_relative);
220+ pid_index++;
221+ }
222+ }
223+
224 td++;
225 }
226 }
227@@ -1304,6 +1415,27 @@ static void createTransPose(TransInfo *t)
228 t->flag &= ~T_PROP_EDIT_ALL;
229 }
230
231+void restoreMirrorPoseBones(TransDataContainer *tc)
232+{
233+ bArmature *arm;
234+
235+ if (tc->obedit) {
236+ arm = tc->obedit->data;
237+ }
238+ else {
239+ BLI_assert(tc->poseobj != NULL);
240+ arm = tc->poseobj->data;
241+ }
242+
243+ if (!(arm->flag & ARM_MIRROR_EDIT)) {
244+ return;
245+ }
246+
247+ for (PoseInitData_Mirror *pid = tc->custom.type.data; pid->pchan; pid++) {
248+ pose_mirror_info_restore(pid);
249+ }
250+}
251+
252 void restoreBones(TransDataContainer *tc)
253 {
254 bArmature *arm;
255diff --git a/source/blender/editors/transform/transform_generics.c b/source/blender/editors/transform/transform_generics.c
256index 8ddc5461e9a..b8a3c9f84e7 100644
257--- a/source/blender/editors/transform/transform_generics.c
258+++ b/source/blender/editors/transform/transform_generics.c
259@@ -781,6 +781,45 @@ static void recalcData_spaceclip(TransInfo *t)
260 }
261 }
262
263+/**
264+ * if pose bone (partial) selected, copy data.
265+ * context; posemode armature, with mirror editing enabled.
266+ *
267+ * \param pid: Optional, apply relative transform when set.
268+ */
269+static void pose_transform_mirror_update(Object *ob, PoseInitData_Mirror *pid)
270+{
271+ float flip_mtx[4][4];
272+ unit_m4(flip_mtx);
273+ flip_mtx[0][0] = -1;
274+
275+ for (bPoseChannel *pchan_orig = ob->pose->chanbase.first; pchan_orig;
276+ pchan_orig = pchan_orig->next) {
277+ /* no layer check, correct mirror is more important */
278+ if (pchan_orig->bone->flag & BONE_TRANSFORM) {
279+ bPoseChannel *pchan = BKE_pose_channel_get_mirrored(ob->pose, pchan_orig->name);
280+
281+ if (pchan) {
282+ /* we assume X-axis flipping for now */
283+ pchan->curve_in_x = pchan_orig->curve_in_x * -1;
284+ pchan->curve_out_x = pchan_orig->curve_out_x * -1;
285+ pchan->roll1 = pchan_orig->roll1 * -1; // XXX?
286+ pchan->roll2 = pchan_orig->roll2 * -1; // XXX?
287+
288+ float pchan_mtx_final[4][4];
289+ BKE_pchan_to_mat4(pchan_orig, pchan_mtx_final);
290+ mul_m4_m4m4(pchan_mtx_final, pchan_mtx_final, flip_mtx);
291+ mul_m4_m4m4(pchan_mtx_final, flip_mtx, pchan_mtx_final);
292+ if (pid) {
293+ mul_m4_m4m4(pchan_mtx_final, pid->offset_mtx, pchan_mtx_final);
294+ pid++;
295+ }
296+ BKE_pchan_apply_mat4(pchan, pchan_mtx_final, false);
297+ }
298+ }
299+ }
300+}
301+
302 /* helper for recalcData() - for object transforms, typically in the 3D view */
303 static void recalcData_objects(TransInfo *t)
304 {
305@@ -992,6 +1031,19 @@ static void recalcData_objects(TransInfo *t)
306 Object *ob = tc->poseobj;
307 bArmature *arm = ob->data;
308
309+ if (arm->flag & ARM_MIRROR_EDIT) {
310+ if (t->state != TRANS_CANCEL) {
311+ PoseInitData_Mirror *pid = NULL;
312+ if (arm->flag & ARM_MIRROR_RELATIVE) {
313+ pid = tc->custom.type.data;
314+ }
315+ pose_transform_mirror_update(ob, pid);
316+ }
317+ else {
318+ restoreMirrorPoseBones(tc);
319+ }
320+ }
321+
322 /* if animtimer is running, and the object already has animation data,
323 * check if the auto-record feature means that we should record 'samples'
324 * (i.e. un-editable animation values)
325diff --git a/source/blender/makesdna/DNA_armature_types.h b/source/blender/makesdna/DNA_armature_types.h
326index 483ff3483d3..fd40459503e 100644
327--- a/source/blender/makesdna/DNA_armature_types.h
328+++ b/source/blender/makesdna/DNA_armature_types.h
329@@ -144,7 +144,7 @@ typedef enum eArmature_Flag {
330 ARM_POSEMODE = (1 << 4),
331 ARM_FLAG_UNUSED_5 = (1 << 5), /* cleared */
332 ARM_DELAYDEFORM = (1 << 6),
333- ARM_FLAG_UNUSED_7 = (1 << 7), /* cleared */
334+ ARM_MIRROR_RELATIVE = (1 << 7),
335 ARM_MIRROR_EDIT = (1 << 8),
336 ARM_AUTO_IK = (1 << 9),
337 /** made option negative, for backwards compat */
338diff --git a/source/blender/makesrna/intern/rna_armature.c b/source/blender/makesrna/intern/rna_armature.c
339index 5461aaa0f1a..fe197faecce 100644
340--- a/source/blender/makesrna/intern/rna_armature.c
341+++ b/source/blender/makesrna/intern/rna_armature.c
342@@ -1359,6 +1359,13 @@ static void rna_def_armature(BlenderRNA *brna)
343 RNA_def_property_update(prop, 0, "rna_Armature_redraw_data");
344 RNA_def_property_flag(prop, PROP_LIB_EXCEPTION);
345
346+ prop = RNA_def_property(srna, "use_mirror_relative", PROP_BOOLEAN, PROP_NONE);
347+ RNA_def_property_boolean_sdna(prop, NULL, "flag", ARM_MIRROR_RELATIVE);
348+ RNA_def_property_ui_text(
349+ prop, "Relative Mirror", "Apply relative transformations in X-mirror mode");
350+ RNA_def_property_update(prop, 0, "rna_Armature_redraw_data");
351+
352+ RNA_def_property_flag(prop, PROP_LIB_EXCEPTION);
353 prop = RNA_def_property(srna, "use_auto_ik", PROP_BOOLEAN, PROP_NONE);
354 RNA_def_property_boolean_sdna(prop, NULL, "flag", ARM_AUTO_IK);
355 RNA_def_property_ui_text(

Sebastian Parborg (zeddb) marked an inline comment as done.EditedMay 3 2019, 3:31 PM

@Campbell Barton (campbellbarton) your code doesn't handle all the cases in relative mode:

Campbell Barton (campbellbarton) updated this revision to Diff 15115.EditedMay 4 2019, 3:14 AM

Fixed translation issue w/ relative matrices & updated the diff.
Also minor changes:

  • Don't calculate the relative matrix when it's not used.
  • Grey out relative mirror when mirror isn't enabled.

Think this is ready for master.

Note, we'll want this functionality for non-transform operators (Clear loc/scale/rotation for eg), logic for mirroring should be moved into utility functions eventually.

Harbormaster completed remote builds in B3505: Diff 15115.May 4 2019, 3:14 AM
Harbormaster completed remote builds in B3505: Diff 15115.
Campbell Barton (campbellbarton) accepted this revision.May 4 2019, 3:47 AM
This revision is now accepted and ready to land.May 4 2019, 3:47 AM
Closed by commit rB09181540b5aa: Implement mirroring in pose mode (absolute and relative) (authored by Sebastian Parborg (zeddb)). · Explain WhyMay 6 2019, 9:51 AM
This revision was automatically updated to reflect the committed changes.
Stanislav Ovcharov (Stan1) added a subscriber: Stanislav Ovcharov (Stan1).May 6 2019, 4:12 PM

Does it solve this problem? - https://blender.community/c/rightclickselect/g1cbbc/

Sebastian Parborg (zeddb) added a comment.May 6 2019, 4:17 PM
In D4765#109808, @Stanislav Ovcharov (Stan1) wrote:

Does it solve this problem? - https://blender.community/c/rightclickselect/g1cbbc/

Relative mode could perhaps solve that issue yes. But it is not exactly what is requested in that post.

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
1 line
source/
blender/
blenloader/
intern/
2 lines
editors/
armature/
38 lines
include/
2 lines
transform/
16 lines
108 lines
62 lines
makesdna/
2 lines
makesrna/
intern/
7 lines

Diff 15051

View Options

release/scripts/startup/bl_ui/space_view3d_toolbar.py

Loading...
View Options

source/blender/blenloader/intern/versioning_280.c

Loading...
View Options

source/blender/editors/armature/armature_utils.c

Loading...
View Options

source/blender/editors/include/ED_armature.h

Loading...
View Options

source/blender/editors/transform/transform.h

Loading...
View Options

source/blender/editors/transform/transform_conversions.c

Loading...
View Options

source/blender/editors/transform/transform_generics.c

Loading...
View Options

source/blender/makesdna/DNA_armature_types.h

Loading...
View Options

source/blender/makesrna/intern/rna_armature.c

Loading...