Fix T38619: Confusing logic for Keying Set keyframing Settings

The logic used for determining whether certain keyframing settings (i.e. visual,
only needed, xyz -> rgb) got applied was wonky. The original intention here was
that the Keying Set settings would override the global settings, and the path
settings would override what was used for the Keying Set. However, that was not
happening in all cases previously, as it was only possible to add flags and not
to turn them off.

This commit fixes that by introducing separate toggles to control whether the
Keying Set/Path's settings override the settings inherited from its parent
(i.e. the Keying Set for the Path, and the User Prefs for the Keying Set).
The icons used for these toggles could get revised a bit (we need something
which communicates "override this"; the current one is the closest I could find)

WARNING: If you have old keying sets, this may cause some breakage!

4 files changed, 70 insertions, 14 deletions

diff --git a/source/blender/blenkernel/intern/anim_sys.c b/source/blender/blenkernel/intern/anim_sys.c
index aa353afadc1..ea168ca6a84 100644
--- a/source/blender/blenkernel/intern/anim_sys.c
+++ b/source/blender/blenkernel/intern/anim_sys.c
@@ -1317,6 +1317,7 @@ KeyingSet *BKE_keyingset_add(ListBase *list, const char idname[], const char nam
 
 	ks->flag = flag;
 	ks->keyingflag = keyingflag;
+	ks->keyingoverride = keyingflag; /* NOTE: assume that if one is set one way, the other should be too, so that it'll work */
 	
 	/* add KeyingSet to list */
 	BLI_addtail(list, ks);
diff --git a/source/blender/editors/animation/keyingsets.c b/source/blender/editors/animation/keyingsets.c
index a5c0eee8d3b..6c2cc10f2a3 100644
--- a/source/blender/editors/animation/keyingsets.c
+++ b/source/blender/editors/animation/keyingsets.c
@@ -914,6 +914,37 @@ short ANIM_validate_keyingset(bContext *C, ListBase *dsources, KeyingSet *ks)
 	return 0;
 } 
 
+/* Determine which keying flags apply based on the override flags */
+static short keyingset_apply_keying_flags(const short base_flags, const short overrides, const short own_flags)
+{
+	short result = 0;
+	
+	/* The logic for whether a keying flag applies is as follows:
+	 *  - If the flag in question is set in "overrides", that means that the
+	 *    status of that flag in "own_flags" is used
+	 *  - If however the flag isn't set, then its value in "base_flags" is used
+	 *    instead (i.e. no override)
+	 */
+#define APPLY_KEYINGFLAG_OVERRIDE(kflag) \
+	if (overrides & kflag) {             \
+		result |= (own_flags & kflag);   \
+	}                                    \
+	else {                               \
+		result |= (base_flags & kflag);  \
+	}
+	
+	/* Apply the flags one by one... 
+	 * (See rna_def_common_keying_flags() for the supported flags)
+	 */
+	APPLY_KEYINGFLAG_OVERRIDE(INSERTKEY_NEEDED)
+	APPLY_KEYINGFLAG_OVERRIDE(INSERTKEY_MATRIX)
+	APPLY_KEYINGFLAG_OVERRIDE(INSERTKEY_XYZ2RGB)
+	
+#undef APPLY_KEYINGFLAG_OVERRIDE
+
+	return result;
+}
+
 /* Given a KeyingSet and context info (if required), modify keyframes for the channels specified
  * 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
@@ -923,7 +954,8 @@ int ANIM_apply_keyingset(bContext *C, ListBase *dsources, bAction *act, KeyingSe
 	Scene *scene = CTX_data_scene(C);
 	ReportList *reports = CTX_wm_reports(C);
 	KS_Path *ksp;
-	int kflag = 0, success = 0;
+	const short base_kflags = ANIM_get_keyframing_flags(scene, 1);
+	short kflag = 0, success = 0;
 	const char *groupname = NULL;
 	
 	/* sanity checks */
