Anim: Deselect Keys before inserting new keys

This commit changes the keying code to deselect keyframes when inserting new keys.
This has been discussed in the Animation & Rigging module meeting [1].
There is also an RCS post about that [2].
Doing this brings key creation in line with object creation,
where only the newly created object is selected.
There has been a previous attempt [3] to do a similar thing.

### Changes
When inserting keys by pressing `I` in the viewport or choosing a keying set,
all keys of the `Action` get deselected before inserting new keys.
New keys are selected by default.
Python RNA functions are **NOT** affected, meaning addons using
those functions will not deselect any keys by default.
The developer has to choose to do so.
To make that easier, there is a new RNA function on the action
`deselect_keys`

[1]: https://devtalk.blender.org/t/2024-05-02-animation-rigging-module-meeting/34493#patches-review-decision-time-5
[2]: https://blender.community/c/rightclickselect/K0hbbc
[3]: https://archive.blender.org/developer/D11623

Pull Request: https://projects.blender.org/blender/blender/pulls/121908

source/blender/animrig/ANIM_action.hh

@@ -921,6 +921,16 @@ void assert_baklava_phase_1_invariants(const Strip &strip);
  */
 Action *convert_to_layered_action(Main &bmain, const Action &action);
 
+/**
+ * Deselect the keys of all actions in the Span. Duplicate entries are only visited once.
+ */
+void deselect_keys_actions(blender::Span<bAction *> actions);
+
+/**
+ * Deselect all keys within the action.
+ */
+void action_deselect_keys(Action &action);
+
 }  // namespace blender::animrig
 
 /* Wrap functions for the DNA structs. */

source/blender/animrig/CMakeLists.txt

