UI: Changes to Graph Editor selection and transform

When introducing "drag-all-selected" support all over Blender, we
figured this wouldn't work well with the Graph Editor
selection/transform behavior.
Hence, William and I worked on the following changes, although we used
this chance to improve the behavior in general too.
For more info see T70634.

* Handles now always move with the key, regardless if they are selected
  or not.
* Selecting the key doesn't select the handles anymore, their selection
  is separate.
* Multiple keys and handles can now be dragged.
* Dragging a handle moves all selected handles **on the same side**.
* Tweak-dragging any handle can never affect any keyframe location,
  only handles.
* G/R/S should behave as before.
* Changing the handle type with a key selected always applies the change
  to both handles.
* Box selection with Ctrl+Drag now allows deselecting handles (used to
  act on entire triple only).
* Box selection //Include Handles// option now only acts on visible
  handles, wasn't the case with Only Selected Keyframes Handles enabled.
* Box selection //Include Handles// is now enabled by default in all
  bundled keymaps.

The changes have been tested for some days by the animators here in the
Blender Animation Studio. Some changes are based on their feedback.

Also, this improves/adds comments for related code.

Differential Revision: https://developer.blender.org/D6235

Reviewed by: Sybren Stüvel, William Reynish

1 files changed, 91 insertions, 43 deletions

diff --git a/source/blender/editors/space_graph/graph_select.c b/source/blender/editors/space_graph/graph_select.c
index 4bd738fca95..9c1d63ef68a 100644
--- a/source/blender/editors/space_graph/graph_select.c
+++ b/source/blender/editors/space_graph/graph_select.c
@@ -552,6 +552,10 @@ static void box_select_graphkeys(bAnimContext *ac,
     ked.data = &scaled_rectf;
   }
 
