Keying Sets: Added 'remove selected from active set' (Alt-K) operator in Outliner

* Cleaned up the helper functions for the Outliner operators which deal with Keying Sets
* Fixed a few minor bugs in the Keying Sets API that won't show up with the current tools, but may crop up later
* Added a new method to find a 'matching' path in a Keying Set. Now adding a new path to a Keying Set will firstly check if there is any similar path already, and skip adding another path.

3 files changed, 156 insertions, 62 deletions

diff --git a/source/blender/blenkernel/BKE_animsys.h b/source/blender/blenkernel/BKE_animsys.h
index 9da78aa5676..44c8d827e8c 100644
--- a/source/blender/blenkernel/BKE_animsys.h
+++ b/source/blender/blenkernel/BKE_animsys.h
@@ -10,6 +10,7 @@ struct ListBase;
 struct Main;
 struct AnimData;
 struct KeyingSet;
+struct KS_Path;
 
 /* ************************************* */
 /* AnimData API */
@@ -35,6 +36,8 @@ struct KeyingSet *BKE_keyingset_add(struct ListBase *list, const char name[], sh
 /* Add a destination to a KeyingSet */
 void BKE_keyingset_add_destination(struct KeyingSet *ks, struct ID *id, const char group_name[], const char rna_path[], int array_index, short flag, short groupmode);
 
+struct KS_Path *BKE_keyingset_find_destination(struct KeyingSet *ks, struct ID *id, const char group_name[], const char rna_path[], int array_index, int group_mode);
+
 /* Free data for KeyingSet but not set itself */
 void BKE_keyingset_free(struct KeyingSet *ks);
 
diff --git a/source/blender/blenkernel/intern/anim_sys.c b/source/blender/blenkernel/intern/anim_sys.c
index 89f4de567f2..0097b30b685 100644
--- a/source/blender/blenkernel/intern/anim_sys.c
+++ b/source/blender/blenkernel/intern/anim_sys.c
@@ -167,6 +167,56 @@ AnimData *BKE_copy_animdata (AnimData *adt)
  * from Python/scripts so that riggers can automate the creation of KeyingSets for their rigs.
  */
 
+/* Finding Tools --------------------------- */
+
+/* Find the first path that matches the given criteria */
+// TODO: do we want some method to perform partial matches too?
+KS_Path *BKE_keyingset_find_destination (KeyingSet *ks, ID *id, const char group_name[], const char rna_path[], int array_index, int group_mode)
+{
+	KS_Path *ksp;
+	
+	/* sanity checks */
+	if ELEM(NULL, ks, rna_path)
+		return NULL;
+	
+	/* ID is optional for relative KeyingSets, but is necessary for absolute KeyingSets */
+	if (id == NULL) {
+		if (ks->flag & KEYINGSET_ABSOLUTE)
+			return NULL;
+	}
+	
+	/* loop over paths in the current KeyingSet, finding the first one where all settings match 
+	 * (i.e. the first one where none of the checks fail and equal 0)
+	 */
+	for (ksp= ks->paths.first; ksp; ksp= ksp->next) {
+		short eq_id=1, eq_path=1, eq_index=1, eq_group=1;
+		
+		/* id */
+		if ((ks->flag & KEYINGSET_ABSOLUTE) && (id != ksp->id))
+			eq_id= 0;
+		
+		/* path */
+		if ((ksp->rna_path==0) || strcmp(rna_path, ksp->rna_path))
+			eq_path= 0;
+			
+		/* index */
+		if (ksp->array_index != array_index)
+			eq_index= 0;
+			
+		/* group */
+		if (group_name) {
+			// FIXME: these checks need to be coded... for now, it's not too important though
+		}
+			
+		/* if all aspects are ok, return */
+		if (eq_id && eq_path && eq_index && eq_group)
+			return ksp;
+	}
+	
+	/* none found */
+	return NULL;
+}
+ 
 /* Defining Tools --------------------------- */
 
 /* Used to create a new 'custom' KeyingSet for the user, that will be automatically added to the stack */
@@ -203,12 +253,16 @@ void BKE_keyingset_add_destination (KeyingSet *ks, ID *id, const char group_name
 	if ELEM(NULL, ks, rna_path)
 		return;
 	
-	/* ID is optional, and should only be provided for absolute KeyingSets */
-	if (id) {
-		if ((ks->flag & KEYINGSET_ABSOLUTE) == 0)
+	/* ID is optional for relative KeyingSets, but is necessary for absolute KeyingSets */
+	if (id == NULL) {
+		if (ks->flag & KEYINGSET_ABSOLUTE)
 			return;
 	}
 	
+	/* don't add if there is already a matching KS_Path in the KeyingSet */
+	if (BKE_keyingset_find_destination(ks, id, group_name, rna_path, array_index, groupmode))
+		return;
+	
 	/* allocate a new KeyingSet Path */
 	ksp= MEM_callocN(sizeof(KS_Path), "KeyingSet Path");
 	
diff --git a/source/blender/editors/space_outliner/outliner.c b/source/blender/editors/space_outliner/outliner.c
index fc737f26957..a3569365041 100644
--- a/source/blender/editors/space_outliner/outliner.c
+++ b/source/blender/editors/space_outliner/outliner.c
@@ -3098,9 +3098,6 @@ enum {
 	KEYINGSET_EDITMODE_REMOVE,
 } eKeyingSet_EditModes;
 
-/* typedef'd function-prototype style for KeyingSet operation callbacks */
-typedef void (*ksEditOp)(SpaceOops *soops, KeyingSet *ks, TreeElement *te, TreeStoreElem *tselem);
- 
 /* Utilities ---------------------------------- */
  
 /* specialised poll callback for these operators to work in Datablocks view only */
@@ -3139,8 +3136,12 @@ static KeyingSet *verify_active_keyingset(Scene *scene, short add)
 	return ks;
 }
 
-/* helper func to add a new KeyingSet Path */
-static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, TreeStoreElem *tselem)
+/* Helper func to extract an RNA path from seleted tree element 
+ * NOTE: the caller must zero-out all values of the pointers that it passes here first, as
+ * this function does not do that yet 
+ */
+static void tree_element_to_path(SpaceOops *soops, TreeElement *te, TreeStoreElem *tselem, 
+							ID **id, char **path, int *array_index, short *flag, short *groupmode)
 {
 	ListBase hierarchy = {NULL, NULL};
 	LinkData *ld;
@@ -3148,11 +3149,7 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 	TreeStoreElem *tse, *tsenext;
 	PointerRNA *ptr, *nextptr;
 	PropertyRNA *prop, *nameprop;
-	ID *id = NULL;
-	char *path=NULL, *newpath=NULL;
-	int array_index= 0;
-	short flag = 0;
-	short groupmode= KSP_GROUP_KSNAME;
+	char *newpath=NULL;
 	
 	/* optimise tricks:
 	 *	- Don't do anything if the selected item is a 'struct', but arrays are allowed
@@ -3160,8 +3157,6 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 	if (tselem->type == TSE_RNA_STRUCT)
 		return;
 	
-	printf("ks_editop_add_cb() \n");
-	
 	/* Overview of Algorithm:
 	 * 	1. Go up the chain of parents until we find the 'root', taking note of the 
 	 *	   levels encountered in reverse-order (i.e. items are added to the start of the list
@@ -3170,7 +3165,6 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 	 *	   (which will become the 'ID' for the KeyingSet Path), and build a  
 	 * 		path as we step through the chain
 	 */
-	// XXX do we want to separate this part out to a helper func for the other editing op at some point?
 	 
 	/* step 1: flatten out hierarchy of parents into a flat chain */
 	for (tem= te->parent; tem; tem= tem->parent) {
@@ -3188,16 +3182,7 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 		prop= tem->directdata;
 		
 		/* check if we're looking for first ID, or appending to path */
-		if (id) {
-			if(tse->type == TSE_RNA_STRUCT)
-				printf("\t tem = RNA Struct '%s' \n", tem->name);
-			else if(tse->type == TSE_RNA_ARRAY_ELEM)
-				printf("\t tem = RNA Array Elem '%s' \n", tem->name);
-			else if (tse->type == TSE_RNA_PROPERTY)
-				printf("\t tem = RNA Property '%s' \n", tem->name);
-			else
-				printf("\t tem = WTF? \n");
-			
+		if (*id) {
 			/* just 'append' property to path 
 			 *	- to prevent memory leaks, we must write to newpath not path, then free old path + swap them
 			 */
@@ -3205,43 +3190,43 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 			if(tse->type == TSE_RNA_PROPERTY) {
 				if(RNA_property_type(ptr, prop) == PROP_POINTER) {
 					/* for pointer we just append property name */
-					newpath= RNA_path_append(path, ptr, prop, 0, NULL);
+					newpath= RNA_path_append(*path, ptr, prop, 0, NULL);
 				}
 				else if(RNA_property_type(ptr, prop) == PROP_COLLECTION) {
 					temnext= (TreeElement*)(ld->next->data);
 					tsenext= TREESTORE(temnext);
-
+					
 					nextptr= &temnext->rnaptr;
 					nameprop= RNA_struct_name_property(nextptr);
-
+					
 					if(nameprop) {
 						/* if possible, use name as a key in the path */
 						char buf[128], *name;
 						name= RNA_property_string_get_alloc(nextptr, nameprop, buf, sizeof(buf));
-
-						newpath= RNA_path_append(path, NULL, prop, 0, name);
-
+						
+						newpath= RNA_path_append(*path, NULL, prop, 0, name);
+						
 						if(name != buf)
 							MEM_freeN(name);
 					}
 					else {
 						/* otherwise use index */
 						int index= 0;
-
+						
 						for(temsub=tem->subtree.first; temsub; temsub=temsub->next, index++)
 							if(temsub == temnext)
 								break;
-
-						newpath= RNA_path_append(path, NULL, prop, index, NULL);
+						
+						newpath= RNA_path_append(*path, NULL, prop, index, NULL);
 					}
-
+					
 					ld= ld->next;
 				}
 			}
 			
 			if(newpath) {
-				if (path) MEM_freeN(path);
-				path= newpath;
+				if (*path) MEM_freeN(*path);
+				*path= newpath;
 				newpath= NULL;
 			}
 		}
@@ -3250,11 +3235,11 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 			if (tse->type == TSE_RNA_STRUCT) {
 				/* ptr->data not ptr->id.data seems to be the one we want, since ptr->data is sometimes the owner of this ID? */
 				if(RNA_struct_is_ID(ptr)) {
-					id= (ID *)ptr->data;
-
+					*id= (ID *)ptr->data;
+					
 					/* clear path */
-					if(path) {
-						MEM_freeN(path);
+					if(*path) {
+						MEM_freeN(*path);
 						path= NULL;
 					}
 				}
@@ -3263,42 +3248,33 @@ static void ks_editop_add_cb(SpaceOops *soops, KeyingSet *ks, TreeElement *te, T
 	}
 
 	/* step 3: if we've got an ID, add the current item to the path */
-	if (id) {
+	if (*id) {
 		/* add the active property to the path */
-		// if array base, add KSP_FLAG_WHOLE_ARRAY
 		ptr= &te->rnaptr;
 		prop= te->directdata;
 		
 		/* array checks */
 		if (tselem->type == TSE_RNA_ARRAY_ELEM) {
 			/* item is part of an array, so must set the array_index */
-			array_index= te->index;
+			*array_index= te->index;
 		}
 		else if (RNA_property_array_length(ptr, prop)) {
 			/* entire array was selected, so keyframe all */
-			flag |= KSP_FLAG_WHOLE_ARRAY;
+			*flag |= KSP_FLAG_WHOLE_ARRAY;
 		}
 		
 		/* path */
-		newpath= RNA_path_append(path, NULL, prop, 0, NULL);
-		if (path) MEM_freeN(path);
-		path= newpath;
-		
-		printf("Adding KeyingSet '%s': Path %s %d \n", ks->name, path, array_index);
-		
-		/* add a new path with the information obtained (only if valid) */
-		// TODO: what do we do with group name? for now, we don't supply one, and just let this use the KeyingSet name
-		if (path)
-			BKE_keyingset_add_destination(ks, id, NULL, path, array_index, flag, groupmode);
+		newpath= RNA_path_append(*path, NULL, prop, 0, NULL);
+		if (*path) MEM_freeN(*path);
+		*path= newpath;
 	}
 
 	/* free temp data */
-	if (path) MEM_freeN(path);
 	BLI_freelistN(&hierarchy);
 }
 
 /* Recursively iterate over tree, finding and working on selected items */
-static void do_outliner_keyingset_editop(SpaceOops *soops, KeyingSet *ks, ListBase *tree, ksEditOp edit_cb)
+static void do_outliner_keyingset_editop(SpaceOops *soops, KeyingSet *ks, ListBase *tree, short mode)
 {
 	TreeElement *te;
 	TreeStoreElem *tselem;
@@ -3308,12 +3284,54 @@ static void do_outliner_keyingset_editop(SpaceOops *soops, KeyingSet *ks, ListBa
 		
 		/* if item is selected, perform operation */
 		if (tselem->flag & TSE_SELECTED) {
-			if (edit_cb) edit_cb(soops, ks, te, tselem);
+			ID *id= NULL;
+			char *path= NULL;
+			int array_index= 0;
+			short flag= 0;
+			short groupmode= KSP_GROUP_KSNAME;
+			
+			/* get id + path + index info from the selected element */
+			tree_element_to_path(soops, te, tselem, 
+					&id, &path, &array_index, &flag, &groupmode);
+			
+			/* only if ID and path were set, should we perform any actions */
+			if (id && path) {
+				/* action depends on mode */
+				switch (mode) {
+					case KEYINGSET_EDITMODE_ADD:
+					{
+						/* add a new path with the information obtained (only if valid) */
+						// TODO: what do we do with group name? for now, we don't supply one, and just let this use the KeyingSet name
+						BKE_keyingset_add_destination(ks, id, NULL, path, array_index, flag, groupmode);
+					}
+						break;
+					case KEYINGSET_EDITMODE_REMOVE:
+					{
+						/* find the relevant path, then remove it from the KeyingSet */
+						KS_Path *ksp= BKE_keyingset_find_destination(ks, id, NULL, path, array_index, groupmode);
+						
+						if (ksp) {
+							/* free path's data */
+							// TODO: we probably need an API method for this 
+							if (ksp->rna_path) MEM_freeN(ksp->rna_path);
+							
+							/* remove path from set */
+							BLI_freelinkN(&ks->paths, ksp);
+						}
+					}
+						break;
+				}
+				
+				/* free path, since it had to be generated */
+				MEM_freeN(path);
+			}
+			
+			
 		}
 		
 		/* go over sub-tree */
 		if ((tselem->flag & TSE_CLOSED)==0)
-			do_outliner_keyingset_editop(soops, ks, &te->subtree, edit_cb);
+			do_outliner_keyingset_editop(soops, ks, &te->subtree, mode);
 	}
 }
 
@@ -3334,8 +3352,7 @@ static int outliner_keyingset_additems_exec(bContext *C, wmOperator *op)
 		return OPERATOR_CANCELLED;
 	
 	/* recursively go into tree, adding selected items */
-	// TODO: make the last arg a callback func instead...
-	do_outliner_keyingset_editop(soutliner, ks, &soutliner->tree, ks_editop_add_cb);
+	do_outliner_keyingset_editop(soutliner, ks, &soutliner->tree, KEYINGSET_EDITMODE_ADD);
 	
 	/* send notifiers */
 	WM_event_add_notifier(C, NC_SCENE|ND_KEYINGSET, NULL);
@@ -3360,6 +3377,25 @@ void OUTLINER_OT_keyingset_add_selected(wmOperatorType *ot)
 
 /* Remove Operator ---------------------------------- */
 
+static int outliner_keyingset_removeitems_exec(bContext *C, wmOperator *op)
+{
+	SpaceOops *soutliner= (SpaceOops*)CTX_wm_space_data(C);
+	Scene *scene= CTX_data_scene(C);
+	KeyingSet *ks= verify_active_keyingset(scene, 1);
+	
+	/* check for invalid states */
+	if (soutliner == NULL)
+		return OPERATOR_CANCELLED;
+	
+	/* recursively go into tree, adding selected items */
+	do_outliner_keyingset_editop(soutliner, ks, &soutliner->tree, KEYINGSET_EDITMODE_REMOVE);
+	
+	/* send notifiers */
+	WM_event_add_notifier(C, NC_SCENE|ND_KEYINGSET, NULL);
+	
+	return OPERATOR_FINISHED;
+}
+
 void OUTLINER_OT_keyingset_remove_selected(wmOperatorType *ot)
 {
 	/* identifiers */
@@ -3367,6 +3403,7 @@ void OUTLINER_OT_keyingset_remove_selected(wmOperatorType *ot)
 	ot->name= "Keyingset Remove Selected";
 	
 	/* api callbacks */
+	ot->exec= outliner_keyingset_removeitems_exec;
 	ot->poll= ed_operator_outliner_datablocks_active;
 	
 	/* flags */