Transformation Channel Driver Variables - "Proper Localspace"

By popular demand, the "Transformation Channel" driver variable type
now has a "local space" transform space option which uses the same
magic that constraints use for defining local-space. This is what many
bug reporters and feature requesters have moaned about for a while
now, so after reviewing several of the bug reports which lead to the
current situation, here is what has been much-wanted for so long!

In order to implement this, I've:
- renamed the old "Local Space" option here to "Transformation Space",
in order to prevent old rigs breaking. This has also been kept, as it
is useful for #21384 (though perhaps with this new option it isn't
needed anymore)
- reviewed my fix for #20870 (IIRC, a Durian-era bug), which related
to the non-uniqueness of matrix->euler decomposition

5 files changed, 75 insertions, 24 deletions

diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c
index d6a9d950015..be2a6a762ee 100644
--- a/source/blender/blenkernel/intern/fcurve.c
+++ b/source/blender/blenkernel/intern/fcurve.c
@@ -42,6 +42,7 @@
 #include "MEM_guardedalloc.h"
 
 #include "DNA_anim_types.h"
+#include "DNA_constraint_types.h"
 #include "DNA_object_types.h"
 
 #include "BLI_blenlib.h"
@@ -52,6 +53,7 @@
 #include "BKE_animsys.h"
 #include "BKE_action.h"
 #include "BKE_armature.h"
+#include "BKE_constraint.h"
 #include "BKE_curve.h" 
 #include "BKE_global.h"
 #include "BKE_object.h"
@@ -1197,7 +1199,7 @@ static float dvar_eval_transChan (ChannelDriver *driver, DriverVar *dvar)
 	Object *ob= (Object *)dtar_id_ensure_proxy_from(dtar->id);
 	bPoseChannel *pchan;
 	float mat[4][4];
-	float eul[3] = {0.0f,0.0f,0.0f};
+	float oldEul[3] = {0.0f,0.0f,0.0f};
 	short useEulers=0, rotOrder=ROT_MODE_EUL;
 	
 	/* check if this target has valid data */
@@ -1210,36 +1212,62 @@ static float dvar_eval_transChan (ChannelDriver *driver, DriverVar *dvar)
 	/* try to get posechannel */
 	pchan= get_pose_channel(ob->pose, dtar->pchan_name);
 	
-	/* check if object or bone, and get transform matrix accordingly */
+	/* check if object or bone, and get transform matrix accordingly 
+	 *	- "useEulers" code is used to prevent the problems associated with non-uniqueness
+	 *	  of euler decomposition from matrices [#20870]
+	 *	- localspace is for [#21384], where parent results are not wanted
+	 *	  but local-consts is for all the common "corrective-shapes-for-limbs" situations
+	 */
 	if (pchan) {
 		/* bone */
 		if (pchan->rotmode > 0) {
-			VECCOPY(eul, pchan->eul);
+			VECCOPY(oldEul, pchan->eul);
 			rotOrder= pchan->rotmode;
 			useEulers = 1;
 		}
 		
 		if (dtar->flag & DTAR_FLAG_LOCALSPACE) {
-			/* specially calculate local matrix, since chan_mat is not valid 
-			 * since it stores delta transform of pose_mat so that deforms work
-			 */
-			pchan_to_mat4(pchan, mat);
+			if (dtar->flag & DTAR_FLAG_LOCAL_CONSTS) {
+				/* just like how the constraints do it! */
+				copy_m4_m4(mat, pchan->pose_mat);
+				constraint_mat_convertspace(ob, pchan, mat, CONSTRAINT_SPACE_POSE, CONSTRAINT_SPACE_LOCAL);
+			}
+			else {
+				/* specially calculate local matrix, since chan_mat is not valid 
+				 * since it stores delta transform of pose_mat so that deforms work
+				 * so it cannot be used here for "transform" space
+				 */
+				pchan_to_mat4(pchan, mat);
+			}
 		}
-		else
+		else {
+			/* worldspace matrix */
 			mul_m4_m4m4(mat, pchan->pose_mat, ob->obmat);
+		}
 	}
 	else {
 		/* object */
 		if (ob->rotmode > 0) {
-			VECCOPY(eul, ob->rot);
+			VECCOPY(oldEul, ob->rot);
 			rotOrder= ob->rotmode;
 			useEulers = 1;
 		}
 		
-		if (dtar->flag & DTAR_FLAG_LOCALSPACE)
-			object_to_mat4(ob, mat);
-		else
+		if (dtar->flag & DTAR_FLAG_LOCALSPACE) {
+			if (dtar->flag & DTAR_FLAG_LOCAL_CONSTS) {
+				/* just like how the constraints do it! */
+				copy_m4_m4(mat, ob->obmat);
+				constraint_mat_convertspace(ob, NULL, mat, CONSTRAINT_SPACE_WORLD, CONSTRAINT_SPACE_LOCAL);
+			}
+			else {
+				/* transforms to matrix */
+				object_to_mat4(ob, mat);
+			}
+		}
+		else {
+			/* worldspace matrix - just the good-old one */
 			copy_m4_m4(mat, ob->obmat);
+		}
 	}
 	
 	/* check which transform */
