Bugfixes: Deleting Keyframes + F-Curves

This commit fixes #19908 and #20239. Deleting keyframes will now delete the F-Curves they came from too, if the F-Curves don't have any more keyframes and/or F-Modifiers providing any further motion info. 

6 files changed, 64 insertions, 42 deletions

diff --git a/source/blender/editors/animation/anim_channels_edit.c b/source/blender/editors/animation/anim_channels_edit.c
index 5842f476bc6..4be9cb2cd9d 100644
--- a/source/blender/editors/animation/anim_channels_edit.c
+++ b/source/blender/editors/animation/anim_channels_edit.c
@@ -86,9 +86,9 @@
 #include "WM_types.h"
 
 /* ************************************************************************** */
-/* CHANNELS API */
+/* CHANNELS API - Exposed API */
 
-/* -------------------------- Exposed API ----------------------------------- */
+/* -------------------------- Selection ------------------------------------- */
 
 /* Set the given animation-channel as the active one for the active context */
 // TODO: extend for animdata types...
@@ -352,6 +352,8 @@ void ANIM_deselect_anim_channels (void *data, short datatype, short test, short
 	BLI_freelistN(&anim_data);
 }
 
+/* ---------------------------- Graph Editor ------------------------------------- */
+
 /* Flush visibility (for Graph Editor) changes up/down hierarchy for changes in the given setting 
  *	- anim_data: list of the all the anim channels that can be chosen
  *		-> filtered using ANIMFILTER_CHANNELS only, since if we took VISIBLE too,
@@ -452,6 +454,35 @@ void ANIM_visibility_flush_anim_channels (bAnimContext *ac, ListBase *anim_data,
 	}
 }
 
+/* -------------------------- F-Curves ------------------------------------- */
+
+/* Delete the given F-Curve from its AnimData block */
+void ANIM_fcurve_delete_from_animdata (bAnimContext *ac, AnimData *adt, FCurve *fcu)
+{
+	/* - if no AnimData, we've got nowhere to remove the F-Curve from 
+	 *	(this doesn't guarantee that the F-Curve is in there, but at least we tried
+	 * - if no F-Curve, there is nothing to remove
+	 */
+	if (ELEM(NULL, adt, fcu))
+		return;
+		
+	/* remove from whatever list it came from
+	 *	- Action Group
+	 *	- Action
+	 *	- Drivers
+	 *	- TODO... some others?
+	 */
+	if (fcu->grp)
+		action_groups_remove_channel(adt->action, fcu);
+	else if ((ac) && (ac->datatype == ANIMCONT_DRIVERS))
+		BLI_remlink(&adt->drivers, fcu);
+	else if (adt->action)
+		BLI_remlink(&adt->action->curves, fcu);
+		
+	/* free the F-Curve itself */
+	free_fcurve(fcu);
+}
+
 /* ************************************************************************** */
 /* OPERATORS */
 
@@ -945,28 +976,8 @@ static int animchannels_delete_exec(bContext *C, wmOperator *op)
 				AnimData *adt= ale->adt;
 				FCurve *fcu= (FCurve *)ale->data;
 				
-				/* if no AnimData, we've got nowhere to remove the F-Curve from */
-				if (adt == NULL)
-					continue;
-					
-				/* remove from whatever list it came from
-				 *	- Action Group
-				 *	- Action
-				 *	- Drivers
-				 *	- TODO... some others?
-				 *
-				 *	note: this isn't well tested, we could also try remove
-				 *	      from all lists just to be safe - campbell
-				 */
-				if (fcu->grp)
-					action_groups_remove_channel(adt->action, fcu);
-				else if (ac.datatype == ANIMCONT_DRIVERS)
-					BLI_remlink(&adt->drivers, fcu);
-				else if (adt->action)
-					BLI_remlink(&adt->action->curves, fcu);
-					
-				/* free the F-Curve itself */
-				free_fcurve(fcu);
+				/* try to free F-Curve */
+				ANIM_fcurve_delete_from_animdata(&ac, adt, fcu);
 			}
 		}
 		
diff --git a/source/blender/editors/animation/keyframes_general.c b/source/blender/editors/animation/keyframes_general.c
index 9d3fc7a0fc4..3b717bafc70 100644
--- a/source/blender/editors/animation/keyframes_general.c
+++ b/source/blender/editors/animation/keyframes_general.c
@@ -110,14 +110,6 @@ void delete_fcurve_keys(FCurve *fcu)
 			MEM_freeN(fcu->bezt);
 		fcu->bezt= NULL;
 	}
-	
-#if 0 // XXX for now, we don't get rid of empty curves...
-	/* Only delete if there isn't an ipo-driver still hanging around on an empty curve */
-	if ((icu->totvert==0) && (icu->driver==NULL)) {
-		BLI_remlink(&ipo->curve, icu);
-		free_ipo_curve(icu);
-	}
-#endif 
 }
 
 /* ---------------- */
