Fix T76595: Indicate the Active Keyframe in Graph Editor

Currently there is a panel that says "Active Keyframe" for numerically editing one keyframe's values, but in the code there is no concept of the "active keyframe."

This patch adds an "active keyframe index" to each FCurve, and displays it with a theme color for the active vertex (which didn't exist before) if the FCurve is active.

{F8536092 size=full}

The active keyframe is not currently set for select operations other than basic click-select, which mirrors the behavior in the 3D view.

Reviewed By: Severin, looch

Maniphest Tasks: T76595

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

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),

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)? */

source/blender/blenkernel/intern/fcurve.c

@@ -829,6 +829,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
  * \{ */

source/blender/blenkernel/intern/fcurve_test.cc

@@ -273,4 +273,38 @@ 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(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

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

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--;

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
    */

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,

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;

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;
+  /* The active keyframe should be selected. */
+  BLI_assert(BEZT_ISSEL_ANY(&fcu->bezt[active_keyframe_index]));
 
-      return true;
-    }
-  }
+  *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 */

source/blender/editors/space_graph/graph_draw.c

@@ -211,6 +211,35 @@ 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 bool edit,
+                                      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 +249,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, edit, pos);
 
   immUnbindProgram();
 }
@@ -270,6 +300,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 +345,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 +1288,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);
 }

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);
   }

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). */

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];

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", "");