@@ -1255,9 +1283,21 @@ static float dvar_eval_transChan (ChannelDriver *driver, DriverVar *dvar)
 		return scale[dtar->transChan - DTAR_TRANSCHAN_SCALEX];
 	}
 	else if (dtar->transChan >= DTAR_TRANSCHAN_ROTX) {
-		/* extract euler rotation (if needed), and choose the right axis */
-		if ((dtar->flag & DTAR_FLAG_LOCALSPACE)==0 || (useEulers == 0))
-			mat4_to_eulO(eul, rotOrder, mat);
+		/* extract rotation as eulers (if needed) 
+		 *	- definitely if rotation order isn't eulers already
+		 *	- if eulers, then we have 2 options:
+		 *		a) decompose transform matrix as required, then try to make eulers from
+		 *		   there compatible with original values
+		 *		b) [NOT USED] directly use the original values (no decomposition) 
+		 *			- only an option for "transform space", if quality is really bad with a)
+		 */
+		float eul[3];
+		
+		mat4_to_eulO(eul, rotOrder, mat);
+		
+		if (useEulers) {
+			compatible_eul(eul, oldEul);
+		}
 		
 		return eul[dtar->transChan - DTAR_TRANSCHAN_ROTX];
 	}
diff --git a/source/blender/editors/space_graph/graph_buttons.c b/source/blender/editors/space_graph/graph_buttons.c
index fb1144b4fa8..e820fdbb642 100644
--- a/source/blender/editors/space_graph/graph_buttons.c
+++ b/source/blender/editors/space_graph/graph_buttons.c
@@ -493,7 +493,7 @@ static void graph_panel_driverVar__transChan(uiLayout *layout, ID *id, DriverVar
 	DriverTarget *dtar= &dvar->targets[0];
 	Object *ob = (Object *)dtar->id;
 	PointerRNA dtar_ptr;
-	uiLayout *col, *row;
+	uiLayout *col, *subcol;
 	
 	/* initialise RNA pointer to the target */
 	RNA_pointer_create(id, &RNA_DriverTarget, dtar, &dtar_ptr); 
@@ -509,9 +509,9 @@ static void graph_panel_driverVar__transChan(uiLayout *layout, ID *id, DriverVar
 			uiItemPointerR(col, &dtar_ptr, "bone_target", &tar_ptr, "bones", "", ICON_BONE_DATA);
 		}
 		
-		row= uiLayoutRow(layout, 1);
-			uiItemR(row, &dtar_ptr, "transform_type", 0, "", ICON_NONE);
-			uiItemR(row, &dtar_ptr, "use_local_space_transform", 0, NULL, ICON_NONE);
+		subcol= uiLayoutColumn(layout, 1);
+			uiItemR(subcol, &dtar_ptr, "transform_type", 0, NULL, ICON_NONE);
+			uiItemR(subcol, &dtar_ptr, "transform_space", 0, "Space", ICON_NONE);
 }
 
 /* driver settings for active F-Curve (only for 'Drivers' mode) */
diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h
index c650d15722d..00a9786fc6f 100644
--- a/source/blender/makesdna/DNA_anim_types.h
+++ b/source/blender/makesdna/DNA_anim_types.h
@@ -289,8 +289,12 @@ typedef enum eDriverTarget_Flag {
 	DTAR_FLAG_STRUCT_REF	= (1<<0),
 		/* idtype can only be 'Object' */
 	DTAR_FLAG_ID_OB_ONLY	= (1<<1),
-		/* toggles localspace (where transforms are manually obtained) */
+	
+	/* "localspace" flags */
+		/* base flag - basically "pre parent+constraints" */
 	DTAR_FLAG_LOCALSPACE	= (1<<2),
+		/* include constraints transformed to space including parents */
+	DTAR_FLAG_LOCAL_CONSTS	= (1<<3),
 } eDriverTarget_Flag;
 
 /* Transform Channels for Driver Targets */
diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c
index 9b8db639cbb..cc18d8d9d0d 100644
--- a/source/blender/makesrna/intern/rna_access.c
+++ b/source/blender/makesrna/intern/rna_access.c
@@ -3102,7 +3102,7 @@ static char *rna_path_token(const char **path, char *fixedbuf, int fixedlen, int
 		/* 2 kinds of lookups now, quoted or unquoted */
 		quote= *p;
 
-		if(quote != '"')
+		if(quote != '"') /* " - this comment is hack for Aligorith's text editor's sanity */
 			quote= 0;
 
 		if(quote==0) {
diff --git a/source/blender/makesrna/intern/rna_fcurve.c b/source/blender/makesrna/intern/rna_fcurve.c
index 263978221bb..ab3665fb8ff 100644
--- a/source/blender/makesrna/intern/rna_fcurve.c
+++ b/source/blender/makesrna/intern/rna_fcurve.c
@@ -1104,6 +1104,12 @@ static void rna_def_drivertarget(BlenderRNA *brna)
 		{DTAR_TRANSCHAN_SCALEY, "SCALE_Y", 0, "Y Scale", ""},
 		{DTAR_TRANSCHAN_SCALEZ, "SCALE_Z", 0, "Z Scale", ""},
 		{0, NULL, 0, NULL, NULL}};
+		
+	static EnumPropertyItem prop_local_space_items[] = {
+		{0, "WORLD_SPACE", 0, "World Space", "Transforms include effects of parenting/restpose and constraints"},
+		{DTAR_FLAG_LOCALSPACE, "TRANSFORM_SPACE", 0, "Transform Space", "Transforms don't include parenting/restpose or constraints"},
+		{DTAR_FLAG_LOCALSPACE|DTAR_FLAG_LOCAL_CONSTS, "LOCAL_SPACE", 0, "Local Space", "Transforms include effects of constraints but not parenting/restpose"},
+		{0, NULL, 0, NULL, NULL}};
 	
 	srna= RNA_def_struct(brna, "DriverTarget", NULL);
 	RNA_def_struct_ui_text(srna, "Driver Target", "Source of input values for driver variables");
@@ -1144,9 +1150,10 @@ static void rna_def_drivertarget(BlenderRNA *brna)
 	RNA_def_property_ui_text(prop, "Type", "Driver variable type");
 	RNA_def_property_update(prop, 0, "rna_DriverTarget_update_data");
 	
-	prop= RNA_def_property(srna, "use_local_space_transform", PROP_BOOLEAN, PROP_NONE);
-	RNA_def_property_boolean_sdna(prop, NULL, "flag", DTAR_FLAG_LOCALSPACE);
-	RNA_def_property_ui_text(prop, "Local Space", "Use transforms in Local Space (as opposed to the worldspace default)");
+	prop= RNA_def_property(srna, "transform_space", PROP_ENUM, PROP_NONE);
+	RNA_def_property_enum_bitflag_sdna(prop, NULL, "flag");
+	RNA_def_property_enum_items(prop, prop_local_space_items);
+	RNA_def_property_ui_text(prop, "Transform Space", "Space in which transforms are used");
 	RNA_def_property_update(prop, 0, "rna_DriverTarget_update_data");
 }