diff --git a/source/blender/editors/animation/keyframing.c b/source/blender/editors/animation/keyframing.c
index 8628e2c506a..3bfb8bdc867 100644
--- a/source/blender/editors/animation/keyframing.c
+++ b/source/blender/editors/animation/keyframing.c
@@ -863,6 +863,7 @@ short insert_keyframe (ID *id, bAction *act, const char group[], const char rna_
  */
 short delete_keyframe (ID *id, bAction *act, const char group[], const char rna_path[], int array_index, float cfra, short flag)
 {
+	AnimData *adt= BKE_animdata_from_id(id);
 	FCurve *fcu = NULL;
 	
 	/* get F-Curve
@@ -871,7 +872,6 @@ short delete_keyframe (ID *id, bAction *act, const char group[], const char rna_
 	 */
 	if (act == NULL) {
 		/* if no action is provided, use the default one attached to this ID-block */
-		AnimData *adt= BKE_animdata_from_id(id);
 		act= adt->action;
 		
 		/* apply NLA-mapping to frame to use (if applicable) */
@@ -913,11 +913,9 @@ short delete_keyframe (ID *id, bAction *act, const char group[], const char rna_
 			/* delete the key at the index (will sanity check + do recalc afterwards) */
 			delete_fcurve_key(fcu, i, 1);
 			
-			/* Only delete curve too if there are no points (we don't need to check for drivers, as they're kept separate) */
-			if (fcu->totvert == 0) {
-				BLI_remlink(&act->curves, fcu);
-				free_fcurve(fcu);
-			}
+			/* Only delete curve too if it won't be doing anything anymore */
+			if ((fcu->totvert == 0) && (list_has_suitable_fmodifier(&fcu->modifiers, 0, FMI_TYPE_GENERATE_CURVE) == 0))
+				ANIM_fcurve_delete_from_animdata(NULL, adt, fcu);
 			
 			/* return success */
 			return 1;
diff --git a/source/blender/editors/include/ED_anim_api.h b/source/blender/editors/include/ED_anim_api.h
index 8a73e12f39b..c9c4c7af18c 100644
--- a/source/blender/editors/include/ED_anim_api.h
+++ b/source/blender/editors/include/ED_anim_api.h
@@ -394,6 +394,10 @@ void ANIM_deselect_anim_channels(void *data, short datatype, short test, short s
 /* Set the 'active' channel of type channel_type, in the given action */
 void ANIM_set_active_channel(bAnimContext *ac, void *data, short datatype, int filter, void *channel_data, short channel_type);
 
+
+/* Delete the F-Curve from the given AnimData block (if possible), as appropriate according to animation context */
+void ANIM_fcurve_delete_from_animdata(bAnimContext *ac, struct AnimData *adt, struct FCurve *fcu);
+
 /* ************************************************ */
 /* DRAWING API */
 /* anim_draw.c */
diff --git a/source/blender/editors/space_action/action_edit.c b/source/blender/editors/space_action/action_edit.c
index 1ba837afba3..22f5f1757c9 100644
--- a/source/blender/editors/space_action/action_edit.c
+++ b/source/blender/editors/space_action/action_edit.c
@@ -613,10 +613,19 @@ static void delete_action_keys (bAnimContext *ac)
 	
 	/* loop through filtered data and delete selected keys */
 	for (ale= anim_data.first; ale; ale= ale->next) {
-		//if (ale->type == ANIMTYPE_GPLAYER)
-		//	delete_gplayer_frames((bGPDlayer *)ale->data);
+		if (ale->type != ANIMTYPE_GPLAYER) {
+			FCurve *fcu= (FCurve *)ale->key_data;
+			AnimData *adt= ale->adt;
+			
+			/* delete selected keyframes only */
+			delete_fcurve_keys(fcu); 
+			
+			/* Only delete curve too if it won't be doing anything anymore */
+			if ((fcu->totvert == 0) && (list_has_suitable_fmodifier(&fcu->modifiers, 0, FMI_TYPE_GENERATE_CURVE) == 0))
+				ANIM_fcurve_delete_from_animdata(ac, ale->adt, fcu);
+		}
 		//else
-			delete_fcurve_keys((FCurve *)ale->key_data); // XXX... this doesn't delete empty curves anymore
+		//	delete_gplayer_frames((bGPDlayer *)ale->data);
 	}
 	
 	/* free filtered list */
diff --git a/source/blender/editors/space_graph/graph_edit.c b/source/blender/editors/space_graph/graph_edit.c
index 381d5becb1e..f20ebcc67b9 100644
--- a/source/blender/editors/space_graph/graph_edit.c
+++ b/source/blender/editors/space_graph/graph_edit.c
@@ -792,7 +792,15 @@ static void delete_graph_keys (bAnimContext *ac)
 	
 	/* loop through filtered data and delete selected keys */
 	for (ale= anim_data.first; ale; ale= ale->next) {
-		delete_fcurve_keys((FCurve *)ale->key_data); // XXX... this doesn't delete empty curves anymore
+		FCurve *fcu= (FCurve *)ale->key_data;
+		AnimData *adt= ale->adt;
+		
+		/* delete selected keyframes only */
+		delete_fcurve_keys(fcu); 
+		
+		/* Only delete curve too if it won't be doing anything anymore */
+		if ((fcu->totvert == 0) && (list_has_suitable_fmodifier(&fcu->modifiers, 0, FMI_TYPE_GENERATE_CURVE) == 0))
+			ANIM_fcurve_delete_from_animdata(ac, ale->adt, fcu);
 	}
 	
 	/* free filtered list */