@@ -932,11 +964,8 @@ int ANIM_apply_keyingset(bContext *C, ListBase *dsources, bAction *act, KeyingSe
 	
 	/* get flags to use */
 	if (mode == MODIFYKEY_MODE_INSERT) {
-		/* use KeyingSet's flags as base */
-		kflag = ks->keyingflag;
-		
-		/* supplement with info from the context */
-		kflag |= ANIM_get_keyframing_flags(scene, 1);
+		/* use context settings as base */
+		kflag = keyingset_apply_keying_flags(base_kflags, ks->keyingoverride, ks->keyingflag);
 	}
 	else if (mode == MODIFYKEY_MODE_DELETE)
 		kflag = 0;
@@ -962,8 +991,8 @@ int ANIM_apply_keyingset(bContext *C, ListBase *dsources, bAction *act, KeyingSe
 			continue;
 		}
 		
-		/* since keying settings can be defined on the paths too, extend the path before using it */
-		kflag2 = (kflag | ksp->keyingflag);
+		/* since keying settings can be defined on the paths too, apply the settings for this path first */
+		kflag2 = keyingset_apply_keying_flags(kflag, ksp->keyingoverride, ksp->keyingflag);
 		
 		/* get pointer to name of group to add channels to */
 		if (ksp->groupmode == KSP_GROUP_NONE)
diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h
index 3d0d6b820d7..9e6b71fcbac 100644
--- a/source/blender/makesdna/DNA_anim_types.h
+++ b/source/blender/makesdna/DNA_anim_types.h
@@ -719,13 +719,13 @@ typedef struct KS_Path {
 	int idtype;				/* ID-type that path can be used on */
 	
 	short groupmode;		/* group naming (eKSP_Grouping) */
-	short pad;
+	short flag;				/* various settings, etc. */
 	
 	char *rna_path;			/* dynamically (or statically in the case of predefined sets) path */
 	int array_index;		/* index that path affects */
 	
-	short flag;				/* various settings, etc. */
-	short keyingflag;		/* settings to supply insertkey() with */
+	short keyingflag;		/* (eInsertKeyFlags) settings to supply insertkey() with */
+	short keyingoverride;	/* (eInsertKeyFlags) for each flag set, the relevant keyingflag bit overrides the default */
 } KS_Path;
 
 /* KS_Path->flag */
@@ -770,10 +770,14 @@ typedef struct KeyingSet {
 	char description[240];	/* (RNA_DYN_DESCR_MAX) short help text. */
 	char typeinfo[64];		/* name of the typeinfo data used for the relative paths */
 	
+	int active_path;		/* index of the active path */
+	
 	short flag;				/* settings for KeyingSet */
-	short keyingflag;		/* settings to supply insertkey() with */
 	
-	int active_path;		/* index of the active path */
+	short keyingflag;		/* (eInsertKeyFlags) settings to supply insertkey() with */
+	short keyingoverride;	/* (eInsertKeyFlags) for each flag set, the relevant keyingflag bit overrides the default */
+	
+	char pad[6];
 } KeyingSet;
 
 /* KeyingSet settings */
diff --git a/source/blender/makesrna/intern/rna_animation.c b/source/blender/makesrna/intern/rna_animation.c
index 8e1d4c0e333..e858381b567 100644
--- a/source/blender/makesrna/intern/rna_animation.c
+++ b/source/blender/makesrna/intern/rna_animation.c
@@ -558,6 +558,28 @@ static void rna_def_common_keying_flags(StructRNA *srna, short reg)
 {
 	PropertyRNA *prop;
 	
+	/* override scene/userpref defaults? */
+	prop = RNA_def_property(srna, "use_insertkey_override_needed", PROP_BOOLEAN, PROP_NONE);
+	RNA_def_property_boolean_sdna(prop, NULL, "keyingoverride", INSERTKEY_NEEDED);
+	RNA_def_property_ui_text(prop, "Override Insert Keyframes Default- Only Needed", 
+	                         "Override default setting to only insert keyframes where they're needed in the relevant F-Curves");
+	if (reg) RNA_def_property_flag(prop, PROP_REGISTER_OPTIONAL);
+	
+	prop = RNA_def_property(srna, "use_insertkey_override_visual", PROP_BOOLEAN, PROP_NONE);
+	RNA_def_property_boolean_sdna(prop, NULL, "keyingoverride", INSERTKEY_MATRIX);
+	RNA_def_property_ui_text(prop, "Override Insert Keyframes Default - Visual", 
+	                         "Override default setting to insert keyframes based on 'visual transforms'");
+	if (reg) RNA_def_property_flag(prop, PROP_REGISTER_OPTIONAL);
+	
+	prop = RNA_def_property(srna, "use_insertkey_override_xyz_to_rgb", PROP_BOOLEAN, PROP_NONE);
+	RNA_def_property_boolean_sdna(prop, NULL, "keyingoverride", INSERTKEY_XYZ2RGB);
+	RNA_def_property_ui_text(prop, "Override F-Curve Colors - XYZ to RGB",
+	                        "Override default setting to set color for newly added transformation F-Curves (Location, Rotation, Scale) "
+							"to be based on the transform axis");
+	if (reg) RNA_def_property_flag(prop, PROP_REGISTER_OPTIONAL);
+	
+	
+	/* value to override defaults with */
 	prop = RNA_def_property(srna, "use_insertkey_needed", PROP_BOOLEAN, PROP_NONE);
 	RNA_def_property_boolean_sdna(prop, NULL, "keyingflag", INSERTKEY_NEEDED);
 	RNA_def_property_ui_text(prop, "Insert Keyframes - Only Needed", "Only insert keyframes where they're needed in the relevant F-Curves");
@@ -570,7 +592,7 @@ static void rna_def_common_keying_flags(StructRNA *srna, short reg)
 	
 	prop = RNA_def_property(srna, "use_insertkey_xyz_to_rgb", PROP_BOOLEAN, PROP_NONE);
 	RNA_def_property_boolean_sdna(prop, NULL, "keyingflag", INSERTKEY_XYZ2RGB);
-	RNA_def_property_ui_text(prop, "F-Curve Colors - XYZ to RGB", "Color for newly added transformation F-Curves (Location, Rotation, Scale) and also Color is based on the transform axis");
+	RNA_def_property_ui_text(prop, "F-Curve Colors - XYZ to RGB", "Color for newly added transformation F-Curves (Location, Rotation, Scale) is based on the transform axis");
 	if (reg) RNA_def_property_flag(prop, PROP_REGISTER_OPTIONAL);
 }