@@ -22,6 +22,7 @@ set(INC_SYS
 set(SRC
   intern/action.cc
   intern/action_runtime.cc
+  intern/action_selection.cc
   intern/anim_rna.cc
   intern/animdata.cc
   intern/bone_collections.cc

source/blender/animrig/intern/action_selection.cc

@@ -0,0 +1,33 @@
+#include "DNA_action_types.h"
+#include "DNA_anim_types.h"
+
+#include "BLI_listbase.h"
+#include "BLI_set.hh"
+
+#include "BKE_anim_data.hh"
+#include "BKE_fcurve.hh"
+
+#include "ANIM_action.hh"
+#include "ANIM_fcurve.hh"
+
+namespace blender::animrig {
+
+void action_deselect_keys(Action &action)
+{
+  for (FCurve *fcu : fcurves_all(action)) {
+    BKE_fcurve_deselect_all_keys(*fcu);
+  }
+}
+
+void deselect_keys_actions(Span<bAction *> actions)
+{
+  Set<bAction *> visited_actions;
+  for (bAction *action : actions) {
+    if (!visited_actions.add(action)) {
+      continue;
+    }
+    action_deselect_keys(action->wrap());
+  }
+}
+
+}  // namespace blender::animrig

source/blender/blenkernel/BKE_fcurve.hh

@@ -442,6 +442,11 @@ bool BKE_fcurve_is_protected(const FCurve *fcu);
  */
 bool BKE_fcurve_has_selected_control_points(const FCurve *fcu);
 
+/**
+ * Deselect all keyframes within that FCurve.
+ */
+void BKE_fcurve_deselect_all_keys(FCurve &fcu);
+
 /**
  * Checks if the F-Curve has a Cycles modifier with simple settings
  * that warrant transition smoothing.

source/blender/blenkernel/intern/fcurve.cc

@@ -1018,6 +1018,16 @@ bool BKE_fcurve_has_selected_control_points(const FCurve *fcu)
   return false;
 }
 
+void BKE_fcurve_deselect_all_keys(FCurve &fcu)
+{
+  if (!fcu.bezt) {
+    return;
+  }
+  for (int i = 0; i < fcu.totvert; i++) {
+    BEZT_DESEL_ALL(&fcu.bezt[i]);
+  }
+}
+
 bool BKE_fcurve_is_keyframable(const FCurve *fcu)
 {
   /* F-Curve's keyframes must be "usable" (i.e. visible + have an effect on final result) */

source/blender/editors/animation/anim_deps.cc

@@ -18,15 +18,21 @@
 #include "DNA_object_types.h"
 #include "DNA_scene_types.h"
 #include "DNA_sequence_types.h"
+#include "DNA_space_types.h"
+#include "DNA_windowmanager_types.h"
 
 #include "BLI_blenlib.h"
+#include "BLI_set.hh"
 #include "BLI_utildefines.h"
 
 #include "BKE_action.h"
 #include "BKE_anim_data.hh"
+#include "BKE_context.hh"
 #include "BKE_fcurve.hh"
 #include "BKE_gpencil_legacy.h"
 #include "BKE_grease_pencil.hh"
+#include "BKE_screen.hh"
+#include "BKE_workspace.hh"
 
 #include "DEG_depsgraph.hh"
 
@@ -38,6 +44,8 @@
 
 #include "ED_anim_api.hh"
 
+#include "ANIM_action.hh"
+
 /* **************************** depsgraph tagging ******************************** */
 
 void ANIM_list_elem_update(Main *bmain, Scene *scene, bAnimListElem *ale)
@@ -460,3 +468,64 @@ void ANIM_animdata_freelist(ListBase *anim_data)
   BLI_freelistN(anim_data);
 #endif
 }
+
+void ANIM_deselect_keys_in_animation_editors(bContext *C)
+{
+  using namespace blender;
+
+  wmWindow *ctx_window = CTX_wm_window(C);
+  ScrArea *ctx_area = CTX_wm_area(C);
+  ARegion *ctx_region = CTX_wm_region(C);
+
+  Set<bAction *> dna_actions;
+  LISTBASE_FOREACH (wmWindow *, win, &CTX_wm_manager(C)->windows) {
+    bScreen *screen = BKE_workspace_active_screen_get(win->workspace_hook);
+
+    LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) {
+      if (!ELEM(area->spacetype, SPACE_GRAPH, SPACE_ACTION)) {
+        continue;
+      }
+      ARegion *window_region = BKE_area_find_region_type(area, RGN_TYPE_WINDOW);
+
+      if (!window_region) {
+        continue;
+      }
+
+      CTX_wm_window_set(C, win);
+      CTX_wm_area_set(C, area);
+      CTX_wm_region_set(C, window_region);
+      bAnimContext ac;
+      if (!ANIM_animdata_get_context(C, &ac)) {
+        continue;
+      }
+      ListBase anim_data = {nullptr, nullptr};
+      int filter = 0;
+      if (ac.spacetype == SPACE_GRAPH) {
+        SpaceGraph *graph_editor = (SpaceGraph *)ac.sl;
+        filter = graph_editor->ads->filterflag;
+      }
+      else {
+        BLI_assert(ac.spacetype == SPACE_ACTION);
+        SpaceAction *action_editor = (SpaceAction *)ac.sl;
+        filter = action_editor->ads.filterflag;
+      }
+      ANIM_animdata_filter(
+          &ac, &anim_data, eAnimFilter_Flags(filter), ac.data, eAnimCont_Types(ac.datatype));
+      LISTBASE_FOREACH (bAnimListElem *, ale, &anim_data) {
+        if (!ale->adt || !ale->adt->action) {
+          continue;
+        }
+        dna_actions.add(ale->adt->action);
+      }
+      ANIM_animdata_freelist(&anim_data);
+    }
+  }
+
+  CTX_wm_window_set(C, ctx_window);
+  CTX_wm_area_set(C, ctx_area);
+  CTX_wm_region_set(C, ctx_region);
+
+  for (bAction *dna_action : dna_actions) {
+    animrig::action_deselect_keys(dna_action->wrap());
+  }
+}

source/blender/editors/animation/keyframing.cc

@@ -40,6 +40,7 @@
 #include "DEG_depsgraph.hh"
 #include "DEG_depsgraph_query.hh"
 
+#include "ED_anim_api.hh"
 #include "ED_keyframing.hh"
 #include "ED_object.hh"
 #include "ED_screen.hh"
