Fix T76595: Indicate the Active Keyframe in Graph Editor

In the graph editor there is a panel that says "Active Keyframe" for
numerically editing a keyframe's values, but in the code there is no
concept of the "active keyframe." Since this is a useful concept to
have for some other features anyway, this commit adds an active
keyframe index value to FCurves. It also displays it with a theme
color for the active vertex (which didn't exist before) if the
FCurve is active.

The active keyframe in the graph editor is treated similarly to the
active vertex in the 3D view. It is the keyframe most recently selected
with a single click, and it is always selected.

For now, the only real functional change is that the active keyframe
appears in white and it should be more predictable which keyframe is
being edited in the sidebar panel.

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

15 files changed, 191 insertions, 31 deletions

diff --git a/release/datafiles/userdef/userdef_default_theme.c b/release/datafiles/userdef/userdef_default_theme.c
index d3175a6fa56..dc81e4c70ac 100644
--- a/release/datafiles/userdef/userdef_default_theme.c
+++ b/release/datafiles/userdef/userdef_default_theme.c
@@ -439,6 +439,7 @@ const bTheme U_theme_default = {
     .group_active = RGBA(0x368024ff),
     .vertex = RGBA(0x000000ff),
     .vertex_select = RGBA(0xff8500ff),
+    .vertex_active = RGBA(0xffffffff),
     .cframe = RGBA(0x5680c2ff),
     .time_scrub_background = RGBA(0x292929e6),
     .time_marker_line = RGBA(0x00000060),
diff --git a/source/blender/blenkernel/BKE_fcurve.h b/source/blender/blenkernel/BKE_fcurve.h
index 9b6189612ba..e42d1cbbc78 100644
--- a/source/blender/blenkernel/BKE_fcurve.h
+++ b/source/blender/blenkernel/BKE_fcurve.h
@@ -244,6 +244,9 @@ bool BKE_fcurve_calc_bounds(struct FCurve *fcu,
                             const bool do_sel_only,
                             const bool include_handles);
 
+void BKE_fcurve_active_keyframe_set(struct FCurve *fcu, const struct BezTriple *active_bezt);
+int BKE_fcurve_active_keyframe_index(const struct FCurve *fcu);
+
 /* .............. */
 
 /* Are keyframes on F-Curve of any use (to final result, and to show in editors)? */
diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c
index d0f23221ed0..856e0e872b3 100644
--- a/source/blender/blenkernel/intern/fcurve.c
+++ b/source/blender/blenkernel/intern/fcurve.c
@@ -830,6 +830,38 @@ bool BKE_fcurve_calc_range(
 /** \} */
 
 /* -------------------------------------------------------------------- */
+/** \name Active Keyframe
+ * \{ */
+
+/**
+ * Set the index that stores the FCurve's active keyframe, assuming that \a active_bezt
+ * is already part of `fcu->bezt`. If NULL, set active keyframe index to "none."
+ */
+void BKE_fcurve_active_keyframe_set(FCurve *fcu, const BezTriple *active_bezt)
+{
+  fcu->active_keyframe_index = (active_bezt == NULL) ? FCURVE_ACTIVE_KEYFRAME_NONE :
+                                                       active_bezt - fcu->bezt;
+}
+
+/**
+ * Get the active keyframe index, with sanity checks for point bounds.
+ */
+int BKE_fcurve_active_keyframe_index(const FCurve *fcu)
+{
+  const int active_keyframe_index = fcu->active_keyframe_index;
+
+  /* Sanity checks. */
+  if ((fcu->bezt == NULL) || (active_keyframe_index >= fcu->totvert) ||
+      (active_keyframe_index < 0)) {
+    return FCURVE_ACTIVE_KEYFRAME_NONE;
+  }
+
+  return active_keyframe_index;
+}
+
+/** \} */
+
+/* -------------------------------------------------------------------- */
 /** \name Status Checks
  * \{ */
 
diff --git a/source/blender/blenkernel/intern/fcurve_test.cc b/source/blender/blenkernel/intern/fcurve_test.cc
index dd672df744b..a58f519adfb 100644
--- a/source/blender/blenkernel/intern/fcurve_test.cc
+++ b/source/blender/blenkernel/intern/fcurve_test.cc
@@ -273,4 +273,39 @@ TEST(fcurve_subdivide, BKE_bezt_subdivide_handles)
   BKE_fcurve_free(fcu);
 }
 
+TEST(fcurve_active_keyframe, ActiveKeyframe)
+{
+  FCurve *fcu = BKE_fcurve_create();
+
+  /* There should be no active keyframe with no points. */
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), FCURVE_ACTIVE_KEYFRAME_NONE);
+
+  /* Check that adding new points sets the active index. */
+  EXPECT_EQ(insert_vert_fcurve(fcu, 1.0f, 7.5f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF), 0);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 0);
+  EXPECT_EQ(insert_vert_fcurve(fcu, 8.0f, 15.0f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF), 1);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 1);
+  EXPECT_EQ(insert_vert_fcurve(fcu, 14.0f, 8.2f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF), 2);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 2);
+
+  /* Check clearing the index. */
+  BKE_fcurve_active_keyframe_set(fcu, NULL);
+  EXPECT_EQ(fcu->active_keyframe_index, FCURVE_ACTIVE_KEYFRAME_NONE);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), FCURVE_ACTIVE_KEYFRAME_NONE);
+
+  /* Check a "normal" action. */
+  BKE_fcurve_active_keyframe_set(fcu, &fcu->bezt[2]);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), 2);
+
+  /* Check out of bounds. */
+  BKE_fcurve_active_keyframe_set(fcu, fcu->bezt - 20);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), FCURVE_ACTIVE_KEYFRAME_NONE);
+
+  /* Check out of bounds again. */
+  BKE_fcurve_active_keyframe_set(fcu, fcu->bezt + 4);
+  EXPECT_EQ(BKE_fcurve_active_keyframe_index(fcu), FCURVE_ACTIVE_KEYFRAME_NONE);
+
+  BKE_fcurve_free(fcu);
+}
+
 }  // namespace blender::bke::tests
