Refactor: centralize responsibility for "Only Insert Needed"

This moves the responsibility for handling the "Only Insert Needed"
flag from `insert_keyframe_value()` to the lower-level function
`insert_vert_fcurve()`.

We're doing this because `insert_vert_fcurve()` is the common entry
point between the new animation system's and old animation system's
keyframing code. This therefore ensures that both systems will
behave the same way with respect to the "Only Insert Needed" flag.

Pull Request: https://projects.blender.org/blender/blender/pulls/121525

source/blender/animrig/intern/fcurve.cc

@@ -6,12 +6,14 @@
  * \ingroup animrig
  */
 
+#include <cfloat>
 #include <cmath>
 #include <cstring>
 
 #include "ANIM_animdata.hh"
 #include "ANIM_fcurve.hh"
 #include "BKE_fcurve.hh"
+#include "BLI_math_base.h"
 #include "BLI_math_vector_types.hh"
 #include "BLI_string.h"
 #include "DNA_anim_types.h"
@@ -271,11 +273,57 @@ void initialize_bezt(BezTriple *beztr,
   beztr->period = 4.1f;
 }
 
+/**
+ * Return whether the given fcurve already evaluates to the same value as the
+ * proposed keyframe at the keyframe's time.
+ *
+ * This is a helper function for determining whether to insert a keyframe or not
+ * when "only insert needed" is enabled.
+ *
+ * Note: this does *not* determine whether inserting the keyframe would change
+ * the fcurve at points other than the keyframe itself. For example, even if
+ * inserting the key wouldn't change the fcurve's value at the time of the
+ * keyframe, the resulting changes to bezier interpolation could change the
+ * fcurve on either side of it. This function intentionally does not account for
+ * that, since that's not how the "only insert needed" feature is supposed to
+ * work.
+ */
+static bool new_key_needed(FCurve &fcu, const float frame, const float value)
+{
+  if (fcu.totvert == 0) {
+    return true;
+  }
+
+  bool replace;
+  const int bezt_index = BKE_fcurve_bezt_binarysearch_index(
+      fcu.bezt, frame, fcu.totvert, &replace);
+
+  if (replace) {
+    /* If there is already a key, we only need to modify it if the proposed value is different. */
+    return fcu.bezt[bezt_index].vec[1][1] != value;
+  }
+
+  const int diff_ulp = 32;
+  const float fcu_eval = evaluate_fcurve(&fcu, frame);
+  /* No need to insert a key if the same value is already the value of the FCurve at that point. */
+  if (compare_ff_relative(fcu_eval, value, FLT_EPSILON, diff_ulp)) {
+    return false;
+  }
+
+  return true;
+}
+
 SingleKeyingResult insert_vert_fcurve(FCurve *fcu,
                                       const float2 position,
                                       const KeyframeSettings &settings,
                                       eInsertKeyFlags flag)
 {
+  BLI_assert(fcu != nullptr);
+
+  if ((flag & INSERTKEY_NEEDED) && !new_key_needed(*fcu, position[0], position[1])) {
+    return SingleKeyingResult::NO_KEY_NEEDED;
+  }
+
   BezTriple beztr = {{{0}}};
   initialize_bezt(&beztr, position, settings, eFCurve_Flags(fcu->flag));
 
@@ -494,7 +542,7 @@ void bake_fcurve_segments(FCurve *fcu)
           /* Add keyframes with these, tagging as 'breakdowns'. */
           for (n = 1, fp = value_cache; n < range && fp; n++, fp++) {
             blender::animrig::insert_vert_fcurve(
-                fcu, {fp->frame, fp->val}, settings, eInsertKeyFlags(1));
+                fcu, {fp->frame, fp->val}, settings, INSERTKEY_NOFLAGS);
           }
 
           MEM_freeN(value_cache);

source/blender/animrig/intern/keyframing.cc

@@ -6,7 +6,6 @@
  * \ingroup animrig
  */
 
-#include <cfloat>
 #include <cmath>
 #include <string>
 
@@ -425,38 +424,6 @@ static eFCU_Cycle_Type remap_cyclic_keyframe_location(FCurve *fcu, float *px, fl
   return type;
 }
 
-/**
- * This helper function determines whether a new keyframe is needed.
- * A keyframe doesn't get added when the FCurve already has the proposed value.
- */
-static bool new_key_needed(FCurve *fcu, const float frame, const float value)
-{
-  if (fcu == nullptr) {
-    return true;
-  }
-  if (fcu->totvert == 0) {
-    return true;
-  }
-
-  bool replace;
-  const int bezt_index = BKE_fcurve_bezt_binarysearch_index(
-      fcu->bezt, frame, fcu->totvert, &replace);
-
-  if (replace) {
-    /* If there is already a key, we only need to modify it if the proposed value is different. */
-    return fcu->bezt[bezt_index].vec[1][1] != value;
-  }
-
-  const int diff_ulp = 32;
-  const float fcu_eval = evaluate_fcurve(fcu, frame);
-  /* No need to insert a key if the same value is already the value of the FCurve at that point. */
-  if (compare_ff_relative(fcu_eval, value, FLT_EPSILON, diff_ulp)) {
-    return false;
-  }
-
-  return true;
-}
-
 static float nla_time_remap(const AnimationEvalContext *anim_eval_context,
                             PointerRNA *id_ptr,
                             AnimData *adt,
@@ -496,15 +463,7 @@ static SingleKeyingResult insert_keyframe_value(
   KeyframeSettings settings = get_keyframe_settings((flag & INSERTKEY_NO_USERPREF) == 0);
   settings.keyframe_type = keytype;
 
-  if ((flag & INSERTKEY_NEEDED) && !new_key_needed(fcu, cfra, curval)) {
-    return SingleKeyingResult::NO_KEY_NEEDED;
-  }
-
-  if (insert_vert_fcurve(fcu, {cfra, curval}, settings, flag) != SingleKeyingResult::SUCCESS) {
-    return SingleKeyingResult::FCURVE_NOT_KEYFRAMEABLE;
-  }
-
-  return SingleKeyingResult::SUCCESS;
+  return insert_vert_fcurve(fcu, {cfra, curval}, settings, flag);
 }
 
 bool insert_keyframe_direct(ReportList *reports,

source/blender/editors/armature/pose_slide.cc

@@ -1717,7 +1717,7 @@ static void propagate_curve_values(ListBase /*tPChanFCurveLink*/ *pflinks,
       const float current_fcu_value = evaluate_fcurve(fcu, source_frame);
       LISTBASE_FOREACH (FrameLink *, target_frame, target_frames) {
         insert_vert_fcurve(
-            fcu, {target_frame->frame, current_fcu_value}, settings, INSERTKEY_NEEDED);
+            fcu, {target_frame->frame, current_fcu_value}, settings, INSERTKEY_NOFLAGS);
       }
     }
   }