@@ -433,6 +434,8 @@ static int insert_key(bContext *C, wmOperator *op)
 
 static int insert_key_exec(bContext *C, wmOperator *op)
 {
+  ANIM_deselect_keys_in_animation_editors(C);
+
   Scene *scene = CTX_data_scene(C);
   /* Use the active keying set if there is one. */
   const int type = RNA_enum_get(op->ptr, "type");
@@ -469,6 +472,8 @@ void ANIM_OT_keyframe_insert(wmOperatorType *ot)
 
 static int keyframe_insert_with_keyingset_exec(bContext *C, wmOperator *op)
 {
+  ANIM_deselect_keys_in_animation_editors(C);
+
   Scene *scene = CTX_data_scene(C);
   KeyingSet *ks = keyingset_get_from_op_with_error(op, op->type->prop, scene);
   if (ks == nullptr) {
@@ -1062,6 +1067,8 @@ static int insert_key_button_exec(bContext *C, wmOperator *op)
         const std::optional<blender::StringRefNull> group = default_channel_group_for_path(
             &ptr, identifier);
 
+        ANIM_deselect_keys_in_animation_editors(C);
+
         /* NOTE: `index == -1` is a magic number, meaning either "operate on all
          * elements" or "not an array property". */
         const std::optional<int> array_index = (all || index < 0) ? std::nullopt :

source/blender/editors/armature/pose_lib_2.cc

@@ -45,6 +45,7 @@
 #include "ED_screen.hh"
 #include "ED_util.hh"
 
+#include "ANIM_action.hh"
 #include "ANIM_bone_collections.hh"
 #include "ANIM_keyframing.hh"
 #include "ANIM_keyingsets.hh"
@@ -155,6 +156,10 @@ static void poselib_keytag_pose(bContext *C, Scene *scene, PoseBlendData *pbd)
     ANIM_relative_keyingset_add_source(sources, &pbd->ob->id, &RNA_PoseBone, pchan);
   }
 
+  if (adt->action) {
+    blender::animrig::action_deselect_keys(adt->action->wrap());
+  }
+
   /* Perform actual auto-keying. */
   ANIM_apply_keyingset(
       C, &sources, ks, blender::animrig::ModifyKeyMode::INSERT, float(scene->r.cfra));

source/blender/editors/include/ED_anim_api.hh

@@ -753,6 +753,12 @@ void ANIM_set_active_channel(bAnimContext *ac,
  */
 bool ANIM_is_active_channel(bAnimListElem *ale);
 
+/**
+ * Deselects the keys displayed within the open animation editors. Depending on the display
+ * settings of those editors, the keys may not be from an action of the selected objects.
+ */
+void ANIM_deselect_keys_in_animation_editors(bContext *C);
+
 /* ************************************************ */
 /* DRAWING API */
 /* `anim_draw.cc` */

source/blender/editors/object/object_transform.cc

@@ -67,8 +67,10 @@
 #include "WM_api.hh"
 #include "WM_types.hh"
 
+#include "ANIM_action.hh"
 #include "ANIM_keyframing.hh"
 
+#include "ED_anim_api.hh"
 #include "ED_armature.hh"
 #include "ED_keyframing.hh"
 #include "ED_mesh.hh"
@@ -341,6 +343,10 @@ static int object_clear_transform_generic_exec(bContext *C,
   /* get KeyingSet to use */
   ks = ANIM_get_keyingset_for_autokeying(scene, default_ksName);
 
+  if (blender::animrig::is_autokey_on(scene)) {
+    ANIM_deselect_keys_in_animation_editors(C);
+  }
+
   for (Object *ob : objects) {
     if (use_transform_data_origin) {
       data_xform_container_item_ensure(xds, ob);

source/blender/editors/space_action/action_edit.cc

@@ -975,6 +975,8 @@ static int actkeys_insertkey_exec(bContext *C, wmOperator *op)
   /* what channels to affect? */
   mode = RNA_enum_get(op->ptr, "type");
 
+  ANIM_deselect_keys_in_animation_editors(C);
+
   /* insert keyframes */
   insert_action_keys(&ac, mode);
 

source/blender/editors/space_graph/graph_edit.cc

@@ -258,6 +258,8 @@ static int graphkeys_insertkey_exec(bContext *C, wmOperator *op)
     return OPERATOR_CANCELLED;
   }
 
+  ANIM_deselect_keys_in_animation_editors(C);
+
   /* Which channels to affect? */
   mode = eGraphKeys_InsertKey_Types(RNA_enum_get(op->ptr, "type"));
 

source/blender/editors/space_view3d/view3d_snap.cc

@@ -41,12 +41,14 @@
 #include "RNA_access.hh"
 #include "RNA_define.hh"
 
+#include "ED_anim_api.hh"
 #include "ED_curves.hh"
 #include "ED_keyframing.hh"
 #include "ED_object.hh"
 #include "ED_screen.hh"
 #include "ED_transverts.hh"
 
+#include "ANIM_action.hh"
 #include "ANIM_bone_collections.hh"
 #include "ANIM_keyframing.hh"
 
@@ -190,9 +192,11 @@ static int snap_sel_to_grid_exec(bContext *C, wmOperator *op)
 
     /* Build object array. */
     Vector<Object *> objects_eval;
+    Vector<Object *> objects_orig;
     {
       FOREACH_SELECTED_EDITABLE_OBJECT_BEGIN (view_layer_eval, v3d, ob_eval) {
         objects_eval.append(ob_eval);
+        objects_orig.append(DEG_get_original_object(ob_eval));
       }
       FOREACH_SELECTED_EDITABLE_OBJECT_END;
     }
@@ -214,6 +218,10 @@ static int snap_sel_to_grid_exec(bContext *C, wmOperator *op)
       xds = object::data_xform_container_create();
     }
 
+    if (blender::animrig::is_autokey_on(scene)) {
+      ANIM_deselect_keys_in_animation_editors(C);
+    }
+
     for (Object *ob_eval : objects_eval) {
       Object *ob = DEG_get_original_object(ob_eval);
       vec[0] = -ob_eval->object_to_world().location()[0] +
@@ -502,6 +510,10 @@ static bool snap_selected_to_location(bContext *C,
       }
     }
 
+    if (blender::animrig::is_autokey_on(scene)) {
+      ANIM_deselect_keys_in_animation_editors(C);
+    }
+
     for (Object *ob : objects) {
       if (ob->parent && BKE_object_flag_test_recursive(ob->parent, OB_DONE)) {
         continue;

source/blender/editors/transform/transform_convert_armature.cc

@@ -25,11 +25,13 @@
 
 #include "BIK_api.h"
 
+#include "ED_anim_api.hh"
 #include "ED_armature.hh"
 
 #include "DEG_depsgraph.hh"
 #include "DEG_depsgraph_query.hh"
 
+#include "ANIM_action.hh"
 #include "ANIM_bone_collections.hh"
 #include "ANIM_keyframing.hh"
 #include "ANIM_rna.hh"
@@ -1660,6 +1662,18 @@ static void special_aftertrans_update__pose(bContext *C, TransInfo *t)
   }
   else {
     const bool canceled = (t->state == TRANS_CANCEL);
+
+    if (blender::animrig::is_autokey_on(t->scene) && !canceled) {
+      blender::Vector<Object *> objects;
+      FOREACH_TRANS_DATA_CONTAINER (t, tc) {
+        for (int i = 0; i < tc->data_len; i++) {
+          const TransData *td = &tc->data[i];
+          objects.append(td->ob);
+        }
+      }
+      ANIM_deselect_keys_in_animation_editors(C);
+    }
+
     GSet *motionpath_updates = BLI_gset_ptr_new("motionpath updates");
 
     FOREACH_TRANS_DATA_CONTAINER (t, tc) {

source/blender/editors/transform/transform_convert_object.cc

@@ -21,8 +21,11 @@
 #include "BKE_rigidbody.h"
 #include "BKE_scene.hh"
 
+#include "ANIM_action.hh"
 #include "ANIM_keyframing.hh"
 #include "ANIM_rna.hh"
+
+#include "ED_anim_api.hh"
 #include "ED_object.hh"
 
 #include "DEG_depsgraph_query.hh"
@@ -904,6 +907,15 @@ static void special_aftertrans_update__object(bContext *C, TransInfo *t)
   TransDataContainer *tc = TRANS_DATA_CONTAINER_FIRST_SINGLE(t);
   bool motionpath_update = false;
 
+  if (blender::animrig::is_autokey_on(t->scene) && !canceled) {
+    blender::Vector<Object *> objects;
+    for (int i = 0; i < tc->data_len; i++) {
+      const TransData *td = &tc->data[i];
+      objects.append(td->ob);
+    }
+    ANIM_deselect_keys_in_animation_editors(C);
+  }
+
   for (int i = 0; i < tc->data_len; i++) {
     TransData *td = tc->data + i;
     ListBase pidlist;

source/blender/makesrna/intern/rna_action.cc

@@ -859,6 +859,12 @@ static void rna_Action_end_frame_set(PointerRNA *ptr, float value)
   CLAMP_MAX(data->frame_start, data->frame_end);
 }
 
+static void rna_Action_deselect_keys(bAction *act)
+{
+  animrig::action_deselect_keys(act->wrap());
+  WM_main_add_notifier(NC_ANIMATION | ND_KEYFRAME | NA_EDITED, nullptr);
+}
+
 /* Used to check if an action (value pointer)
  * is suitable to be assigned to the ID-block that is ptr. */
 bool rna_Action_id_poll(PointerRNA *ptr, PointerRNA value)
@@ -2092,6 +2098,10 @@ static void rna_def_action(BlenderRNA *brna)
   RNA_def_property_float_funcs(prop, "rna_Action_curve_frame_range_get", nullptr, nullptr);
   RNA_def_property_clear_flag(prop, PROP_EDITABLE);
 
+  FunctionRNA *func = RNA_def_function(srna, "deselect_keys", "rna_Action_deselect_keys");
+  RNA_def_function_ui_description(
+      func, "Deselects all keys of the Action. The selection status of F-Curves is unchanged");
+
   rna_def_action_legacy(brna, srna);
 
   /* API calls */

tests/python/bl_animation_keyframing.py

@@ -178,6 +178,19 @@ class InsertKeyTest(AbstractKeyframingTest, unittest.TestCase):
         _fcurve_paths_match(keyed_object.animation_data.action.fcurves, keyed_rna_paths)
         bpy.data.objects.remove(keyed_object, do_unlink=True)
 
+    def test_key_selection_state(self):
+        keyed_object = _create_animation_object()
+        bpy.context.preferences.edit.key_insert_channels = {"LOCATION"}
+        with bpy.context.temp_override(**_get_view3d_context()):
+            bpy.ops.anim.keyframe_insert()
+            bpy.context.scene.frame_set(5)
+            bpy.ops.anim.keyframe_insert()
+
+        for fcurve in keyed_object.animation_data.action.fcurves:
+            self.assertEqual(len(fcurve.keyframe_points), 2)
+            self.assertFalse(fcurve.keyframe_points[0].select_control_point)
+            self.assertTrue(fcurve.keyframe_points[1].select_control_point)
+
 
 class VisualKeyingTest(AbstractKeyframingTest, unittest.TestCase):
     """ Check if visual keying produces the correct keyframe values. """
@@ -368,6 +381,20 @@ class AutoKeyframingTest(AbstractKeyframingTest, unittest.TestCase):
         expected_paths = [f"{bone_path}.location", f"{bone_path}.rotation_euler", f"{bone_path}.scale"]
         _fcurve_paths_match(action.fcurves, expected_paths)
 
+    def test_key_selection_state(self):
+        armature_obj = _create_armature()
+        bpy.ops.object.mode_set(mode='POSE')
+        bpy.ops.transform.translate(value=(1, 0, 0))
+        bpy.context.scene.frame_set(5)
+        bpy.ops.transform.translate(value=(0, 1, 0))
+        bpy.ops.object.mode_set(mode='OBJECT')
+
+        action = armature_obj.animation_data.action
+        for fcurve in action.fcurves:
+            self.assertEqual(len(fcurve.keyframe_points), 2)
+            self.assertFalse(fcurve.keyframe_points[0].select_control_point)
+            self.assertTrue(fcurve.keyframe_points[1].select_control_point)
+
 
 class InsertAvailableTest(AbstractKeyframingTest, unittest.TestCase):