Bugfix #19576: Auto keyframing does not record rotations on object level animation

The hardcoded paths for rotation keyframes on objects got broken by my commits to rename the rotation properties. I've taken this opportunity to recode the auto-keyframing code here to use the builtin keyingsets instead of going through and manually calling insert_keyframe(), thus preventing this problem in future.

7 files changed, 72 insertions, 115 deletions

diff --git a/source/blender/editors/animation/keyframing.c b/source/blender/editors/animation/keyframing.c
index 10d45d53761..75218e6ea6e 100644
--- a/source/blender/editors/animation/keyframing.c
+++ b/source/blender/editors/animation/keyframing.c
@@ -981,12 +981,15 @@ static int insert_key_exec (bContext *C, wmOperator *op)
 	}
 	
 	/* try to insert keyframes for the channels specified by KeyingSet */
-	success= modify_keyframes(C, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
-	printf("KeyingSet '%s' - Successfully added %d Keyframes \n", ks->name, success);
+	success= modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
+	if (G.f & G_DEBUG)
+		printf("KeyingSet '%s' - Successfully added %d Keyframes \n", ks->name, success);
 	
 	/* report failure? */
 	if (success == 0)
 		BKE_report(op->reports, RPT_WARNING, "Keying Set failed to insert any keyframes");
+	else
+		WM_event_add_notifier(C, NC_ANIMATION|ND_KEYFRAME_EDIT, NULL);
 	
 	/* free temp context-data if available */
 	if (dsources.first) {
@@ -995,10 +998,7 @@ static int insert_key_exec (bContext *C, wmOperator *op)
 	}
 	
 	/* send updates */
-	ED_anim_dag_flush_update(C);	
-	
-	/* for now, only send ND_KEYS for KeyingSets */
-	WM_event_add_notifier(C, ND_KEYS, NULL);
+	ED_anim_dag_flush_update(C);
 	
 	return OPERATOR_FINISHED;
 }
@@ -1132,12 +1132,15 @@ static int delete_key_exec (bContext *C, wmOperator *op)
 	}
 	
 	/* try to insert keyframes for the channels specified by KeyingSet */
-	success= modify_keyframes(C, &dsources, NULL, ks, MODIFYKEY_MODE_DELETE, cfra);
-	printf("KeyingSet '%s' - Successfully removed %d Keyframes \n", ks->name, success);
+	success= modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_DELETE, cfra);
+	if (G.f & G_DEBUG)
+		printf("KeyingSet '%s' - Successfully removed %d Keyframes \n", ks->name, success);
 	
 	/* report failure? */
 	if (success == 0)
 		BKE_report(op->reports, RPT_WARNING, "Keying Set failed to remove any keyframes");
+	else
+		WM_event_add_notifier(C, NC_ANIMATION|ND_KEYFRAME_EDIT, NULL);
 	
 	/* free temp context-data if available */
 	if (dsources.first) {
@@ -1148,9 +1151,6 @@ static int delete_key_exec (bContext *C, wmOperator *op)
 	/* send updates */
 	ED_anim_dag_flush_update(C);	
 	
-	/* for now, only send ND_KEYS for KeyingSets */
-	WM_event_add_notifier(C, ND_KEYS, NULL);
-	
 	return OPERATOR_FINISHED;
 }
 
