== Action Editor - Overlapping Keyframes Bugfix ==

Now when moving keyframes in the Action Editor, any existing keyframes on the frames where a selected keyframe lands (after the transform) will be removed. This is to prevent stacks of keyframes which cause blips and headaches for animators (especially stressed animators with a looming deadline).

I've added an option to the Action Editor's View menu to turn this behaviour on/off (by default, it's on). This shouldn't need to be used too much, and may be removed in due course. If it stays, it'll need a better name...

source/blender/include/BSE_editipo.h

@@ -157,7 +157,7 @@ void mirror_ipo_keys(struct Ipo *ipo, short mirror_mode);
 void setipotype_ipo(struct Ipo *ipo, int code);
 void set_ipo_key_selection(struct Ipo *ipo, int sel);
 int is_ipo_key_selected(struct Ipo *ipo);
-void delete_icu_key(struct IpoCurve *icu, int index);
+void delete_icu_key(struct IpoCurve *icu, int index, short do_recalc);
 void delete_ipo_keys(struct Ipo *ipo);
 int fullselect_ipo_keys(struct Ipo *ipo);
 int add_trans_ipo_keys(struct Ipo *ipo, struct TransVert *tv, int tvtot);

source/blender/makesdna/DNA_action_types.h

@@ -219,7 +219,9 @@ typedef enum SACTION_FLAG {
 		/* draw time in seconds instead of time in frames */
 	SACTION_DRAWTIME = (1<<2),
 		/* don't filter action channels according to visibility */
-	SACTION_NOHIDE = (1<<3)
+	SACTION_NOHIDE = (1<<3),
+		/* don't kill overlapping keyframes after transform */
+	SACTION_NOTRANSKEYCULL = (1<<4)
 } SACTION_FLAG;	
 
 /* SpaceAction AutoSnap Settings (also used by SpaceNLA) */

source/blender/src/editipo.c

@@ -2624,10 +2624,10 @@ void insertkey_smarter(ID *id, int blocktype, char *actname, char *constname, in
 			/* delete keyframe immediately before/after newly added */
 			switch (insert_mode) {
 				case KEYNEEDED_DELPREV:
-					delete_icu_key(icu, icu->totvert-2);
+					delete_icu_key(icu, icu->totvert-2, 1);
 					break;
 				case KEYNEEDED_DELNEXT:
-					delete_icu_key(icu, 1);
+					delete_icu_key(icu, 1, 1);
 					break;
 			}
 		}
@@ -5642,21 +5642,24 @@ void remake_object_ipos(Object *ob)
 
 /* Only delete the nominated keyframe from provided ipo-curve. 
  * Not recommended to be used many times successively. For that
- * there is delete_ipo_keys(). */
+ * there is delete_ipo_keys(). 
-void delete_icu_key(IpoCurve *icu, int index)
+ */
+void delete_icu_key(IpoCurve *icu, int index, short do_recalc)
 {
 	/* firstly check that index is valid */
 	if (index < 0) 
 		index *= -1;
+	if (icu == NULL) 
+		return;
 	if (index >= icu->totvert)
 		return;
-	if (!icu) return;
 	
 	/*	Delete this key */
 	memcpy(&icu->bezt[index], &icu->bezt[index+1], sizeof(BezTriple)*(icu->totvert-index-1));
 	icu->totvert--;
 	
-	/* recalc handles */
+	/* recalc handles - only if it won't cause problems */
+	if (do_recalc)
 		calchandles_ipocurve(icu);
 }
 

source/blender/src/header_action.c

@@ -98,7 +98,8 @@ enum {
 	ACTMENU_VIEW_NOHIDE,
 	ACTMENU_VIEW_OPENLEVELS,
 	ACTMENU_VIEW_CLOSELEVELS,
-	ACTMENU_VIEW_EXPANDALL
+	ACTMENU_VIEW_EXPANDALL,
+	ACTMENU_VIEW_TRANSDELDUPS
 };
 
 enum {
@@ -333,6 +334,9 @@ static void do_action_viewmenu(void *arg, int event)
 		case ACTMENU_VIEW_EXPANDALL: /* Expands all channels */
 			expand_all_action();
 			break;
+		case ACTMENU_VIEW_TRANSDELDUPS: /* Don't delete duplicate/overlapping keyframes after transform */
+			G.saction->flag ^= SACTION_NOTRANSKEYCULL;
+			break;
 	}
 	allqueue(REDRAWVIEW3D, 0);
 }
@@ -379,6 +383,13 @@ static uiBlock *action_viewmenu(void *arg_unused)
 					 menuwidth, 19, NULL, 0.0, 0.0, 1, 
 					 ACTMENU_VIEW_NOHIDE, "");
 			