diff --git a/source/blender/blenloader/intern/versioning_userdef.c b/source/blender/blenloader/intern/versioning_userdef.c
index 04302935deb..e81735c698d 100644
--- a/source/blender/blenloader/intern/versioning_userdef.c
+++ b/source/blender/blenloader/intern/versioning_userdef.c
@@ -253,6 +253,8 @@ static void do_versions_theme(const UserDef *userdef, bTheme *btheme)
    */
   {
     /* Keep this block, even when empty. */
+
+    FROM_DEFAULT_V4_UCHAR(space_graph.vertex_active);
   }
 
 #undef FROM_DEFAULT_V4_UCHAR
diff --git a/source/blender/editors/animation/keyframes_general.c b/source/blender/editors/animation/keyframes_general.c
index ea032446fc6..0bec20c739e 100644
--- a/source/blender/editors/animation/keyframes_general.c
+++ b/source/blender/editors/animation/keyframes_general.c
@@ -117,6 +117,9 @@ bool delete_fcurve_keys(FCurve *fcu)
   /* Delete selected BezTriples */
   for (int i = 0; i < fcu->totvert; i++) {
     if (fcu->bezt[i].f2 & SELECT) {
+      if (i == fcu->active_keyframe_index) {
+        BKE_fcurve_active_keyframe_set(fcu, NULL);
+      }
       memmove(&fcu->bezt[i], &fcu->bezt[i + 1], sizeof(BezTriple) * (fcu->totvert - i - 1));
       fcu->totvert--;
       i--;
diff --git a/source/blender/editors/animation/keyframing.c b/source/blender/editors/animation/keyframing.c
index ed3e236fed3..e4b73ea65a0 100644
--- a/source/blender/editors/animation/keyframing.c
+++ b/source/blender/editors/animation/keyframing.c
@@ -606,6 +606,8 @@ int insert_vert_fcurve(
   /* add temp beztriple to keyframes */
   a = insert_bezt_fcurve(fcu, &beztr, flag);
 
+  fcu->active_keyframe_index = a;
+
   /* what if 'a' is a negative index?
    * for now, just exit to prevent any segfaults
    */
diff --git a/source/blender/editors/include/UI_resources.h b/source/blender/editors/include/UI_resources.h
index 31cb62117c5..a7b87d38472 100644
--- a/source/blender/editors/include/UI_resources.h
+++ b/source/blender/editors/include/UI_resources.h
@@ -92,6 +92,7 @@ typedef enum ThemeColorID {
   TH_TRANSFORM,
   TH_VERTEX,
   TH_VERTEX_SELECT,
+  TH_VERTEX_ACTIVE,
   TH_VERTEX_UNREFERENCED,
   TH_VERTEX_SIZE,
   TH_OUTLINE_WIDTH,
diff --git a/source/blender/editors/interface/resources.c b/source/blender/editors/interface/resources.c
index ec6f99f167d..e55f84916b3 100644
--- a/source/blender/editors/interface/resources.c
+++ b/source/blender/editors/interface/resources.c
@@ -362,6 +362,9 @@ const uchar *UI_ThemeGetColorPtr(bTheme *btheme, int spacetype, int colorid)
         case TH_VERTEX_SELECT:
           cp = ts->vertex_select;
           break;
+        case TH_VERTEX_ACTIVE:
+          cp = ts->vertex_active;
+          break;
         case TH_VERTEX_BEVEL:
           cp = ts->vertex_bevel;
           break;
diff --git a/source/blender/editors/space_graph/graph_buttons.c b/source/blender/editors/space_graph/graph_buttons.c
index 45c43a5853d..bbdca5280d4 100644
--- a/source/blender/editors/space_graph/graph_buttons.c
+++ b/source/blender/editors/space_graph/graph_buttons.c
@@ -235,40 +235,27 @@ static void graph_panel_properties(const bContext *C, Panel *panel)
 /* ******************* active Keyframe ************** */
 
 /* get 'active' keyframe for panel editing */
-static bool get_active_fcurve_keyframe_edit(FCurve *fcu,
+static bool get_active_fcurve_keyframe_edit(const FCurve *fcu,
                                             BezTriple **r_bezt,
                                             BezTriple **r_prevbezt)
 {
-  BezTriple *b;
-  int i;
-
   /* zero the pointers */
   *r_bezt = *r_prevbezt = NULL;
 
-  /* sanity checks */
-  if ((fcu->bezt == NULL) || (fcu->totvert == 0)) {
+  const int active_keyframe_index = BKE_fcurve_active_keyframe_index(fcu);
+  if (active_keyframe_index == FCURVE_ACTIVE_KEYFRAME_NONE) {
     return false;
   }
 
-  /* find first selected keyframe for now, and call it the active one
-   * - this is a reasonable assumption, given that whenever anyone
-   *   wants to edit numerically, there is likely to only be 1 vert selected
-   */
-  for (i = 0, b = fcu->bezt; i < fcu->totvert; i++, b++) {
-    if (BEZT_ISSEL_ANY(b)) {
-      /* found
-       * - 'previous' is either the one before, of the keyframe itself (which is still fine)
-       *   XXX: we can just make this null instead if needed
-       */
-      *r_prevbezt = (i > 0) ? b - 1 : b;
-      *r_bezt = b;
-
-      return true;
-    }
-  }
+  /* The active keyframe should be selected. */
+  BLI_assert(BEZT_ISSEL_ANY(&fcu->bezt[active_keyframe_index]));
+
+  *r_bezt = &fcu->bezt[active_keyframe_index];
+  /* Previous is either one before the active, or the point itself if it's the first. */
+  const int prev_index = max_ii(active_keyframe_index - 1, 0);
+  *r_prevbezt = &fcu->bezt[prev_index];
 
-  /* not found */
-  return false;
+  return true;
 }
 
 /* update callback for active keyframe properties - base updates stuff */
diff --git a/source/blender/editors/space_graph/graph_draw.c b/source/blender/editors/space_graph/graph_draw.c
index af7a7d9a4de..c6acc8260b7 100644
--- a/source/blender/editors/space_graph/graph_draw.c
+++ b/source/blender/editors/space_graph/graph_draw.c
@@ -211,6 +211,32 @@ static void draw_fcurve_selected_keyframe_vertices(
   immEnd();
 }
 
+/**
+ * Draw the extra indicator for the active point.
+ */
+static void draw_fcurve_active_vertex(const FCurve *fcu, const View2D *v2d, const uint pos)
+{
+  const int active_keyframe_index = BKE_fcurve_active_keyframe_index(fcu);
+  if (!(fcu->flag & FCURVE_ACTIVE) || active_keyframe_index == FCURVE_ACTIVE_KEYFRAME_NONE) {
+    return;
+  }
+
+  const float fac = 0.05f * BLI_rctf_size_x(&v2d->cur);
+  const BezTriple *bezt = &fcu->bezt[active_keyframe_index];
+
+  if (!IN_RANGE(bezt->vec[1][0], (v2d->cur.xmin - fac), (v2d->cur.xmax + fac))) {
+    return;
+  }
+  if (!(bezt->f2 & SELECT)) {
+    return;
+  }
+
+  immBegin(GPU_PRIM_POINTS, 1);
+  immUniformThemeColor(TH_VERTEX_ACTIVE);
+  immVertex2fv(pos, bezt->vec[1]);
+  immEnd();
+}
+
 /* helper func - draw keyframe vertices only for an F-Curve */
 static void draw_fcurve_keyframe_vertices(FCurve *fcu, View2D *v2d, bool edit, uint pos)
 {
@@ -220,6 +246,7 @@ static void draw_fcurve_keyframe_vertices(FCurve *fcu, View2D *v2d, bool edit, u
 
   draw_fcurve_selected_keyframe_vertices(fcu, v2d, edit, false, pos);
   draw_fcurve_selected_keyframe_vertices(fcu, v2d, edit, true, pos);
+  draw_fcurve_active_vertex(fcu, v2d, pos);
 
   immUnbindProgram();
 }
@@ -270,6 +297,39 @@ static void draw_fcurve_selected_handle_vertices(
   immEnd();
 }
 
+/**
+ * Draw the extra handles for the active point.
+ */
+static void draw_fcurve_active_handle_vertices(const FCurve *fcu,
+                                               const bool sel_handle_only,
+                                               const uint pos)
+{
+  const int active_keyframe_index = BKE_fcurve_active_keyframe_index(fcu);
+  if (!(fcu->flag & FCURVE_ACTIVE) || active_keyframe_index == FCURVE_ACTIVE_KEYFRAME_NONE) {
+    return;
+  }
+
+  const BezTriple *bezt = &fcu->bezt[active_keyframe_index];
+
+  if (sel_handle_only && !BEZT_ISSEL_ANY(bezt)) {
+    return;
+  }
+
+  float active_col[4];
+  UI_GetThemeColor4fv(TH_VERTEX_ACTIVE, active_col);
+  immUniform4fv("outlineColor", active_col);
+  immUniformColor3fvAlpha(active_col, 0.01f); /* Almost invisible - only keep for smoothness. */
+  immBeginAtMost(GPU_PRIM_POINTS, 2);
+
+  if ((bezt->f1 & SELECT)) {
+    immVertex2fv(pos, bezt->vec[0]);
+  }
+  if ((bezt->f3 & SELECT)) {
+    immVertex2fv(pos, bezt->vec[2]);
+  }
+  immEnd();
+}
+
 /* helper func - draw handle vertices only for an F-Curve (if it is not protected) */
 static void draw_fcurve_handle_vertices(FCurve *fcu, View2D *v2d, bool sel_handle_only, uint pos)
 {
@@ -282,6 +342,7 @@ static void draw_fcurve_handle_vertices(FCurve *fcu, View2D *v2d, bool sel_handl
 
   draw_fcurve_selected_handle_vertices(fcu, v2d, false, sel_handle_only, pos);
   draw_fcurve_selected_handle_vertices(fcu, v2d, true, sel_handle_only, pos);
+  draw_fcurve_active_handle_vertices(fcu, sel_handle_only, pos);
 
   immUnbindProgram();
 }
@@ -1224,10 +1285,22 @@ void graph_draw_curves(bAnimContext *ac, SpaceGraph *sipo, ARegion *region, shor
    * draw curve, then handle-lines, and finally vertices in this order so that
    * the data will be layered correctly
    */
+  bAnimListElem *ale_active_fcurve = NULL;
   for (ale = anim_data.first; ale; ale = ale->next) {
+    const FCurve *fcu = (FCurve *)ale->key_data;
+    if (fcu->flag & FCURVE_ACTIVE) {
+      ale_active_fcurve = ale;
+      continue;
+    }
     draw_fcurve(ac, sipo, region, ale);
   }
 
+  /* Draw the active FCurve last so that it (especially the active keyframe)
+   * shows on top of the other curves. */
+  if (ale_active_fcurve != NULL) {
+    draw_fcurve(ac, sipo, region, ale_active_fcurve);
+  }
+
   /* free list of curves */
   ANIM_animdata_freelist(&anim_data);
 }
diff --git a/source/blender/editors/space_graph/graph_select.c b/source/blender/editors/space_graph/graph_select.c
index 0c05942ec4b..433d7d6774b 100644
--- a/source/blender/editors/space_graph/graph_select.c
+++ b/source/blender/editors/space_graph/graph_select.c
@@ -1485,7 +1485,6 @@ static int mouse_graph_keys(bAnimContext *ac,
     /* only if there's keyframe */
     if (nvi->bezt) {
       bezt = nvi->bezt; /* Used to check `bezt` selection is set. */
-      /* depends on selection mode */
       if (select_mode == SELECT_INVERT) {
         if (nvi->hpoint == NEAREST_HANDLE_KEY) {
           bezt->f2 ^= SELECT;
@@ -1510,6 +1509,10 @@ static int mouse_graph_keys(bAnimContext *ac,
           bezt->f3 |= SELECT;
         }
       }
+
+      if (!run_modal && BEZT_ISSEL_ANY(bezt) && !already_selected) {
+        BKE_fcurve_active_keyframe_set(nvi->fcu, bezt);
+      }
     }
     else if (nvi->fpt) {
       // TODO: need to handle sample points
@@ -1555,10 +1558,11 @@ static int mouse_graph_keys(bAnimContext *ac,
     }
   }
 
-  /* set active F-Curve (NOTE: sync the filter flags with findnearest_fcurve_vert) */
-  /* needs to be called with (sipo->flag & SIPO_SELCUVERTSONLY)
+  /* Set active F-Curve, except when dragging the selected keys.
+   * needs to be called with (sipo->flag & SIPO_SELCUVERTSONLY)
    * otherwise the active flag won't be set T26452. */
-  if (nvi->fcu->flag & FCURVE_SELECTED) {
+  if (!run_modal && nvi->fcu->flag & FCURVE_SELECTED) {
+    /* NOTE: Sync the filter flags with findnearest_fcurve_vert. */
     int filter = (ANIMFILTER_DATA_VISIBLE | ANIMFILTER_CURVE_VISIBLE | ANIMFILTER_NODUPLIS);
     ANIM_set_active_channel(ac, ac->data, ac->datatype, filter, nvi->fcu, nvi->ctype);
   }
diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h
index 7cb9978f768..a13ed0b2924 100644
--- a/source/blender/makesdna/DNA_anim_types.h
+++ b/source/blender/makesdna/DNA_anim_types.h
@@ -550,6 +550,9 @@ typedef enum eDriver_Flags {
 
 /* F-Curves -------------------------------------- */
 
+/** When #active_keyframe_index is set to this, the FCurve does not have an active keyframe. */
+#define FCURVE_ACTIVE_KEYFRAME_NONE -1
+
 /**
  * FPoint (fpt)
  *
@@ -587,10 +590,15 @@ typedef struct FCurve {
   /** Total number of points which define the curve (i.e. size of arrays in FPoints). */
   unsigned int totvert;
 
+  /**
+   * Index of active keyframe in #bezt for numerical editing in the interface. A value of
+   * #FCURVE_ACTIVE_KEYFRAME_NONE indicates that the FCurve has no active keyframe.
+   */
+  int active_keyframe_index;
+
   /* value cache + settings */
   /** Value stored from last time curve was evaluated (not threadsafe, debug display only!). */
   float curval;
-  char _pad2[4];
   /** User-editable settings for this curve. */
   short flag;
   /** Value-extending mode for this curve (does not cover). */
diff --git a/source/blender/makesdna/DNA_userdef_types.h b/source/blender/makesdna/DNA_userdef_types.h
index f0a1378920e..c41a8bb9bc1 100644
--- a/source/blender/makesdna/DNA_userdef_types.h
+++ b/source/blender/makesdna/DNA_userdef_types.h
@@ -275,7 +275,8 @@ typedef struct ThemeSpace {
   unsigned char wire[4], wire_edit[4], select[4];
   unsigned char lamp[4], speaker[4], empty[4], camera[4];
   unsigned char active[4], group[4], group_active[4], transform[4];
-  unsigned char vertex[4], vertex_select[4], vertex_bevel[4], vertex_unreferenced[4];
+  unsigned char vertex[4], vertex_select[4], vertex_active[4], vertex_bevel[4],
+      vertex_unreferenced[4];
   unsigned char edge[4], edge_select[4];
   unsigned char edge_seam[4], edge_sharp[4], edge_facesel[4], edge_crease[4], edge_bevel[4];
   /** Solid faces. */
@@ -357,7 +358,7 @@ typedef struct ThemeSpace {
   unsigned char path_before[4], path_after[4];
   unsigned char path_keyframe_before[4], path_keyframe_after[4];
   unsigned char camera_path[4];
-  unsigned char _pad1[2];
+  unsigned char _pad1[6];
 
   unsigned char gp_vertex_size;
   unsigned char gp_vertex[4], gp_vertex_select[4];
diff --git a/source/blender/makesrna/intern/rna_userdef.c b/source/blender/makesrna/intern/rna_userdef.c
index 2e9838d689b..8d36b7dc08f 100644
--- a/source/blender/makesrna/intern/rna_userdef.c
+++ b/source/blender/makesrna/intern/rna_userdef.c
@@ -1895,6 +1895,11 @@ static void rna_def_userdef_theme_spaces_vertex(StructRNA *srna)
   RNA_def_property_ui_text(prop, "Vertex Select", "");
   RNA_def_property_update(prop, 0, "rna_userdef_theme_update");
 
+  prop = RNA_def_property(srna, "vertex_active", PROP_FLOAT, PROP_COLOR_GAMMA);
+  RNA_def_property_array(prop, 3);
+  RNA_def_property_ui_text(prop, "Active Vertex", "");
+  RNA_def_property_update(prop, 0, "rna_userdef_theme_update");
+
   prop = RNA_def_property(srna, "vertex_size", PROP_INT, PROP_PIXEL);
   RNA_def_property_range(prop, 1, 32);
   RNA_def_property_ui_text(prop, "Vertex Size", "");