Fix T38407: Bone roll calculation flips local axes at wrong rotation angle.

Basic idea is now to have the transformes bones keep "facing" the armature's Z axis, see comments in code for details.

That might not be ideal, but at least we now have humanly predictable and consistent results.

2 files changed, 22 insertions, 29 deletions

diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c
index 31123827c4c..ceb4a49882d 100644
--- a/source/blender/editors/transform/transform_conversions.c
+++ b/source/blender/editors/transform/transform_conversions.c
@@ -1182,6 +1182,7 @@ static void createTransArmatureVerts(TransInfo *t)
 				}
 			}
 			else {
+				const float Z[3] = {0.0f, 0.0f, 1.0f};
 				if (ebo->flag & BONE_TIPSEL) {
 					copy_v3_v3(td->iloc, ebo->tail);
 					copy_v3_v3(td->center, (t->around == V3D_LOCAL) ? ebo->head : td->iloc);
@@ -1196,8 +1197,9 @@ static void createTransArmatureVerts(TransInfo *t)
 					ED_armature_ebone_to_mat3(ebo, td->axismtx);
 
 					if ((ebo->flag & BONE_ROOTSEL) == 0) {
+						/* To fix roll, see comments in transform_generic.c::recalcData_objects() */
 						td->extra = ebo;
-						td->ival = ebo->roll;
+						td->ival = ebo->roll - ED_rollBoneToVector(ebo, Z, false);
 					}
 
 					td->ext = NULL;
@@ -1219,8 +1221,9 @@ static void createTransArmatureVerts(TransInfo *t)
 
 					ED_armature_ebone_to_mat3(ebo, td->axismtx);
 
-					td->extra = ebo; /* to fix roll */
-					td->ival = ebo->roll;
+					/* To fix roll, see comments in transform_generic.c::recalcData_objects() */
+					td->extra = ebo;
+					td->ival = ebo->roll - ED_rollBoneToVector(ebo, Z, false);
 
 					td->ext = NULL;
 					td->val = NULL;
diff --git a/source/blender/editors/transform/transform_generics.c b/source/blender/editors/transform/transform_generics.c
index af4beff4bc9..26c907673f7 100644
--- a/source/blender/editors/transform/transform_generics.c
+++ b/source/blender/editors/transform/transform_generics.c
@@ -798,35 +798,25 @@ static void recalcData_objects(TransInfo *t)
 			
 			if (!ELEM3(t->mode, TFM_BONE_ROLL, TFM_BONE_ENVELOPE, TFM_BONESIZE)) {
 				/* fix roll */
+				/* Previous method basically tried to get a rotation transform from org ebo Y axis to final ebo Y axis,
+				 * apply this same rotation to org ebo Z axis to get an "up_axis", and compute a new roll value
+				 * so that final ebo's Z axis would be "aligned" with that up_axis.
+				 * There are two issues with that method:
+				 *   - There are many cases where the computed up_axis does not gives a result people would expect.
+				 *   - Applying a same transform in a single step or in several smaller ones would not give the same
+				 *     result! See e.g. T38407.
+				 * Now, instead of trying to be smart with complex axis/angle handling, just store diff roll
+				 * (diff between real init roll and virtual init roll where bone's Z axis would be "aligned" with
+				 * armature's Z axis), and do the reverse to get final roll.
+				 * This method at least gives predictable, consistent results (the bone basically keeps "facing"
+				 * the armature's Z axis).
+				 */
 				for (i = 0; i < t->total; i++, td++) {
 					if (td->extra) {
-						float vec[3], up_axis[3];
-						float qrot[4];
-						float roll;
-						
-						ebo = td->extra;
+						const float Z[3] = {0.0f, 0.0f, 1.0f};
 
-						if (t->state == TRANS_CANCEL) {
-							/* restore roll */
-							ebo->roll = td->ival;
-						}
-						else {
-							copy_v3_v3(up_axis, td->axismtx[2]);
-							
-							if (t->mode != TFM_ROTATION) {
-								sub_v3_v3v3(vec, ebo->tail, ebo->head);
-								normalize_v3(vec);
-								rotation_between_vecs_to_quat(qrot, td->axismtx[1], vec);
-								mul_qt_v3(qrot, up_axis);
-							}
-							else {
-								mul_m3_v3(t->mat, up_axis);
-							}
-							
-							/* roll has a tendency to flip in certain orientations - [#34283], [#33974] */
-							roll = ED_rollBoneToVector(ebo, up_axis, false);
-							ebo->roll = angle_compat_rad(roll, td->ival);
-						}
+						ebo = td->extra;
+						ebo->roll = td->ival + ED_rollBoneToVector(ebo, Z, false);
 					}
 				}
 			}