diff --git a/source/blender/editors/animation/keyingsets.c b/source/blender/editors/animation/keyingsets.c
index 99cdf0a86f4..a9a3612b79b 100644
--- a/source/blender/editors/animation/keyingsets.c
+++ b/source/blender/editors/animation/keyingsets.c
@@ -1252,9 +1252,8 @@ short modifykey_get_context_data (bContext *C, ListBase *dsources, KeyingSet *ks
  * by the KeyingSet. This takes into account many of the different combinations of using KeyingSets.
  * Returns the number of channels that keyframes were added to
  */
-int modify_keyframes (bContext *C, ListBase *dsources, bAction *act, KeyingSet *ks, short mode, float cfra)
+int modify_keyframes (Scene *scene, ListBase *dsources, bAction *act, KeyingSet *ks, short mode, float cfra)
 {
-	Scene *scene= CTX_data_scene(C);
 	KS_Path *ksp;
 	int kflag=0, success= 0;
 	char *groupname= NULL;
@@ -1319,24 +1318,16 @@ int modify_keyframes (bContext *C, ListBase *dsources, bAction *act, KeyingSet *
 					success+= delete_keyframe(ksp->id, act, groupname, ksp->rna_path, i, cfra, kflag);
 			}
 			
-			/* send notifiers and set recalc-flags */
-			// TODO: hopefully this doesn't result in execessive flooding of the notifier stack
-			if (C && ksp->id) {
+			/* set recalc-flags */
+			if (ksp->id) {
 				switch (GS(ksp->id->name)) {
 					case ID_OB: /* Object (or Object-Related) Keyframes */
 					{
 						Object *ob= (Object *)ksp->id;
 						
 						ob->recalc |= OB_RECALC;
-						WM_event_add_notifier(C, NC_OBJECT|ND_KEYS, ksp->id);
 					}
 						break;
-					case ID_MA: /* Material Keyframes */
-						WM_event_add_notifier(C, NC_MATERIAL|ND_KEYS, ksp->id);
-						break;
-					default: /* Any keyframes */
-						WM_event_add_notifier(C, NC_ANIMATION|ND_KEYFRAME_EDIT, NULL);
-						break;
 				}
 			}
 		}
@@ -1457,24 +1448,16 @@ int modify_keyframes (bContext *C, ListBase *dsources, bAction *act, KeyingSet *
 				MEM_freeN(path);
 			}
 			
-			/* send notifiers and set recalc-flags */
-			// TODO: hopefully this doesn't result in execessive flooding of the notifier stack
-			if (C && cks->id) {
+			/* set recalc-flags */
+			if (cks->id) {
 				switch (GS(cks->id->name)) {
 					case ID_OB: /* Object (or Object-Related) Keyframes */
 					{
 						Object *ob= (Object *)cks->id;
 						
 						ob->recalc |= OB_RECALC;
-						WM_event_add_notifier(C, NC_OBJECT|ND_KEYS, cks->id);
 					}
 						break;
-					case ID_MA: /* Material Keyframes */
-						WM_event_add_notifier(C, NC_MATERIAL|ND_KEYS, cks->id);
-						break;
-					default: /* Any keyframes */
-						WM_event_add_notifier(C, NC_ANIMATION|ND_KEYFRAME_EDIT, NULL);
-						break;
 				}
 			}
 		}
diff --git a/source/blender/editors/armature/poseSlide.c b/source/blender/editors/armature/poseSlide.c
index c73208c54c2..1e0df79d0e6 100644
--- a/source/blender/editors/armature/poseSlide.c
+++ b/source/blender/editors/armature/poseSlide.c
@@ -560,11 +560,11 @@ static void pose_slide_autoKeyframe (bContext *C, tPoseSlideOp *pso)
 			
 			/* insert keyframes */
 			if (pchan->flag & POSE_LOC)
-				modify_keyframes(C, &dsources, NULL, pso->ks_loc, MODIFYKEY_MODE_INSERT, (float)pso->cframe);
+				modify_keyframes(pso->scene, &dsources, NULL, pso->ks_loc, MODIFYKEY_MODE_INSERT, (float)pso->cframe);
 			if (pchan->flag & POSE_ROT)
-				modify_keyframes(C, &dsources, NULL, pso->ks_rot, MODIFYKEY_MODE_INSERT, (float)pso->cframe);
+				modify_keyframes(pso->scene, &dsources, NULL, pso->ks_rot, MODIFYKEY_MODE_INSERT, (float)pso->cframe);
 			if (pchan->flag & POSE_SIZE)
-				modify_keyframes(C, &dsources, NULL, pso->ks_scale, MODIFYKEY_MODE_INSERT, (float)pso->cframe);
+				modify_keyframes(pso->scene, &dsources, NULL, pso->ks_scale, MODIFYKEY_MODE_INSERT, (float)pso->cframe);
 		}
 	}
 }
