Fix T38619: Confusing logic for Keying Set keyframing Settings
authorJoshua Leung <aligorith@gmail.com>
Mon, 26 Jan 2015 13:09:50 +0000 (02:09 +1300)
committerJoshua Leung <aligorith@gmail.com>
Mon, 26 Jan 2015 13:10:26 +0000 (02:10 +1300)
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!

release/scripts/startup/bl_ui/properties_scene.py
source/blender/blenkernel/intern/anim_sys.c
source/blender/editors/animation/keyingsets.c
source/blender/makesdna/DNA_anim_types.h
source/blender/makesrna/intern/rna_animation.c

index 5d822cef9afe61a9426ff5b0485537b3392b04c5..5e4c9314479ea7617654faa1418f155739d2279a 100644 (file)
@@ -85,7 +85,53 @@ class SCENE_PT_unit(SceneButtonsPanel, Panel):
             row.prop(unit, "use_separate")
 
 
-class SCENE_PT_keying_sets(SceneButtonsPanel, Panel):
+class SceneKeyingSetsPanel():
+    def draw_keyframing_settings(self, context, layout, ks, ksp):
+        self.draw_keyframing_setting(context, layout, ks, ksp, "Needed",
+                                     "use_insertkey_override_needed", "use_insertkey_needed",
+                                     userpref_fallback="use_keyframe_insert_needed")
+
+        self.draw_keyframing_setting(context, layout, ks, ksp, "Visual",
+                                     "use_insertkey_override_visual", "use_insertkey_visual",
+                                     userpref_fallback="use_visual_keying")
+
+        self.draw_keyframing_setting(context, layout, ks, ksp, "XYZ to RGB",
+                                     "use_insertkey_override_xyz_to_rgb", "use_insertkey_xyz_to_rgb")
+
+    def draw_keyframing_setting(self, context, layout, ks, ksp, label, toggle_prop, prop, userpref_fallback=None):
+        if ksp:
+            item = ksp
+
+            if getattr(ks, toggle_prop):
+                owner = ks
+                propname = prop
+            else:
+                owner = context.user_preferences.edit
+                if userpref_fallback:
+                    propname = userpref_fallback
+                else:
+                    propname = prop
+        else:
+            item = ks
+
+            owner = context.user_preferences.edit
+            if userpref_fallback:
+                propname = userpref_fallback
+            else:
+                propname = prop
+
+        row = layout.row(align=True)
+        row.prop(item, toggle_prop, text="", icon='STYLUS_PRESSURE', toggle=True) # XXX: needs dedicated icon
+
+        subrow = row.row()
+        subrow.active = getattr(item, toggle_prop)
+        if subrow.active:
+            subrow.prop(item, prop, text=label)
+        else:
+            subrow.prop(owner, propname, text=label)
+
+
+class SCENE_PT_keying_sets(SceneButtonsPanel, SceneKeyingSetsPanel, Panel):
     bl_label = "Keying Sets"
     COMPAT_ENGINES = {'BLENDER_RENDER', 'BLENDER_GAME'}
 
@@ -115,12 +161,10 @@ class SCENE_PT_keying_sets(SceneButtonsPanel, Panel):
 
             col = row.column()
             col.label(text="Keyframing Settings:")
-            col.prop(ks, "use_insertkey_needed", text="Needed")
-            col.prop(ks, "use_insertkey_visual", text="Visual")
-            col.prop(ks, "use_insertkey_xyz_to_rgb", text="XYZ to RGB")
+            self.draw_keyframing_settings(context, col, ks, None)
 
 
-class SCENE_PT_keying_set_paths(SceneButtonsPanel, Panel):
+class SCENE_PT_keying_set_paths(SceneButtonsPanel, SceneKeyingSetsPanel, Panel):
     bl_label = "Active Keying Set"
     COMPAT_ENGINES = {'BLENDER_RENDER', 'BLENDER_GAME'}
 
@@ -173,9 +217,7 @@ class SCENE_PT_keying_set_paths(SceneButtonsPanel, Panel):
 
             col = row.column()
             col.label(text="Keyframing Settings:")
-            col.prop(ksp, "use_insertkey_needed", text="Needed")
-            col.prop(ksp, "use_insertkey_visual", text="Visual")
-            col.prop(ksp, "use_insertkey_xyz_to_rgb", text="XYZ to RGB")
+            self.draw_keyframing_settings(context, col, ks, ksp)
 
 
 class SCENE_PT_color_management(SceneButtonsPanel, Panel):
index aa353afadc1e86e843ece0eb9603f7a60ea8d3f8..ea168ca6a8466d23ab61669f9dc9bfc2c3d30bc6 100644 (file)
@@ -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);
index a5c0eee8d3b42faa9fae192ae132428976215b51..6c2cc10f2a37db1d0a60cc3b71036b19c74c778b 100644 (file)
@@ -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)
index 3d0d6b820d731e5cc53e7533615132b344b6dcb4..9e6b71fcbacce7798762e13f2d76f86424b1e703 100644 (file)
@@ -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 */
index 8e1d4c0e333a2016d675e3efd4f2b1cffdb6ae56..e858381b567e5721281bfd2246a8794c86c27fbf 100644 (file)
@@ -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);
 }