+  if (sipo->flag & SIPO_SELVHANDLESONLY) {
+    ked.iterflags |= KEYFRAME_ITER_HANDLES_DEFAULT_INVISIBLE;
+  }
+
   /* treat handles separately? */
   if (incl_handles) {
     ked.iterflags |= KEYFRAME_ITER_INCL_HANDLES;
@@ -722,7 +726,7 @@ void GRAPH_OT_select_box(wmOperatorType *ot)
   ot->prop = RNA_def_boolean(ot->srna, "axis_range", 0, "Axis Range", "");
   RNA_def_boolean(ot->srna,
                   "include_handles",
-                  0,
+                  true,
                   "Include Handles",
                   "Are handles tested individually against the selection criteria");
 
@@ -1417,23 +1421,42 @@ void GRAPH_OT_select_leftright(wmOperatorType *ot)
  */
 
 /* option 1) select keyframe directly under mouse */
-static void mouse_graph_keys(bAnimContext *ac,
-                             const int mval[2],
-                             short select_mode,
-                             const bool deselect_all,
-                             const bool curves_only)
+static int mouse_graph_keys(bAnimContext *ac,
+                            const int mval[2],
+                            eEditKeyframes_Select select_mode,
+                            const bool deselect_all,
+                            const bool curves_only,
+                            bool wait_to_deselect_others)
 {
   SpaceGraph *sipo = (SpaceGraph *)ac->sl;
   tNearestVertInfo *nvi;
   BezTriple *bezt = NULL;
+  bool run_modal = false;
 
   /* find the beztriple that we're selecting, and the handle that was clicked on */
   nvi = find_nearest_fcurve_vert(ac, mval);
 
+  if (select_mode != SELECT_REPLACE) {
+    /* The modal execution to delay deselecting other items is only needed for normal click
+     * selection, i.e. for SELECT_REPLACE. */
+    wait_to_deselect_others = false;
+  }
+
+  sipo->runtime.flag &= ~(SIPO_RUNTIME_FLAG_TWEAK_HANDLES_LEFT |
+                          SIPO_RUNTIME_FLAG_TWEAK_HANDLES_RIGHT);
+
+  const bool already_selected =
+      (nvi != NULL) && (((nvi->hpoint == NEAREST_HANDLE_KEY) && (nvi->bezt->f2 & SELECT)) ||
+                        ((nvi->hpoint == NEAREST_HANDLE_LEFT) && (nvi->bezt->f1 & SELECT)) ||
+                        ((nvi->hpoint == NEAREST_HANDLE_RIGHT) && (nvi->bezt->f3 & SELECT)));
+
+  if (wait_to_deselect_others && nvi && already_selected) {
+    run_modal = true;
+  }
   /* For replacing selection, if we have something to select, we have to clear existing selection.
    * The same goes if we found nothing to select, and deselect_all is true
    * (deselect on nothing behavior). */
-  if ((nvi != NULL && select_mode == SELECT_REPLACE) || (nvi == NULL && deselect_all)) {
+  else if ((nvi != NULL && select_mode == SELECT_REPLACE) || (nvi == NULL && deselect_all)) {
     /* reset selection mode */
     select_mode = SELECT_ADD;
 
@@ -1450,7 +1473,7 @@ static void mouse_graph_keys(bAnimContext *ac,
   }
 
   if (nvi == NULL) {
-    return;
+    return deselect_all ? OPERATOR_FINISHED : OPERATOR_CANCELLED;
   }
 
   /* if points can be selected on this F-Curve */
@@ -1461,17 +1484,9 @@ static void mouse_graph_keys(bAnimContext *ac,
       bezt = nvi->bezt; /* used to check bezt seletion is set */
       /* depends on selection mode */
       if (select_mode == SELECT_INVERT) {
-        /* keyframe - invert select of all */
         if (nvi->hpoint == NEAREST_HANDLE_KEY) {
-          if (BEZT_ISSEL_ANY(bezt)) {
-            BEZT_DESEL_ALL(bezt);
-          }
-          else {
-            BEZT_SEL_ALL(bezt);
-          }
+          bezt->f2 ^= SELECT;
         }
-
-        /* handles - toggle selection of relevant handle */
         else if (nvi->hpoint == NEAREST_HANDLE_LEFT) {
           /* toggle selection */
           bezt->f1 ^= SELECT;
@@ -1482,11 +1497,9 @@ static void mouse_graph_keys(bAnimContext *ac,
         }
       }
       else {
-        /* if the keyframe was clicked on, select all verts of given beztriple */
         if (nvi->hpoint == NEAREST_HANDLE_KEY) {
-          BEZT_SEL_ALL(bezt);
+          bezt->f2 |= SELECT;
         }
-        /* otherwise, select the handle that applied */
         else if (nvi->hpoint == NEAREST_HANDLE_LEFT) {
           bezt->f1 |= SELECT;
         }
@@ -1547,8 +1560,17 @@ static void mouse_graph_keys(bAnimContext *ac,
     ANIM_set_active_channel(ac, ac->data, ac->datatype, filter, nvi->fcu, nvi->ctype);
   }
 
+  if (nvi->hpoint == NEAREST_HANDLE_LEFT) {
+    sipo->runtime.flag |= SIPO_RUNTIME_FLAG_TWEAK_HANDLES_LEFT;
+  }
+  else if (nvi->hpoint == NEAREST_HANDLE_RIGHT) {
+    sipo->runtime.flag |= SIPO_RUNTIME_FLAG_TWEAK_HANDLES_RIGHT;
+  }
+
   /* free temp sample data for filtering */
   MEM_freeN(nvi);
+
+  return run_modal ? OPERATOR_RUNNING_MODAL : OPERATOR_FINISHED;
 }
 
 /* Option 2) Selects all the keyframes on either side of the current frame
@@ -1556,11 +1578,15 @@ static void mouse_graph_keys(bAnimContext *ac,
 /* (see graphkeys_select_leftright) */
 
 /* Option 3) Selects all visible keyframes in the same frame as the mouse click */
-static void graphkeys_mselect_column(bAnimContext *ac, const int mval[2], short select_mode)
+static int graphkeys_mselect_column(bAnimContext *ac,
+                                    const int mval[2],
+                                    eEditKeyframes_Select select_mode,
+                                    bool wait_to_deselect_others)
 {
   ListBase anim_data = {NULL, NULL};
   bAnimListElem *ale;
   int filter;
+  bool run_modal = false;
 
   KeyframeEditFunc select_cb, ok_cb;
   KeyframeEditData ked;
@@ -1572,15 +1598,22 @@ static void graphkeys_mselect_column(bAnimContext *ac, const int mval[2], short
 
   /* check if anything to select */
   if (nvi == NULL) {
-    return;
+    return OPERATOR_CANCELLED;
   }
 
   /* get frame number on which elements should be selected */
   // TODO: should we restrict to integer frames only?
   selx = nvi->frame;
 
-  /* if select mode is replace, deselect all keyframes first */
-  if (select_mode == SELECT_REPLACE) {
+  if (select_mode != SELECT_REPLACE) {
+    /* Doesn't need to deselect anything -> Pass. */
+  }
+  else if (wait_to_deselect_others && (nvi->bezt->f2 & SELECT)) {
+    run_modal = true;
+  }
+  /* If select mode is replace (and we don't do delayed deselection on mouse release), deselect all
+   * keyframes first. */
+  else {
     /* reset selection mode to add to selection */
     select_mode = SELECT_ADD;
 
@@ -1622,12 +1655,14 @@ static void graphkeys_mselect_column(bAnimContext *ac, const int mval[2], short
   MEM_freeN(nvi);
   BLI_freelistN(&ked.list);
   ANIM_animdata_freelist(&anim_data);
+
+  return run_modal ? OPERATOR_RUNNING_MODAL : OPERATOR_FINISHED;
 }
 
 /* ------------------- */
 
 /* handle clicking */
-static int graphkeys_clickselect_invoke(bContext *C, wmOperator *op, const wmEvent *event)
+static int graphkeys_clickselect_exec(bContext *C, wmOperator *op)
 {
   bAnimContext ac;
 
@@ -1639,27 +1674,37 @@ static int graphkeys_clickselect_invoke(bContext *C, wmOperator *op, const wmEve
   /* select mode is either replace (deselect all, then add) or add/extend */
   const short selectmode = RNA_boolean_get(op->ptr, "extend") ? SELECT_INVERT : SELECT_REPLACE;
   const bool deselect_all = RNA_boolean_get(op->ptr, "deselect_all");
+  /* See #WM_operator_properties_generic_select() for a detailed description of the how and why of
+   * this. */
+  const bool wait_to_deselect_others = RNA_boolean_get(op->ptr, "wait_to_deselect_others");
+  int mval[2];
+  int ret_val;
+
+  mval[0] = RNA_int_get(op->ptr, "mouse_x");
+  mval[1] = RNA_int_get(op->ptr, "mouse_y");
 
   /* figure out action to take */
   if (RNA_boolean_get(op->ptr, "column")) {
     /* select all keyframes in the same frame as the one that was under the mouse */
-    graphkeys_mselect_column(&ac, event->mval, selectmode);
+    ret_val = graphkeys_mselect_column(&ac, mval, selectmode, wait_to_deselect_others);
   }
   else if (RNA_boolean_get(op->ptr, "curves")) {
     /* select all keyframes in the same F-Curve as the one under the mouse */
-    mouse_graph_keys(&ac, event->mval, selectmode, deselect_all, true);
+    ret_val = mouse_graph_keys(&ac, mval, selectmode, deselect_all, true, wait_to_deselect_others);
   }
   else {
     /* select keyframe under mouse */
-    mouse_graph_keys(&ac, event->mval, selectmode, deselect_all, false);
+    ret_val = mouse_graph_keys(
+        &ac, mval, selectmode, deselect_all, false, wait_to_deselect_others);
   }
 
-  /* set notifier that keyframe selection (and also channel selection in some cases) has changed */
+  /* set notifier that keyframe selection (and also channel selection in some cases) has
+   * changed */
   WM_event_add_notifier(C, NC_ANIMATION | ND_KEYFRAME | NA_SELECTED, NULL);
   WM_event_add_notifier(C, NC_ANIMATION | ND_ANIMCHAN | NA_SELECTED, NULL);
 
   /* for tweak grab to work */
-  return OPERATOR_FINISHED | OPERATOR_PASS_THROUGH;
+  return ret_val | OPERATOR_PASS_THROUGH;
 }
 
 void GRAPH_OT_clickselect(wmOperatorType *ot)
@@ -1672,19 +1717,22 @@ void GRAPH_OT_clickselect(wmOperatorType *ot)
   ot->description = "Select keyframes by clicking on them";
 
   /* callbacks */
-  ot->invoke = graphkeys_clickselect_invoke;
   ot->poll = graphop_visible_keyframes_poll;
+  ot->exec = graphkeys_clickselect_exec;
+  ot->invoke = WM_generic_select_invoke;
+  ot->modal = WM_generic_select_modal;
 
   /* flags */
   ot->flag = OPTYPE_UNDO;
 
   /* properties */
-  prop = RNA_def_boolean(
-      ot->srna,
-      "extend",
-      0,
-      "Extend Select",
-      "Toggle keyframe selection instead of leaving newly selected keyframes only");  // SHIFTKEY
+  WM_operator_properties_generic_select(ot);
+  prop = RNA_def_boolean(ot->srna,
+                         "extend",
+                         0,
+                         "Extend Select",
+                         "Toggle keyframe selection instead of leaving newly selected "
+                         "keyframes only");  // SHIFTKEY
   RNA_def_property_flag(prop, PROP_SKIP_SAVE);
 
   prop = RNA_def_boolean(ot->srna,
@@ -1694,12 +1742,12 @@ void GRAPH_OT_clickselect(wmOperatorType *ot)
                          "Deselect all when nothing under the cursor");
   RNA_def_property_flag(prop, PROP_SKIP_SAVE);
 
-  prop = RNA_def_boolean(
-      ot->srna,
-      "column",
-      0,
-      "Column Select",
-      "Select all keyframes that occur on the same frame as the one under the mouse");  // ALTKEY
+  prop = RNA_def_boolean(ot->srna,
+                         "column",
+                         0,
+                         "Column Select",
+                         "Select all keyframes that occur on the same frame as the one under "
+                         "the mouse");  // ALTKEY
   RNA_def_property_flag(prop, PROP_SKIP_SAVE);
 
   prop = RNA_def_boolean(ot->srna,