+		// this option may get removed in future... 
+	uiDefIconTextBut(block, BUTM, 1, (G.saction->flag & SACTION_NOTRANSKEYCULL)?ICON_CHECKBOX_DEHLT:ICON_CHECKBOX_HLT, 
+					 "AfterTrans Delete Dupli-Frames|", 0, yco-=20, 
+					 menuwidth, 19, NULL, 0.0, 0.0, 1, 
+					 ACTMENU_VIEW_TRANSDELDUPS, "");
+			
+		
 	uiDefIconTextBut(block, BUTM, 1, (G.v2d->flag & V2D_VIEWLOCK)?ICON_CHECKBOX_HLT:ICON_CHECKBOX_DEHLT, 
 					 "Lock Time to Other Windows|", 0, yco-=20, 
 					 menuwidth, 19, NULL, 0.0, 0.0, 1, 

source/blender/src/poselib.c

@@ -455,7 +455,7 @@ void poselib_remove_pose (Object *ob, TimeMarker *marker)
 			for (i=0, bezt=icu->bezt; i < icu->totvert; i++, bezt++) {
 				/* check if remove... */
 				if (IS_EQ(bezt->vec[1][0], marker->frame)) {
-					delete_icu_key(icu, i);
+					delete_icu_key(icu, i, 1);
 					break;
 				}
 			}	

source/blender/src/transform_conversions.c

@@ -2346,6 +2346,102 @@ void flushTransIpoData(TransInfo *t)
 
 /* ********************* ACTION/NLA EDITOR ****************** */
 
+/* Called by special_aftertrans_update to make sure selected keyframes replace
+ * any other keyframes which may reside on that frame (that is not selected).
+ */
+static void posttrans_ipo_clean (Ipo *ipo)
+{
+	IpoCurve *icu;
+	int i;
+	
+	/* delete any keyframes that occur on same frame as selected keyframe, but is not selected */
+	for (icu= ipo->curve.first; icu; icu= icu->next) {
+		float *selcache;	/* cache for frame numbers of selected frames (icu->totvert*sizeof(float)) */
+		int len, index;		/* number of frames in cache, item index */
+		
+		/* allocate memory for the cache */
+		// TODO: investigate using GHash for this instead?
+		if (icu->totvert == 0) 
+			continue;
+		selcache= MEM_callocN(sizeof(float)*icu->totvert, "IcuSelFrameNums");
+		len= 0;
+		index= 0;
+		
+		/* We do 2 loops, 1 for marking keyframes for deletion, one for deleting 
+		 * as there is no guarantee what order the keyframes are exactly, even though 
+		 * they have been sorted by time.
+		 */
+		 
+		/*	Loop 1: find selected keyframes   */
+		for (i = 0; i < icu->totvert; i++) {
+			BezTriple *bezt= &icu->bezt[i];
+			
+			if (BEZSELECTED(bezt)) {
+				selcache[index]= bezt->vec[1][0];
+				index++;
+				len++;
+			}
+		}
+		
+		/* Loop 2: delete unselected keyframes on the same frames (if any keyframes were found) */
+		if (len) {
+			for (i = 0; i < icu->totvert; i++) {
+				BezTriple *bezt= &icu->bezt[i];
+				
+				if (BEZSELECTED(bezt) == 0) {
+					/* check beztriple should be removed according to cache */
+					for (index= 0; index < len; index++) {
+						if (IS_EQ(bezt->vec[1][0], selcache[index])) {
+							delete_icu_key(icu, i, 0);
+							break;
+						}
+						else if (bezt->vec[1][0] > selcache[index])
+							break;
+					}
+				}
+			}
+			
+			testhandles_ipocurve(icu);
+		}
+		
+		/* free cache */
+		MEM_freeN(selcache);
+	}
+}
+
+/* Called by special_aftertrans_update to make sure selected keyframes replace
+ * any other keyframes which may reside on that frame (that is not selected).
+ * remake_action_ipos should have already been called 
+ */
+static void posttrans_action_clean (bAction *act)
+{
+	ListBase act_data = {NULL, NULL};
+	bActListElem *ale;
+	int filter;
+	
+	/* filter data */
+	filter= (ACTFILTER_VISIBLE | ACTFILTER_FOREDIT | ACTFILTER_IPOKEYS);
+	actdata_filter(&act_data, filter, act, ACTCONT_ACTION);
+	
+	/* loop through relevant data, removing keyframes from the ipo-blocks that were attached 
+	 *  	- all keyframes are converted in/out of global time 
+	 */
+	for (ale= act_data.first; ale; ale= ale->next) {
+		if (NLA_ACTION_SCALED) {
+			actstrip_map_ipo_keys(OBACT, ale->key_data, 0, 1); 
+			posttrans_ipo_clean(ale->key_data);
+			actstrip_map_ipo_keys(OBACT, ale->key_data, 1, 1);
+		}
+		else 
+			posttrans_ipo_clean(ale->key_data);
+	}
+	
+	/* free temp data */
+	BLI_freelistN(&act_data);
+}
+
+/* ----------------------------- */
+
 /* This function tests if a point is on the "mouse" side of the cursor/frame-marking */
 static short FrameOnMouseSide(char side, float frame, float cframe)
 {
@@ -3293,7 +3389,6 @@ void special_aftertrans_update(TransInfo *t)
 		ob = OBACT;
 		
 		if (datatype == ACTCONT_ACTION) {
-			/* Update the curve */
 			/* Depending on the lock status, draw necessary views */
 			if (ob) {
 				ob->ctime= -1234567.0f;
@@ -3304,8 +3399,13 @@ void special_aftertrans_update(TransInfo *t)
 					DAG_object_flush_update(G.scene, ob, OB_RECALC_OB);
 			}
 			
+			/* Do curve updates */
 			remake_action_ipos((bAction *)data);
 			
+			/* Do curve cleanups? */
+			if ((G.saction->flag & SACTION_NOTRANSKEYCULL)==0)
+				posttrans_action_clean((bAction *)data);
+			
 			G.saction->flag &= ~SACTION_MOVING;
 		}
 		else if (datatype == ACTCONT_SHAPEKEY) {
@@ -3318,6 +3418,9 @@ void special_aftertrans_update(TransInfo *t)
 					sort_time_ipocurve(icu);
 					testhandles_ipocurve(icu);
 				}
+				
+				if ((G.saction->flag & SACTION_NOTRANSKEYCULL)==0)
+					posttrans_ipo_clean(key->ipo);
 			}
 			
 			DAG_object_flush_update(G.scene, OBACT, OB_RECALC_DATA);