Animation: add function for deduplicating FCurve keys

Introduce `BKE_fcurve_deduplicate_keys()` to merge keys that are on the
same time, or within the time comparison threshold (1/100th of a frame).

When merging two consecutive keys, the last one 'wins' and determines
the final key *value*. The first key's *time* is retained, to ensure the
reference point for the next comparisons is stable. The only exception
here is when there is a key exactly on an integer frame number, in which
case that one is preferred.

The function is exposed in RNA as `fcurve.keyframe_points.deduplicate()`

This commit also introduces a new function `BKE_fcurve_bezt_shrink(fcu,
new_totvert);` that can reallocate the `bezt` array to ensure removed
keys no longer take up memory.

The RNA function `fcurve.update()` currently performs two steps, which
are now exposed to RNA as well, as `keyframe_points.sort()` and
`keyframe_points.handles_recalc()`. This is so that Python code can
sort, deduplicate, and then recalculate the handles only once (calling
`update` + `deduplicate` would do the latter twice).

In Blender 4.0 the deduplication will also be part of `fcurve.update()`,
see #107126.

Reviewed on https://projects.blender.org/blender/blender/pulls/107089

scripts/modules/bpy_extras/anim_utils.py

@@ -571,4 +571,8 @@ class KeyframesCo:
             keyframe_points.foreach_set("co", co_buffer)
             keyframe_points.foreach_set("interpolation", ipo_buffer)
 
-            fcurve.update()
+            # TODO: in Blender 4.0 the next lines can be replaced with one call to `fcurve.update()`.
+            # See https://projects.blender.org/blender/blender/issues/107126 for more info.
+            keyframe_points.sort()
+            keyframe_points.deduplicate()
+            keyframe_points.handles_recalc()

source/blender/blenkernel/BKE_fcurve.h

@@ -416,6 +416,7 @@ int BKE_fcurve_active_keyframe_index(const struct FCurve *fcu);
  * Move the indexed keyframe to the given value,
  * and move the handles with it to ensure the slope remains the same.
  */
+void BKE_fcurve_keyframe_move_time_with_handles(BezTriple *keyframe, const float new_time);
 void BKE_fcurve_keyframe_move_value_with_handles(struct BezTriple *keyframe, float new_value);
 
 /* .............. */
@@ -472,6 +473,14 @@ bool BKE_fcurve_bezt_subdivide_handles(struct BezTriple *bezt,
                                        struct BezTriple *next,
                                        float *r_pdelta);
 
+/**
+ * Resize the FCurve 'bezt' array to fit the given length.
+ *
+ * \param new_totvert new number of elements in the FCurve's `bezt` array.
+ * Constraint: `0 <= new_totvert <= fcu->totvert`
+ */
+void BKE_fcurve_bezt_shrink(struct FCurve *fcu, int new_totvert);
+
 /**
  * Delete a keyframe from an F-curve at a specific index.
  */
@@ -499,6 +508,21 @@ void BKE_fcurve_merge_duplicate_keys(struct FCurve *fcu,
                                      const int sel_flag,
                                      const bool use_handle);
 
+/**
+ * Ensure the FCurve is a proper function, such that every X-coordinate of the
+ * timeline has only one value of the FCurve. In other words, removes duplicate
+ * keyframes.
+ *
+ * Contrary to #BKE_fcurve_merge_duplicate_keys, which is intended for
+ * interactive use, and where selection matters, this is a simpler deduplication
+ * where the last duplicate "wins".
+ *
+ * Assumes the keys are sorted (see #sort_time_fcurve).
+ *
+ * After deduplication, call `BKE_fcurve_handles_recalc(fcu);`
+ */
+void BKE_fcurve_deduplicate_keys(struct FCurve *fcu);
+
 /* -------- Curve Sanity -------- */
 
 /**

source/blender/blenkernel/intern/fcurve.c

@@ -883,6 +883,14 @@ int BKE_fcurve_active_keyframe_index(const FCurve *fcu)
 
 /** \} */
 