diff --git a/source/blender/editors/armature/poselib.c b/source/blender/editors/armature/poselib.c
index 386cb6512a3..d34da201ef5 100644
--- a/source/blender/editors/armature/poselib.c
+++ b/source/blender/editors/armature/poselib.c
@@ -329,6 +329,7 @@ static int poselib_add_menu_invoke (bContext *C, wmOperator *op, wmEvent *evt)
 
 static int poselib_add_exec (bContext *C, wmOperator *op)
 {
+	Scene *scene= CTX_data_scene(C);
 	Object *ob= CTX_data_active_object(C);
 	bAction *act = poselib_validate(ob);
 	bArmature *arm= (ob) ? ob->data : NULL;
@@ -385,7 +386,7 @@ static int poselib_add_exec (bContext *C, wmOperator *op)
 				/* KeyingSet to use depends on rotation mode (but that's handled by the templates code)  */
 				if (poselib_ks_locrotscale == NULL)
 					poselib_ks_locrotscale= ANIM_builtin_keyingset_get_named(NULL, "LocRotScale");
-				modify_keyframes(C, &dsources, act, poselib_ks_locrotscale, MODIFYKEY_MODE_INSERT, (float)frame);
+				modify_keyframes(scene, &dsources, act, poselib_ks_locrotscale, MODIFYKEY_MODE_INSERT, (float)frame);
 			}
 		}
 	}
@@ -772,7 +773,7 @@ static void poselib_keytag_pose (bContext *C, Scene *scene, tPoseLib_PreviewData
 					cks.pchan= pchan;
 					
 					/* now insert the keyframe */
-					modify_keyframes(C, &dsources, NULL, poselib_ks_locrotscale, MODIFYKEY_MODE_INSERT, (float)CFRA);
+					modify_keyframes(scene, &dsources, NULL, poselib_ks_locrotscale, MODIFYKEY_MODE_INSERT, (float)CFRA);
 					
 					/* clear any unkeyed tags */
 					if (pchan->bone)
@@ -783,10 +784,12 @@ static void poselib_keytag_pose (bContext *C, Scene *scene, tPoseLib_PreviewData
 					if (pchan->bone)
 						pchan->bone->flag |= BONE_UNKEYED;
 				}
-		
 			}
 		}
 	}
+	
+	/* send notifiers for this */
+	WM_event_add_notifier(C, NC_ANIMATION|ND_KEYFRAME_EDIT, NULL);
 }
 
 /* Apply the relevant changes to the pose */
diff --git a/source/blender/editors/armature/poseobject.c b/source/blender/editors/armature/poseobject.c
index 461a70bbf86..1531d922e04 100644
--- a/source/blender/editors/armature/poseobject.c
+++ b/source/blender/editors/armature/poseobject.c
@@ -1089,7 +1089,7 @@ static int pose_paste_exec (bContext *C, wmOperator *op)
 					/* init cks for this PoseChannel, then use the relative KeyingSets to keyframe it */
 					cks.pchan= pchan;
 					
-					modify_keyframes(C, &dsources, NULL, posePaste_ks_locrotscale, MODIFYKEY_MODE_INSERT, (float)CFRA);
+					modify_keyframes(scene, &dsources, NULL, posePaste_ks_locrotscale, MODIFYKEY_MODE_INSERT, (float)CFRA);
 					
 					/* clear any unkeyed tags */
 					if (chan->bone)
@@ -1118,6 +1118,7 @@ static int pose_paste_exec (bContext *C, wmOperator *op)
 	
 	/* notifiers for updates */
 	WM_event_add_notifier(C, NC_OBJECT|ND_POSE|ND_TRANSFORM, ob);
+	WM_event_add_notifier(C, NC_ANIMATION|ND_KEYFRAME_EDIT, NULL); // XXX not really needed, but here for completeness...
 
 	return OPERATOR_FINISHED;
 }
diff --git a/source/blender/editors/include/ED_keyframing.h b/source/blender/editors/include/ED_keyframing.h
index 92dda4162cc..2b41cb5c9db 100644
--- a/source/blender/editors/include/ED_keyframing.h
+++ b/source/blender/editors/include/ED_keyframing.h
@@ -122,7 +122,7 @@ enum {
 } eModifyKey_Modes;
 
 /* Keyframing Helper Call - use the provided Keying Set to Add/Remove Keyframes */
