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

5 files changed, 154 insertions, 103 deletions

diff --git a/source/blender/editors/transform/transform.c b/source/blender/editors/transform/transform.c
index b493bbe8867..f3d26f85471 100644
--- a/source/blender/editors/transform/transform.c
+++ b/source/blender/editors/transform/transform.c
@@ -2359,6 +2359,7 @@ bool initTransform(bContext *C, TransInfo *t, wmOperator *op, const wmEvent *eve
 
   /* Needed to translate tweak events to mouse buttons. */
   t->launch_event = event ? WM_userdef_event_type_from_keymap_type(event->type) : -1;
+  t->is_launch_event_tweak = event ? ISTWEAK(event->type) : false;
 
   /* XXX Remove this when wm_operator_call_internal doesn't use window->eventstate
    * (which can have type = 0) */
diff --git a/source/blender/editors/transform/transform.h b/source/blender/editors/transform/transform.h
index 0264069c368..e5e7be025e4 100644
--- a/source/blender/editors/transform/transform.h
+++ b/source/blender/editors/transform/transform.h
@@ -213,7 +213,8 @@ typedef struct TransData2D {
   float ih1[2], ih2[2];
 } TransData2D;
 
-/** Used to store 2 handles for each #TransData in case the other handle wasn't selected. */
+/** Used to store 2 handles for each #TransData in case the other handle wasn't selected. Also to
+ * unset temporary flags. */
 typedef struct TransDataCurveHandleFlags {
   char ih1, ih2;
   char *h1, *h2;
@@ -631,6 +632,9 @@ typedef struct TransInfo {
   /*************** NEW STUFF *********************/
   /** event type used to launch transform. */
   short launch_event;
+  /** Is the actual launch event a tweak event? (launch_event above is set to the corresponding
+   * mouse button then.) */
+  bool is_launch_event_tweak;
 
   struct {
     /** Orientation type when when we're not constrained.
diff --git a/source/blender/editors/transform/transform_convert.c b/source/blender/editors/transform/transform_convert.c
index 0c49d67f7a2..db8f36883f8 100644
--- a/source/blender/editors/transform/transform_convert.c
+++ b/source/blender/editors/transform/transform_convert.c
@@ -921,10 +921,17 @@ typedef struct tRetainedKeyframe {
   size_t del_count; /* number of keyframes of this sort that have been deleted so far */
 } tRetainedKeyframe;
 
-/* Called during special_aftertrans_update to make sure selected keyframes replace
+/**
+ * Called during special_aftertrans_update to make sure selected keyframes replace
  * any other keyframes which may reside on that frame (that is not selected).
+ *
+ * \param sel_flag: The flag (bezt.f1/2/3) value to use to determine selection. Usually `SELECT`,
+ *                  but may want to use a different one at times (if caller does not operate on
+ *                  selection).
  */
-static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle)
+static void posttrans_fcurve_clean(FCurve *fcu,
+                                   const eBezTriple_Flag sel_flag,
+                                   const bool use_handle)
 {
   /* NOTE: We assume that all keys are sorted */
   ListBase retained_keys = {NULL, NULL};
@@ -1036,7 +1043,7 @@ static void posttrans_fcurve_clean(FCurve *fcu, const bool use_handle)
   }
 
   /* 3) Recalculate handles */
-  testhandles_fcurve(fcu, use_handle);
+  testhandles_fcurve(fcu, sel_flag, use_handle);
 
   /* cleanup */
   BLI_freelistN(&retained_keys);
@@ -1064,11 +1071,11 @@ static void posttrans_action_clean(bAnimContext *ac, bAction *act)
 
     if (adt) {
       ANIM_nla_mapping_apply_fcurve(adt, ale->key_data, 0, 0);
-      posttrans_fcurve_clean(ale->key_data, false); /* only use handles in graph editor */
+      posttrans_fcurve_clean(ale->key_data, SELECT, false); /* only use handles in graph editor */
       ANIM_nla_mapping_apply_fcurve(adt, ale->key_data, 1, 0);
     }
     else {
-      posttrans_fcurve_clean(ale->key_data, false); /* only use handles in graph editor */
+      posttrans_fcurve_clean(ale->key_data, SELECT, false); /* only use handles in graph editor */
     }
   }
 
@@ -1090,7 +1097,7 @@ typedef struct BeztMap {
 /* This function converts an FCurve's BezTriple array to a BeztMap array
  * NOTE: this allocates memory that will need to get freed later
  */
-static BeztMap *bezt_to_beztmaps(BezTriple *bezts, int totvert, const short UNUSED(use_handle))
+static BeztMap *bezt_to_beztmaps(BezTriple *bezts, int totvert)
 {
   BezTriple *bezt = bezts;
   BezTriple *prevbezt = NULL;
@@ -1118,7 +1125,7 @@ static BeztMap *bezt_to_beztmaps(BezTriple *bezts, int totvert, const short UNUS
 }
 
 /* This function copies the code of sort_time_ipocurve, but acts on BeztMap structs instead */
-static void sort_time_beztmaps(BeztMap *bezms, int totvert, const short UNUSED(use_handle))
+static void sort_time_beztmaps(BeztMap *bezms, int totvert)
 {
   BeztMap *bezm;
   int i, ok = 1;
@@ -1163,8 +1170,7 @@ static void sort_time_beztmaps(BeztMap *bezms, int totvert, const short UNUSED(u
 }
 
 /* This function firstly adjusts the pointers that the transdata has to each BezTriple */
-static void beztmap_to_data(
-    TransInfo *t, FCurve *fcu, BeztMap *bezms, int totvert, const short UNUSED(use_handle))
+static void beztmap_to_data(TransInfo *t, FCurve *fcu, BeztMap *bezms, int totvert)
 {
   BezTriple *bezts = fcu->bezt;
   BeztMap *bezm;
@@ -1270,9 +1276,9 @@ void remake_graph_transdata(TransInfo *t, ListBase *anim_data)
 
       /* adjust transform-data pointers */
       /* note, none of these functions use 'use_handle', it could be removed */
-      bezm = bezt_to_beztmaps(fcu->bezt, fcu->totvert, use_handle);
-      sort_time_beztmaps(bezm, fcu->totvert, use_handle);
-      beztmap_to_data(t, fcu, bezm, fcu->totvert, use_handle);
+      bezm = bezt_to_beztmaps(fcu->bezt, fcu->totvert);
+      sort_time_beztmaps(bezm, fcu->totvert);
+      beztmap_to_data(t, fcu, bezm, fcu->totvert);
 
       /* free mapping stuff */
       MEM_freeN(bezm);
@@ -1281,7 +1287,7 @@ void remake_graph_transdata(TransInfo *t, ListBase *anim_data)
       sort_time_fcurve(fcu);
 
       /* make sure handles are all set correctly */
-      testhandles_fcurve(fcu, use_handle);
+      testhandles_fcurve(fcu, BEZT_FLAG_TEMP_TAG, use_handle);
     }
   }
 }
@@ -1959,11 +1965,11 @@ void special_aftertrans_update(bContext *C, TransInfo *t)
         if ((saction->flag & SACTION_NOTRANSKEYCULL) == 0 && ((canceled == 0) || (duplicate))) {
           if (adt) {
             ANIM_nla_mapping_apply_fcurve(adt, fcu, 0, 0);
-            posttrans_fcurve_clean(fcu, false); /* only use handles in graph editor */
+            posttrans_fcurve_clean(fcu, SELECT, false); /* only use handles in graph editor */
             ANIM_nla_mapping_apply_fcurve(adt, fcu, 1, 0);
           }
           else {
-            posttrans_fcurve_clean(fcu, false); /* only use handles in graph editor */
+            posttrans_fcurve_clean(fcu, SELECT, false); /* only use handles in graph editor */
           }
         }
       }
@@ -2103,11 +2109,11 @@ void special_aftertrans_update(bContext *C, TransInfo *t)
         if ((sipo->flag & SIPO_NOTRANSKEYCULL) == 0 && ((canceled == 0) || (duplicate))) {
           if (adt) {
             ANIM_nla_mapping_apply_fcurve(adt, fcu, 0, 0);
-            posttrans_fcurve_clean(fcu, use_handle);
+            posttrans_fcurve_clean(fcu, BEZT_FLAG_TEMP_TAG, use_handle);
             ANIM_nla_mapping_apply_fcurve(adt, fcu, 1, 0);
           }
           else {
-            posttrans_fcurve_clean(fcu, use_handle);
+            posttrans_fcurve_clean(fcu, BEZT_FLAG_TEMP_TAG, use_handle);
           }
         }
       }
diff --git a/source/blender/editors/transform/transform_convert_graph.c b/source/blender/editors/transform/transform_convert_graph.c
index f3d7592127c..f6f982e854a 100644
--- a/source/blender/editors/transform/transform_convert_graph.c
+++ b/source/blender/editors/transform/transform_convert_graph.c
@@ -1,4 +1,4 @@
-/*
+/*
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version 2
@@ -156,21 +156,55 @@ static bool graph_edit_use_local_center(TransInfo *t)
   return ((t->around == V3D_AROUND_LOCAL_ORIGINS) && (graph_edit_is_translation_mode(t) == false));
 }
 
+/**
+ * Get the effective selection of a triple for transform, i.e. return if the left handle, right
+ * handle and/or the center point should be affected by transform.
+ */
+static void graph_bezt_get_transform_selection(const TransInfo *t,
+                                               const BezTriple *bezt,
+                                               const bool use_handle,
+                                               bool *r_left_handle,
+                                               bool *r_key,
+                                               bool *r_right_handle)
+{
+  SpaceGraph *sipo = (SpaceGraph *)t->sa->spacedata.first;
+  bool key = (bezt->f2 & SELECT) != 0;
+  bool left = use_handle ? ((bezt->f1 & SELECT) != 0) : key;
+  bool right = use_handle ? ((bezt->f3 & SELECT) != 0) : key;
+
+  if (use_handle && t->is_launch_event_tweak) {
+    if (sipo->runtime.flag & SIPO_RUNTIME_FLAG_TWEAK_HANDLES_LEFT) {
+      key = right = false;
+    }
+    else if (sipo->runtime.flag & SIPO_RUNTIME_FLAG_TWEAK_HANDLES_RIGHT) {
+      left = key = false;
+    }
+  }
+
+  /* Whenever we move the key, we also move both handles. */
+  if (key) {
+    left = right = true;
+  }
+
+  *r_key = key;
+  *r_left_handle = left;
+  *r_right_handle = right;
+}
+
 static void graph_key_shortest_dist(
     TransInfo *t, FCurve *fcu, TransData *td_start, TransData *td, int cfra, bool use_handle)
 {
   int j = 0;
   TransData *td_iter = td_start;
+  bool sel_key, sel_left, sel_right;
 
   td->dist = FLT_MAX;
   for (; j < fcu->totvert; j++) {
     BezTriple *bezt = fcu->bezt + j;
     if (FrameOnMouseSide(t->frame_side, bezt->vec[1][0], cfra)) {
-      const bool sel2 = (bezt->f2 & SELECT) != 0;
-      const bool sel1 = use_handle ? (bezt->f1 & SELECT) != 0 : sel2;
-      const bool sel3 = use_handle ? (bezt->f3 & SELECT) != 0 : sel2;
+      graph_bezt_get_transform_selection(t, bezt, use_handle, &sel_left, &sel_key, &sel_right);
 
-      if (sel1 || sel2 || sel3) {
+      if (sel_left || sel_key || sel_right) {
         td->dist = td->rdist = min_ff(td->dist, fabs(td_iter->center[0] - td->center[0]));
       }
 
@@ -179,6 +213,15 @@ static void graph_key_shortest_dist(
   }
 }
 
+/**
+ * It is important to note that this doesn't always act on the selection (like it's usually done),
+ * it acts on a subset of it. E.g. the selection code may leave a hint that we just dragged on a
+ * left or right handle (SIPO_RUNTIME_FLAG_TWEAK_HANDLES_LEFT/RIGHT) and then we only transform the
+ * selected left or right handles accordingly.
+ * The points to be transformed are tagged with BEZT_FLAG_TEMP_TAG; some lower level curve
+ * functions may need to be made aware of this. It's ugly that these act based on selection state
+ * anyway.
+ */
 void createTransGraphEditData(bContext *C, TransInfo *t)
 {
   SpaceGraph *sipo = (SpaceGraph *)t->sa->spacedata.first;
@@ -198,11 +241,11 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
   BezTriple *bezt;
   int count = 0, i;
   float mtx[3][3], smtx[3][3];
-  const bool is_translation_mode = graph_edit_is_translation_mode(t);
   const bool use_handle = !(sipo->flag & SIPO_NOHANDLES);
   const bool use_local_center = graph_edit_use_local_center(t);
   const bool is_prop_edit = (t->flag & T_PROP_EDIT) != 0;
   short anim_map_flag = ANIM_UNITCONV_ONLYSEL | ANIM_UNITCONV_SELVERTS;
+  bool sel_key, sel_left, sel_right;
 
   /* determine what type of data we are operating on */
   if (ANIM_animdata_get_context(C, &ac) == 0) {
@@ -253,33 +296,29 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
       cfra = (float)CFRA;
     }
 
-    /* Only include BezTriples whose 'keyframe'
-     * occurs on the same side of the current frame as mouse. */
     for (i = 0, bezt = fcu->bezt; i < fcu->totvert; i++, bezt++) {
+      /* Only include BezTriples whose 'keyframe'
+       * occurs on the same side of the current frame as mouse. */
       if (FrameOnMouseSide(t->frame_side, bezt->vec[1][0], cfra)) {
-        const bool sel2 = (bezt->f2 & SELECT) != 0;
-        const bool sel1 = use_handle ? (bezt->f1 & SELECT) != 0 : sel2;
-        const bool sel3 = use_handle ? (bezt->f3 & SELECT) != 0 : sel2;
+        graph_bezt_get_transform_selection(t, bezt, use_handle, &sel_left, &sel_key, &sel_right);
 
         if (is_prop_edit) {
           curvecount += 3;
-          if (sel2 || sel1 || sel3) {
+          if (sel_key || sel_left || sel_right) {
             selected = true;
           }
         }
         else {
-          if (!is_translation_mode || !(sel2)) {
-            if (sel1) {
-              count++;
-            }
+          if (sel_left) {
+            count++;
+          }
 
-            if (sel3) {
-              count++;
-            }
+          if (sel_right) {
+            count++;
           }
 
           /* only include main vert if selected */
-          if (sel2 && !use_local_center) {
+          if (sel_key && !use_local_center) {
             count++;
           }
         }
@@ -366,19 +405,21 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
     unit_scale = ANIM_unit_mapping_get_factor(
         ac.scene, ale->id, ale->key_data, anim_map_flag, &offset);
 
-    /* only include BezTriples whose 'keyframe' occurs on the same side
-     * of the current frame as mouse (if applicable) */
     for (i = 0, bezt = fcu->bezt; i < fcu->totvert; i++, bezt++) {
-      if (FrameOnMouseSide(t->frame_side, bezt->vec[1][0], cfra)) {
-        const bool sel2 = (bezt->f2 & SELECT) != 0;
-        const bool sel1 = use_handle ? (bezt->f1 & SELECT) != 0 : sel2;
-        const bool sel3 = use_handle ? (bezt->f3 & SELECT) != 0 : sel2;
+      /* Ensure temp flag is cleared for all triples, we use it. */
+      bezt->f1 &= ~BEZT_FLAG_TEMP_TAG;
+      bezt->f2 &= ~BEZT_FLAG_TEMP_TAG;
+      bezt->f3 &= ~BEZT_FLAG_TEMP_TAG;
 
+      /* only include BezTriples whose 'keyframe' occurs on the same side
+       * of the current frame as mouse (if applicable) */
+      if (FrameOnMouseSide(t->frame_side, bezt->vec[1][0], cfra)) {
         TransDataCurveHandleFlags *hdata = NULL;
-        /* short h1=1, h2=1; */ /* UNUSED */
+
+        graph_bezt_get_transform_selection(t, bezt, use_handle, &sel_left, &sel_key, &sel_right);
 
         if (is_prop_edit) {
-          bool is_sel = (sel2 || sel1 || sel3);
+          bool is_sel = (sel_key || sel_left || sel_right);
           /* we always select all handles for proportional editing if central handle is selected */
           initTransDataCurveHandles(td, bezt);
           bezt_to_transdata(td++,
@@ -422,69 +463,69 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
                             smtx,
                             unit_scale,
                             offset);
+
+          if (is_sel) {
+            bezt->f1 |= BEZT_FLAG_TEMP_TAG;
+            bezt->f2 |= BEZT_FLAG_TEMP_TAG;
+            bezt->f3 |= BEZT_FLAG_TEMP_TAG;
+          }
         }
         else {
           /* only include handles if selected, irrespective of the interpolation modes.
            * also, only treat handles specially if the center point isn't selected.
            */
-          if (!is_translation_mode || !(sel2)) {
-            if (sel1) {
-              hdata = initTransDataCurveHandles(td, bezt);
-              bezt_to_transdata(td++,
-                                td2d++,
-                                tdg++,
-                                adt,
-                                bezt,
-                                0,
-                                sel1,
-                                true,
-                                intvals,
-                                mtx,
-                                smtx,
-                                unit_scale,
-                                offset);
-            }
-            else {
-              /* h1 = 0; */ /* UNUSED */
-            }
+          if (sel_left) {
+            hdata = initTransDataCurveHandles(td, bezt);
+            bezt_to_transdata(td++,
+                              td2d++,
+                              tdg++,
+                              adt,
+                              bezt,
+                              0,
+                              sel_left,
+                              true,
+                              intvals,
+                              mtx,
+                              smtx,
+                              unit_scale,
+                              offset);
+            bezt->f1 |= BEZT_FLAG_TEMP_TAG;
+          }
 
-            if (sel3) {
-              if (hdata == NULL) {
-                hdata = initTransDataCurveHandles(td, bezt);
-              }
-              bezt_to_transdata(td++,
-                                td2d++,
-                                tdg++,
-                                adt,
-                                bezt,
-                                2,
-                                sel3,
-                                true,
-                                intvals,
-                                mtx,
-                                smtx,
-                                unit_scale,
-                                offset);
-            }
-            else {
-              /* h2 = 0; */ /* UNUSED */
+          if (sel_right) {
+            if (hdata == NULL) {
+              hdata = initTransDataCurveHandles(td, bezt);
             }
+            bezt_to_transdata(td++,
+                              td2d++,
+                              tdg++,
+                              adt,
+                              bezt,
+                              2,
+                              sel_right,
+                              true,
+                              intvals,
+                              mtx,
+                              smtx,
+                              unit_scale,
+                              offset);
+            bezt->f3 |= BEZT_FLAG_TEMP_TAG;
           }
 
           /* only include main vert if selected */
-          if (sel2 && !use_local_center) {
+          if (sel_key && !use_local_center) {
             /* move handles relative to center */
-            if (is_translation_mode) {
-              if (sel1) {
+            if (graph_edit_is_translation_mode(t)) {
+              if (sel_left) {
                 td->flag |= TD_MOVEHANDLE1;
               }
-              if (sel3) {
+              if (sel_right) {
                 td->flag |= TD_MOVEHANDLE2;
               }
             }
 
             /* if handles were not selected, store their selection status */
-            if (!(sel1) || !(sel3)) {
+            if (!(sel_left) || !(sel_right)) {
               if (hdata == NULL) {
                 hdata = initTransDataCurveHandles(td, bezt);
               }
@@ -496,13 +537,14 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
                               adt,
                               bezt,
                               1,
-                              sel2,
+                              sel_key,
                               false,
                               intvals,
                               mtx,
                               smtx,
                               unit_scale,
                               offset);
+            bezt->f2 |= BEZT_FLAG_TEMP_TAG;
           }
           /* Special hack (must be done after #initTransDataCurveHandles(),
            * as that stores handle settings to restore...):
@@ -513,7 +555,7 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
            */
           if (ELEM(bezt->h1, HD_AUTO, HD_AUTO_ANIM) && ELEM(bezt->h2, HD_AUTO, HD_AUTO_ANIM) &&
               ELEM(t->mode, TFM_ROTATION, TFM_RESIZE)) {
-            if (hdata && (sel1) && (sel3)) {
+            if (hdata && (sel_left) && (sel_right)) {
               bezt->h1 = HD_ALIGN;
               bezt->h2 = HD_ALIGN;
             }
@@ -523,7 +565,7 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
     }
 
     /* Sets handles based on the selection */
-    testhandles_fcurve(fcu, use_handle);
+    testhandles_fcurve(fcu, BEZT_FLAG_TEMP_TAG, use_handle);
   }
 
   if (is_prop_edit) {
@@ -551,15 +593,13 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
         cfra = (float)CFRA;
       }
 
-      /* only include BezTriples whose 'keyframe' occurs on the
-       * same side of the current frame as mouse (if applicable) */
       for (i = 0, bezt = fcu->bezt; i < fcu->totvert; i++, bezt++) {
+        /* only include BezTriples whose 'keyframe' occurs on the
+         * same side of the current frame as mouse (if applicable) */
         if (FrameOnMouseSide(t->frame_side, bezt->vec[1][0], cfra)) {
-          const bool sel2 = (bezt->f2 & SELECT) != 0;
-          const bool sel1 = use_handle ? (bezt->f1 & SELECT) != 0 : sel2;
-          const bool sel3 = use_handle ? (bezt->f3 & SELECT) != 0 : sel2;
+          graph_bezt_get_transform_selection(t, bezt, use_handle, &sel_left, &sel_key, &sel_right);
 
-          if (sel1 || sel2) {
+          if (sel_left || sel_key) {
             td->dist = td->rdist = 0.0f;
           }
           else {
@@ -567,7 +607,7 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
           }
           td++;
 
-          if (sel2) {
+          if (sel_key) {
             td->dist = td->rdist = 0.0f;
           }
           else {
@@ -575,7 +615,7 @@ void createTransGraphEditData(bContext *C, TransInfo *t)
           }
           td++;
 
-          if (sel3 || sel2) {
+          if (sel_right || sel_key) {
             td->dist = td->rdist = 0.0f;
           }
           else {
diff --git a/source/blender/editors/transform/transform_generics.c b/source/blender/editors/transform/transform_generics.c
index 0df0b70a56a..548944e2d74 100644
--- a/source/blender/editors/transform/transform_generics.c
+++ b/source/blender/editors/transform/transform_generics.c
@@ -448,7 +448,7 @@ static void recalcData_graphedit(TransInfo *t)
       dosort++;
     }
     else {
-      calchandles_fcurve(fcu);
+      calchandles_fcurve_ex(fcu, BEZT_FLAG_TEMP_TAG);
     }
 
     /* set refresh tags for objects using this animation,