+void BKE_fcurve_keyframe_move_time_with_handles(BezTriple *keyframe, const float new_time)
+{
+  const float time_delta = new_time - keyframe->vec[1][0];
+  keyframe->vec[0][0] += time_delta;
+  keyframe->vec[1][0] = new_time;
+  keyframe->vec[2][0] += time_delta;
+}
+
 void BKE_fcurve_keyframe_move_value_with_handles(struct BezTriple *keyframe, const float new_value)
 {
   const float value_delta = new_value - keyframe->vec[1][1];
@@ -1659,6 +1667,24 @@ bool BKE_fcurve_bezt_subdivide_handles(struct BezTriple *bezt,
   return true;
 }
 
+void BKE_fcurve_bezt_shrink(struct FCurve *fcu, const int new_totvert)
+{
+  BLI_assert(new_totvert >= 0);
+  BLI_assert(new_totvert <= fcu->totvert);
+
+  /* No early return when new_totvert == fcu->totvert. There is no way to know the intention of the
+   * caller, nor the history of the FCurve so far, so `fcu->bezt` may actually have allocated space
+   * for more than `fcu->totvert` keys. */
+
+  if (new_totvert == 0) {
+    fcurve_bezt_free(fcu);
+    return;
+  }
+
+  fcu->bezt = MEM_reallocN(fcu->bezt, new_totvert * sizeof(*(fcu->bezt)));
+  fcu->totvert = new_totvert;
+}
+
 void BKE_fcurve_delete_key(FCurve *fcu, int index)
 {
   /* sanity check */
@@ -1849,6 +1875,52 @@ void BKE_fcurve_merge_duplicate_keys(FCurve *fcu, const int sel_flag, const bool
   BLI_freelistN(&retained_keys);
 }
 
+void BKE_fcurve_deduplicate_keys(FCurve *fcu)
+{
+  BLI_assert_msg(fcu->bezt, "this function only works with regular (non-sampled) FCurves");
+  if (fcu->totvert < 2 || fcu->bezt == NULL) {
+    return;
+  }
+
+  int prev_bezt_index = 0;
+  for (int i = 1; i < fcu->totvert; i++) {
+    BezTriple *bezt = &fcu->bezt[i];
+    BezTriple *prev_bezt = &fcu->bezt[prev_bezt_index];
+
+    const float bezt_x = bezt->vec[1][0];
+    const float prev_x = prev_bezt->vec[1][0];
+
+    if (bezt_x - prev_x <= BEZT_BINARYSEARCH_THRESH) {
+      /* Replace 'prev_bezt', as it has the same X-coord as 'bezt' and the last one wins. */
+      *prev_bezt = *bezt;
+
+      if (floor(bezt_x) == bezt_x) {
+        /* Keep the 'bezt_x' coordinate, as being on a frame is more desirable
+         * than being ever so slightly off. */
+      }
+      else {
+        /* Move the retained key to the old X-coordinate, to 'anchor' the X-coordinate used for
+         * subsequente comparisons. Without this, the reference X-coordinate would keep moving
+         * forward in time, potentially merging in more keys than desired. */
+        BKE_fcurve_keyframe_move_time_with_handles(prev_bezt, prev_x);
+      }
+      continue;
+    }
+
+    /* Next iteration should look at the current element. However, because of the deletions, that
+     * may not be at index 'i'; after this increment, `prev_bezt_index` points at where the current
+     * element should go. */
+    prev_bezt_index++;
+
+    if (prev_bezt_index != i) {
+      /* This bezt should be kept, so copy it to its new location in the array. */
+      fcu->bezt[prev_bezt_index] = *bezt;
+    }
+  }
+
+  BKE_fcurve_bezt_shrink(fcu, prev_bezt_index + 1);
+}
+
 /** \} */
 
 /* -------------------------------------------------------------------- */

source/blender/blenkernel/intern/fcurve_test.cc

@@ -348,6 +348,37 @@ TEST(BKE_fcurve, BKE_fcurve_keyframe_move_value_with_handles)
   BKE_fcurve_free(fcu);
 }
 
+TEST(BKE_fcurve, BKE_fcurve_keyframe_move_time_with_handles)
+{
+  FCurve *fcu = BKE_fcurve_create();
+
+  insert_vert_fcurve(fcu, 1.0f, 7.5f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF);
+  insert_vert_fcurve(fcu, 8.0f, 15.0f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF);
+  insert_vert_fcurve(fcu, 14.0f, 8.2f, BEZT_KEYTYPE_KEYFRAME, INSERTKEY_NO_USERPREF);
+
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][0], 5.2671194f);
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][1], 15.0f);
+
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][0], 8.0f);
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][1], 15.0f);
+
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][0], 10.342469f);
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][1], 15.0f);
+
+  BKE_fcurve_keyframe_move_time_with_handles(&fcu->bezt[1], 47.0f);
+
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][0], 44.2671194f) << "Left handle time should be updated";
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[0][1], 15.0f) << "Left handle should not move in value";
+
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][0], 47.0f) << "Frame time should have been updated";
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[1][1], 15.0f) << "Frame should not move in value";
+
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][0], 49.342469f) << "Right handle time should be updated";
+  EXPECT_FLOAT_EQ(fcu->bezt[1].vec[2][1], 15.0f) << "Right handle should not move in value";
+
+  BKE_fcurve_free(fcu);
+}
+
 TEST(BKE_fcurve, BKE_fcurve_calc_range)
 {
   FCurve *fcu = BKE_fcurve_create();
@@ -571,7 +602,7 @@ static FCurve *testcurve_with_duplicates()
   set_key(fcu, 3, 47.0f, 1.0f);
   set_key(fcu, 4, 7.0f, 2.0f);
   set_key(fcu, 5, 47.0f, 2.0f);
-  set_key(fcu, 6, 47.00001f, 3.0f);
+  set_key(fcu, 6, 47.0f + BEZT_BINARYSEARCH_THRESH, 3.0f);
   set_key(fcu, 7, 7.0f, 3.0f);
   set_key(fcu, 8, 3.0f, 1.0f);
   set_key(fcu, 9, 2.0f, 1.0f);
@@ -596,10 +627,79 @@ TEST(BKE_fcurve, sort_time_fcurve_stability)
   EXPECT_V2_NEAR(fcu->bezt[5].vec[1], float2(7.0f, 3.0f), 1e-3);
   EXPECT_V2_NEAR(fcu->bezt[6].vec[1], float2(47.0f, 1.0f), 1e-3);
   EXPECT_V2_NEAR(fcu->bezt[7].vec[1], float2(47.0f, 2.0f), 1e-3);
-  EXPECT_V2_NEAR(fcu->bezt[8].vec[1], float2(47.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[8].vec[1], float2(47.0f + BEZT_BINARYSEARCH_THRESH, 3.0f), 1e-3);
   EXPECT_V2_NEAR(fcu->bezt[9].vec[1], float2(327.16f, 1.0f), 1e-3);
 
   BKE_fcurve_free(fcu);
 }
 
+TEST(BKE_fcurve, BKE_fcurve_deduplicate_keys)
+{
+  FCurve *fcu = testcurve_with_duplicates();
+  ASSERT_EQ(fcu->totvert, 10);
+  sort_time_fcurve(fcu);
+
+  BKE_fcurve_deduplicate_keys(fcu);
+  ASSERT_GE(fcu->totvert, 6); /* Protect against out-of-bounds access. */
+  EXPECT_EQ(fcu->totvert, 6); /* The actual expected value. */
+  EXPECT_V2_NEAR(fcu->bezt[0].vec[1], float2(1.0f, 1.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[1].vec[1], float2(2.0f, 1.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[2].vec[1], float2(3.0f, 1.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[3].vec[1], float2(7.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[4].vec[1], float2(47.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[5].vec[1], float2(327.16f, 1.0f), 1e-3);
+
+  BKE_fcurve_free(fcu);
+}
+
+TEST(BKE_fcurve, BKE_fcurve_deduplicate_keys_edge_cases)
+{
+  FCurve *fcu = testcurve_with_duplicates();
+  ASSERT_EQ(fcu->totvert, 10);
+
+  /* Update the 2nd and 2nd-to-last keys to test the edge cases. */
+  set_key(fcu, 0, 1, 1);
+  set_key(fcu, 1, 1, 2);
+  set_key(fcu, 8, 327.16f, 1);
+  set_key(fcu, 9, 327.16f, 2);
+
+  sort_time_fcurve(fcu);
+
+  BKE_fcurve_deduplicate_keys(fcu);
+  ASSERT_EQ(fcu->totvert, 4);
+  EXPECT_V2_NEAR(fcu->bezt[0].vec[1], float2(1.0f, 2.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[1].vec[1], float2(7.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[2].vec[1], float2(47.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[3].vec[1], float2(327.16f, 2.0f), 1e-3);
+
+  BKE_fcurve_free(fcu);
+}
+
+TEST(BKE_fcurve, BKE_fcurve_deduplicate_keys_prefer_whole_frames)
+{
+  FCurve *fcu = testcurve_with_duplicates();
+  ASSERT_EQ(fcu->totvert, 10);
+
+  /* Update the first key around 47.0 to be slightly before the frame. This gives us three keys on
+   * 47-epsilon, 47, and 47+epsilon. The keys at index 5 and 6 already have this value, so the
+   * `set_key` calls are unnecessary, but this way this test has a more local overview of the
+   * situation under test. */
+  set_key(fcu, 3, 47.0f - BEZT_BINARYSEARCH_THRESH, 1.0f);
+  set_key(fcu, 5, 47.0f, 2.0f);
+  set_key(fcu, 6, 47.0f + BEZT_BINARYSEARCH_THRESH, 3.0f);
+
+  sort_time_fcurve(fcu);
+
+  BKE_fcurve_deduplicate_keys(fcu);
+  ASSERT_EQ(fcu->totvert, 6);
+  EXPECT_V2_NEAR(fcu->bezt[0].vec[1], float2(1.0f, 1.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[1].vec[1], float2(2.0f, 1.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[2].vec[1], float2(3.0f, 1.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[3].vec[1], float2(7.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[4].vec[1], float2(47.0f, 3.0f), 1e-3);
+  EXPECT_V2_NEAR(fcu->bezt[5].vec[1], float2(327.16f, 1.0f), 1e-3);
+
+  BKE_fcurve_free(fcu);
+}
+
 }  // namespace blender::bke::tests

source/blender/makesrna/intern/rna_fcurve.c

@@ -692,6 +692,7 @@ static void rna_tag_animation_update(Main *bmain, ID *id)
 static void rna_FCurve_update_data_ex(ID *id, FCurve *fcu, Main *bmain)
 {
   sort_time_fcurve(fcu);
+  /* TODO: Blender 4.0 should call BKE_fcurve_deduplicate_keys(fcu) here. */
   BKE_fcurve_handles_recalc(fcu);
 
   rna_tag_animation_update(bmain, id);
@@ -1106,6 +1107,24 @@ static void rna_FKeyframe_points_clear(ID *id, FCurve *fcu, Main *bmain)
   rna_tag_animation_update(bmain, id);
 }
 
+static void rna_FKeyframe_points_sort(ID *id, FCurve *fcu, Main *bmain)
+{
+  sort_time_fcurve(fcu);
+  rna_tag_animation_update(bmain, id);
+}
+
+static void rna_FKeyframe_points_deduplicate(ID *id, FCurve *fcu, Main *bmain)
+{
+  BKE_fcurve_deduplicate_keys(fcu);
+  rna_tag_animation_update(bmain, id);
+}
+
+static void rna_FKeyframe_points_handles_recalc(ID *id, FCurve *fcu, Main *bmain)
+{
+  BKE_fcurve_handles_recalc(fcu);
+  rna_tag_animation_update(bmain, id);
+}
+
 static FCM_EnvelopeData *rna_FModifierEnvelope_points_add(
     ID *id, FModifier *fmod, Main *bmain, ReportList *reports, float frame)
 {
@@ -2402,6 +2421,22 @@ static void rna_def_fcurve_keyframe_points(BlenderRNA *brna, PropertyRNA *cprop)
   func = RNA_def_function(srna, "clear", "rna_FKeyframe_points_clear");
   RNA_def_function_ui_description(func, "Remove all keyframes from an F-Curve");
   RNA_def_function_flag(func, FUNC_USE_SELF_ID | FUNC_USE_MAIN);
+
+  func = RNA_def_function(srna, "sort", "rna_FKeyframe_points_sort");
+  RNA_def_function_ui_description(func, "Ensure all keyframe points are chronologically sorted");
+  RNA_def_function_flag(func, FUNC_USE_SELF_ID | FUNC_USE_MAIN);
+
+  func = RNA_def_function(srna, "deduplicate", "rna_FKeyframe_points_deduplicate");
+  RNA_def_function_ui_description(
+      func,
+      "Ensure there are no duplicate keys. Assumes that the points have already been sorted");
+  RNA_def_function_flag(func, FUNC_USE_SELF_ID | FUNC_USE_MAIN);
+
+  func = RNA_def_function(srna, "handles_recalc", "rna_FKeyframe_points_handles_recalc");
+  RNA_def_function_ui_description(func,
+                                  "Update handles after modifications to the keyframe points, to "
+                                  "update things like auto-clamping");
+  RNA_def_function_flag(func, FUNC_USE_SELF_ID | FUNC_USE_MAIN);
 }
 
 static void rna_def_fcurve(BlenderRNA *brna)