-int modify_keyframes(struct bContext *C, struct ListBase *dsources, struct bAction *act, struct KeyingSet *ks, short mode, float cfra);
+int modify_keyframes(struct Scene *scene, struct ListBase *dsources, struct bAction *act, struct KeyingSet *ks, short mode, float cfra);
 
 /* -------- */
 
diff --git a/source/blender/editors/transform/transform_conversions.c b/source/blender/editors/transform/transform_conversions.c
index 1b11d43c200..d2c1c9f55f0 100644
--- a/source/blender/editors/transform/transform_conversions.c
+++ b/source/blender/editors/transform/transform_conversions.c
@@ -4242,10 +4242,15 @@ void autokeyframe_ob_cb_func(Scene *scene, View3D *v3d, Object *ob, int tmode)
 
 	// TODO: this should probably be done per channel instead...
 	if (autokeyframe_cfra_can_key(scene, id)) {
-		AnimData *adt= ob->adt;
+		bCommonKeySrc cks;
+		ListBase dsources = {&cks, &cks};
 		float cfra= (float)CFRA; // xxx this will do for now
 		short flag = 0;
 		
+		/* init common-key-source for use by KeyingSets */
+		memset(&cks, 0, sizeof(bCommonKeySrc));
+		cks.id= &ob->id;
+		
 		if (IS_AUTOKEY_FLAG(INSERTNEEDED))
 			flag |= INSERTKEY_NEEDED;
 		if (IS_AUTOKEY_FLAG(AUTOMATKEY))
@@ -4254,6 +4259,8 @@ void autokeyframe_ob_cb_func(Scene *scene, View3D *v3d, Object *ob, int tmode)
 			flag |= INSERTKEY_REPLACE;
 			
 		if (IS_AUTOKEY_FLAG(INSERTAVAIL)) {
+			AnimData *adt= ob->adt;
+			
 			/* only key on available channels */
 			if (adt && adt->action) {
 				for (fcu= adt->action->curves.first; fcu; fcu= fcu->next) {
@@ -4292,41 +4299,25 @@ void autokeyframe_ob_cb_func(Scene *scene, View3D *v3d, Object *ob, int tmode)
 					doScale = 1;
 			}
 			
-			// TODO: the group names here are temporary...
-			// TODO: should this be made to use the builtin KeyingSets instead?
+			/* insert keyframes for the affected sets of channels using the builtin KeyingSets found */
 			if (doLoc) {
-				insert_keyframe(id, NULL, "Object Transform", "location", 0, cfra, flag);
-				insert_keyframe(id, NULL, "Object Transform", "location", 1, cfra, flag);
-				insert_keyframe(id, NULL, "Object Transform", "location", 2, cfra, flag);
+				KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "Location");
+				modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 			}
 			if (doRot) {
-				insert_keyframe(id, NULL, "Object Transform", "rotation", 0, cfra, flag);
-				insert_keyframe(id, NULL, "Object Transform", "rotation", 1, cfra, flag);
-				insert_keyframe(id, NULL, "Object Transform", "rotation", 2, cfra, flag);
+				KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "Rotation");
+				modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 			}
 			if (doScale) {
-				insert_keyframe(id, NULL, "Object Transform", "scale", 0, cfra, flag);
-				insert_keyframe(id, NULL, "Object Transform", "scale", 1, cfra, flag);
-				insert_keyframe(id, NULL, "Object Transform", "scale", 2, cfra, flag);
+				KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "Scale");
+				modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 			}
 		}
+		/* insert keyframe in all (transform) channels */
 		else {
-			// TODO: the group names here are temporary...
-			// TODO: should this be made to use the builtin KeyingSets instead?
-			insert_keyframe(id, NULL, "Object Transform", "location", 0, cfra, flag);
-			insert_keyframe(id, NULL, "Object Transform", "location", 1, cfra, flag);
-			insert_keyframe(id, NULL, "Object Transform", "location", 2, cfra, flag);
-			
-			insert_keyframe(id, NULL, "Object Transform", "rotation", 0, cfra, flag);
-			insert_keyframe(id, NULL, "Object Transform", "rotation", 1, cfra, flag);
-			insert_keyframe(id, NULL, "Object Transform", "rotation", 2, cfra, flag);
-			
-			insert_keyframe(id, NULL, "Object Transform", "scale", 0, cfra, flag);
-			insert_keyframe(id, NULL, "Object Transform", "scale", 1, cfra, flag);
-			insert_keyframe(id, NULL, "Object Transform", "scale", 2, cfra, flag);
+			KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "LocRotScale");
+			modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 		}
-			
-		// XXX todo... find a way to send notifiers from here...
 	}
 }
 
@@ -4346,9 +4337,14 @@ void autokeyframe_pose_cb_func(Scene *scene, View3D *v3d, Object *ob, int tmode,
 
 	// TODO: this should probably be done per channel instead...
 	if (autokeyframe_cfra_can_key(scene, id)) {
+		bCommonKeySrc cks;
+		ListBase dsources = {&cks, &cks};
 		float cfra= (float)CFRA;
 		short flag= 0;
-		char buf[512];
+		
+		/* init common-key-source for use by KeyingSets */
+		memset(&cks, 0, sizeof(bCommonKeySrc));
+		cks.id= &ob->id;
 		
 		/* flag is initialised from UserPref keyframing settings
 		 *	- special exception for targetless IK - INSERTKEY_MATRIX keyframes should get
@@ -4401,60 +4397,34 @@ void autokeyframe_pose_cb_func(Scene *scene, View3D *v3d, Object *ob, int tmode,
 					}
 					
 					if (doLoc) {
-						sprintf(buf, "pose.pose_channels[\"%s\"].location", pchan->name);
-						insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
+						KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "Location");
+						
+						/* init cks for this PoseChannel, then use the relative KeyingSets to keyframe it */
+						cks.pchan= pchan;
+						modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 					}
 					if (doRot) {
-						// FIXME: better to just use the keyingsets for this instead...
-						if (pchan->rotmode == ROT_MODE_QUAT) {
-							sprintf(buf, "pose.pose_channels[\"%s\"].rotation_quaternion", pchan->name);
-							insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-							insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-							insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
-							insert_keyframe(id, NULL, pchan->name, buf, 3, cfra, flag);
-						}
-						else {
-							sprintf(buf, "pose.pose_channels[\"%s\"].rotation_euler", pchan->name);
-							insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-							insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-							insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
-						}
+						KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "Rotation");
+						
+						/* init cks for this PoseChannel, then use the relative KeyingSets to keyframe it */
+						cks.pchan= pchan;
+						modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 					}
 					if (doScale) {
-						sprintf(buf, "pose.pose_channels[\"%s\"].scale", pchan->name);
-						insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
+						KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "Scale");
+						
+						/* init cks for this PoseChannel, then use the relative KeyingSets to keyframe it */
+						cks.pchan= pchan;
+						modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 					}
 				}
-				/* insert keyframe in any channel that's appropriate */
+				/* insert keyframe in all (transform) channels */
 				else {
-					sprintf(buf, "pose.pose_channels[\"%s\"].location", pchan->name);
-					insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-					insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-					insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
-					
-					// FIXME: better to just use the keyingsets for this instead...
-					if (pchan->rotmode == ROT_MODE_QUAT) {
-						sprintf(buf, "pose.pose_channels[\"%s\"].rotation", pchan->name);
-						insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 3, cfra, flag);
-					}
-					else {
-						sprintf(buf, "pose.pose_channels[\"%s\"].rotation_euler", pchan->name);
-						insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-						insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
-					}
+					KeyingSet *ks= ANIM_builtin_keyingset_get_named(NULL, "LocRotScale");
 					
-					sprintf(buf, "pose.pose_channels[\"%s\"].scale", pchan->name);
-					insert_keyframe(id, NULL, pchan->name, buf, 0, cfra, flag);
-					insert_keyframe(id, NULL, pchan->name, buf, 1, cfra, flag);
-					insert_keyframe(id, NULL, pchan->name, buf, 2, cfra, flag);
+					/* init cks for this PoseChannel, then use the relative KeyingSets to keyframe it */
+					cks.pchan= pchan;
+					modify_keyframes(scene, &dsources, NULL, ks, MODIFYKEY_MODE_INSERT, cfra);
 				}
 			}
 		}