Cleanup: Fixing anti-patterns in fcurve.c

This is a clean-up pass that eliminates a few problematic patterns:

* Eliminating redundant parentheses around simple expressions.
* Combing declaration and assignment of variables where appropriate.
* Moving variable declarations closer to their first use.
* Many variables and arguments have been marked as `const`.
* Using `LISTBASE_FOREACH_*` variants where applicable instead of
  manually managing loop control flow.

There are no functional changes.

Reviewed By: sybren

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

1 files changed, 57 insertions, 83 deletions

diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c
index aa99a5f605a..3e772e37177 100644
--- a/source/blender/blenkernel/intern/fcurve.c
+++ b/source/blender/blenkernel/intern/fcurve.c
@@ -85,8 +85,6 @@ void BKE_fcurve_free(FCurve *fcu)
 
 void BKE_fcurves_free(ListBase *list)
 {
-  FCurve *fcu, *fcn;
-
   /* Sanity check. */
   if (list == NULL) {
     return;
@@ -96,7 +94,8 @@ void BKE_fcurves_free(ListBase *list)
    * as we store reference to next, and freeing only touches the curve
    * it's given.
    */
-  for (fcu = list->first; fcu; fcu = fcn) {
+  FCurve *fcn = NULL;
+  for (FCurve *fcu = list->first; fcu; fcu = fcn) {
     fcn = fcu->next;
     BKE_fcurve_free(fcu);
   }
@@ -113,15 +112,13 @@ void BKE_fcurves_free(ListBase *list)
 
 FCurve *BKE_fcurve_copy(const FCurve *fcu)
 {
-  FCurve *fcu_d;
-
   /* Sanity check. */
   if (fcu == NULL) {
     return NULL;
   }
 
   /* Make a copy. */
-  fcu_d = MEM_dupallocN(fcu);
+  FCurve *fcu_d = MEM_dupallocN(fcu);
 
   fcu_d->next = fcu_d->prev = NULL;
   fcu_d->grp = NULL;
@@ -145,8 +142,6 @@ FCurve *BKE_fcurve_copy(const FCurve *fcu)
 
 void BKE_fcurves_copy(ListBase *dst, ListBase *src)
 {
-  FCurve *dfcu, *sfcu;
-
   /* Sanity checks. */
   if (ELEM(NULL, dst, src)) {
     return;
@@ -156,8 +151,8 @@ void BKE_fcurves_copy(ListBase *dst, ListBase *src)
   BLI_listbase_clear(dst);
 
   /* Copy one-by-one. */
-  for (sfcu = src->first; sfcu; sfcu = sfcu->next) {
-    dfcu = BKE_fcurve_copy(sfcu);
+  LISTBASE_FOREACH (FCurve *, sfcu, src) {
+    FCurve *dfcu = BKE_fcurve_copy(sfcu);
     BLI_addtail(dst, dfcu);
   }
 }
@@ -203,12 +198,10 @@ FCurve *id_data_find_fcurve(
 {
   /* Anim vars */
   AnimData *adt = BKE_animdata_from_id(id);
-  FCurve *fcu = NULL;
 
   /* Rna vars */
   PointerRNA ptr;
   PropertyRNA *prop;
-  char *path;
 
   if (r_driven) {
     *r_driven = false;
@@ -225,7 +218,7 @@ FCurve *id_data_find_fcurve(
     return NULL;
   }
 
-  path = RNA_path_from_ID_to_property(&ptr, prop);
+  char *path = RNA_path_from_ID_to_property(&ptr, prop);
   if (path == NULL) {
     return NULL;
   }
@@ -233,7 +226,7 @@ FCurve *id_data_find_fcurve(
   /* FIXME: The way drivers are handled here (always NULL-ifying `fcu`) is very weird, this needs
    * to be re-checked I think?. */
   bool is_driven = false;
-  fcu = BKE_animadata_fcurve_find_by_rna_path(adt, path, index, NULL, &is_driven);
+  FCurve *fcu = BKE_animadata_fcurve_find_by_rna_path(adt, path, index, NULL, &is_driven);
   if (is_driven) {
     if (r_driven != NULL) {
       *r_driven = is_driven;
@@ -248,15 +241,13 @@ FCurve *id_data_find_fcurve(
 
 FCurve *BKE_fcurve_find(ListBase *list, const char rna_path[], const int array_index)
 {
-  FCurve *fcu;
-
   /* Sanity checks. */
-  if (ELEM(NULL, list, rna_path) || (array_index < 0)) {
+  if (ELEM(NULL, list, rna_path) || array_index < 0) {
     return NULL;
   }
 
   /* Check paths of curves, then array indices... */
-  for (fcu = list->first; fcu; fcu = fcu->next) {
+  LISTBASE_FOREACH (FCurve *, fcu, list) {
     /* Check indices first, much cheaper than a string comparison. */
     /* Simple string-compare (this assumes that they have the same root...) */
     if (UNLIKELY(fcu->array_index == array_index && fcu->rna_path &&
@@ -276,15 +267,13 @@ FCurve *BKE_fcurve_find(ListBase *list, const char rna_path[], const int array_i
 
 FCurve *BKE_fcurve_iter_step(FCurve *fcu_iter, const char rna_path[])
 {
-  FCurve *fcu;
-
   /* Sanity checks. */
   if (ELEM(NULL, fcu_iter, rna_path)) {
     return NULL;
   }
 
   /* Check paths of curves, then array indices... */
-  for (fcu = fcu_iter; fcu; fcu = fcu->next) {
+  for (FCurve *fcu = fcu_iter; fcu; fcu = fcu->next) {
     /* Simple string-compare (this assumes that they have the same root...) */
     if (fcu->rna_path && STREQ(fcu->rna_path, rna_path)) {
       return fcu;
@@ -296,7 +285,6 @@ FCurve *BKE_fcurve_iter_step(FCurve *fcu_iter, const char rna_path[])
 
 int BKE_fcurves_filter(ListBase *dst, ListBase *src, const char *dataPrefix, const char *dataName)
 {
-  FCurve *fcu;
   int matches = 0;
 
   /* Sanity checks. */
@@ -311,7 +299,7 @@ int BKE_fcurves_filter(ListBase *dst, ListBase *src, const char *dataPrefix, con
   char *quotedName = alloca(quotedName_size);
 
   /* Search each F-Curve one by one. */
-  for (fcu = src->first; fcu; fcu = fcu->next) {
+  LISTBASE_FOREACH (FCurve *, fcu, src) {
     /* Check if quoted string matches the path. */
     if (fcu->rna_path == NULL) {
       continue;
@@ -487,16 +475,14 @@ static int BKE_fcurve_bezt_binarysearch_index_ex(const BezTriple array[],
    * - Keyframe to be added is to be added out of current bounds.
    * - Keyframe to be added would replace one of the existing ones on bounds.
    */
-  if ((arraylen <= 0) || (array == NULL)) {
+  if (arraylen <= 0 || array == NULL) {
     CLOG_WARN(&LOG, "encountered invalid array");
     return 0;
   }
 
   /* Check whether to add before/after/on. */
-  float framenum;
-
   /* 'First' Keyframe (when only one keyframe, this case is used) */
-  framenum = array[0].vec[1][0];
+  float framenum = array[0].vec[1][0];
   if (IS_EQT(frame, framenum, threshold)) {
     *r_replace = true;
     return 0;
@@ -522,9 +508,9 @@ static int BKE_fcurve_bezt_binarysearch_index_ex(const BezTriple array[],
     /* Compute and get midpoint. */
 
     /* We calculate the midpoint this way to avoid int overflows... */
-    int mid = start + ((end - start) / 2);
+    const int mid = start + ((end - start) / 2);
 
-    float midfra = array[mid].vec[1][0];
+    const float midfra = array[mid].vec[1][0];
 
     /* Check if exactly equal to midpoint. */
     if (IS_EQT(frame, midfra, threshold)) {
@@ -589,10 +575,8 @@ static short get_fcurve_end_keyframes(const FCurve *fcu,
 
   /* Only include selected items? */
   if (do_sel_only) {
-    BezTriple *bezt;
-
     /* Find first selected. */
-    bezt = fcu->bezt;
+    BezTriple *bezt = fcu->bezt;
     for (int i = 0; i < fcu->totvert; bezt++, i++) {
       if (BEZT_ISSEL_ANY(bezt)) {
         *first = bezt;
@@ -696,10 +680,9 @@ bool BKE_fcurve_calc_bounds(const FCurve *fcu,
 
       /* Only loop over keyframes to find extents for values if needed. */
       if (ymin || ymax) {
-        FPoint *fpt;
-        int i;
+        int i = 0;
 
-        for (fpt = fcu->fpt, i = 0; i < fcu->totvert; fpt++, i++) {
+        for (FPoint *fpt = fcu->fpt; i < fcu->totvert; fpt++, i++) {
           if (fpt->vec[1] < yminv) {
             yminv = fpt->vec[1];
           }
@@ -855,7 +838,7 @@ void BKE_fcurve_active_keyframe_set(FCurve *fcu, const BezTriple *active_bezt)
 
   /* Gracefully handle out-of-bounds pointers. Ideally this would do a BLI_assert() as well, but
    * then the unit tests would break in debug mode. */
-  ptrdiff_t offset = active_bezt - fcu->bezt;
+  const ptrdiff_t offset = active_bezt - fcu->bezt;
   if (offset < 0 || offset >= fcu->totvert) {
     fcu->active_keyframe_index = FCURVE_ACTIVE_KEYFRAME_NONE;
     return;
@@ -872,8 +855,7 @@ int BKE_fcurve_active_keyframe_index(const FCurve *fcu)
   const int active_keyframe_index = fcu->active_keyframe_index;
 
   /* Array access boundary checks. */
-  if ((fcu->bezt == NULL) || (active_keyframe_index >= fcu->totvert) ||
-      (active_keyframe_index < 0)) {
+  if (fcu->bezt == NULL || active_keyframe_index >= fcu->totvert || active_keyframe_index < 0) {
     return FCURVE_ACTIVE_KEYFRAME_NONE;
   }
 
@@ -914,11 +896,9 @@ bool BKE_fcurve_are_keyframes_usable(const FCurve *fcu)
 
   /* If it has modifiers, none of these should "drastically" alter the curve. */
   if (fcu->modifiers.first) {
-    FModifier *fcm;
-
     /* Check modifiers from last to first, as last will be more influential. */
     /* TODO: optionally, only check modifier if it is the active one... (Joshua Leung 2010) */
-    for (fcm = fcu->modifiers.last; fcm; fcm = fcm->prev) {
+    LISTBASE_FOREACH_BACKWARD (FModifier *, fcm, &fcu->modifiers) {
       /* Ignore if muted/disabled. */
       if (fcm->flag & (FMODIFIER_FLAG_DISABLED | FMODIFIER_FLAG_MUTED)) {
         continue;
@@ -962,7 +942,7 @@ bool BKE_fcurve_are_keyframes_usable(const FCurve *fcu)
 
 bool BKE_fcurve_is_protected(const FCurve *fcu)
 {
-  return ((fcu->flag & FCURVE_PROTECTED) || ((fcu->grp) && (fcu->grp->flag & AGRP_PROTECTED)));
+  return ((fcu->flag & FCURVE_PROTECTED) || (fcu->grp && (fcu->grp->flag & AGRP_PROTECTED)));
 }
 
 bool BKE_fcurve_has_selected_control_points(const FCurve *fcu)
@@ -1048,9 +1028,6 @@ float fcurve_samplingcb_evalcurve(FCurve *fcu, void *UNUSED(data), float evaltim
 
 void fcurve_store_samples(FCurve *fcu, void *data, int start, int end, FcuSampleFunc sample_cb)
 {
-  FPoint *fpt, *new_fpt;
-  int cfra;
-
   /* Sanity checks. */
   /* TODO: make these tests report errors using reports not CLOG's (Joshua Leung 2009) */
   if (ELEM(NULL, fcu, sample_cb)) {
@@ -1063,10 +1040,11 @@ void fcurve_store_samples(FCurve *fcu, void *data, int start, int end, FcuSample
   }
 
   /* Set up sample data. */
-  fpt = new_fpt = MEM_callocN(sizeof(FPoint) * (end - start + 1), "FPoint Samples");
+  FPoint *new_fpt;
+  FPoint *fpt = new_fpt = MEM_callocN(sizeof(FPoint) * (end - start + 1), "FPoint Samples");
 
   /* Use the sampling callback at 1-frame intervals from start to end frames. */
-  for (cfra = start; cfra <= end; cfra++, fpt++) {
+  for (int cfra = start; cfra <= end; cfra++, fpt++) {
     fpt->vec[0] = (float)cfra;
     fpt->vec[1] = sample_cb(fcu, data, (float)cfra);
   }
@@ -1119,12 +1097,12 @@ void fcurve_samples_to_keyframes(FCurve *fcu, const int start, const int end)
     MEM_freeN(fcu->bezt);
   }
 
-  BezTriple *bezt;
   FPoint *fpt = fcu->fpt;
   int keyframes_to_insert = end - start;
   int sample_points = fcu->totvert;
 
-  bezt = fcu->bezt = MEM_callocN(sizeof(*fcu->bezt) * (size_t)keyframes_to_insert, __func__);
+  BezTriple *bezt = fcu->bezt = MEM_callocN(sizeof(*fcu->bezt) * (size_t)keyframes_to_insert,
+                                            __func__);
   fcu->totvert = keyframes_to_insert;
 
   /* Get first sample point to 'copy' as keyframe. */
@@ -1231,7 +1209,6 @@ static BezTriple *cycle_offset_triple(
 
 void BKE_fcurve_handles_recalc_ex(FCurve *fcu, eBezTriple_Flag handle_sel_flag)
 {
-  BezTriple *bezt, *prev, *next;
   int a = fcu->totvert;
 
   /* Error checking:
@@ -1247,12 +1224,12 @@ void BKE_fcurve_handles_recalc_ex(FCurve *fcu, eBezTriple_Flag handle_sel_flag)
   BezTriple *first = &fcu->bezt[0], *last = &fcu->bezt[fcu->totvert - 1];
   BezTriple tmp;
 
-  bool cycle = BKE_fcurve_is_cyclic(fcu) && BEZT_IS_AUTOH(first) && BEZT_IS_AUTOH(last);
+  const bool cycle = BKE_fcurve_is_cyclic(fcu) && BEZT_IS_AUTOH(first) && BEZT_IS_AUTOH(last);
 
   /* Get initial pointers. */
-  bezt = fcu->bezt;
-  prev = cycle_offset_triple(cycle, &tmp, &fcu->bezt[fcu->totvert - 2], last, first);
-  next = (bezt + 1);
+  BezTriple *bezt = fcu->bezt;
+  BezTriple *prev = cycle_offset_triple(cycle, &tmp, &fcu->bezt[fcu->totvert - 2], last, first);
+  BezTriple *next = (bezt + 1);
 
   /* Loop over all beztriples, adjusting handles. */
   while (a--) {
@@ -1319,15 +1296,14 @@ void BKE_fcurve_handles_recalc(FCurve *fcu)
 
 void testhandles_fcurve(FCurve *fcu, eBezTriple_Flag sel_flag, const bool use_handle)
 {
-  BezTriple *bezt;
-  uint a;
-
   /* Only beztriples have handles (bpoints don't though). */
   if (ELEM(NULL, fcu, fcu->bezt)) {
     return;
   }
 
   /* Loop over beztriples. */
+  BezTriple *bezt;
+  uint a;
   for (a = 0, bezt = fcu->bezt; a < fcu->totvert; a++, bezt++) {
     BKE_nurb_bezt_handle_test(bezt, sel_flag, use_handle, false);
   }
@@ -1701,12 +1677,12 @@ void BKE_fcurve_delete_key(FCurve *fcu, int index)
 
 bool BKE_fcurve_delete_keys_selected(FCurve *fcu)
 {
-  bool changed = false;
-
   if (fcu->bezt == NULL) { /* ignore baked curves */
     return false;
   }
 
+  bool changed = false;
+
   /* Delete selected BezTriples */
   for (int i = 0; i < fcu->totvert; i++) {
     if (fcu->bezt[i].f2 & SELECT) {
@@ -1742,9 +1718,9 @@ void BKE_fcurve_delete_keys_all(FCurve *fcu)
 static float fcurve_eval_keyframes_extrapolate(
     FCurve *fcu, BezTriple *bezts, float evaltime, int endpoint_offset, int direction_to_neighbor)
 {
-  BezTriple *endpoint_bezt = bezts + endpoint_offset; /* The first/last keyframe. */
-  BezTriple *neighbor_bezt = endpoint_bezt +
-                             direction_to_neighbor; /* The second (to last) keyframe. */
+  const BezTriple *endpoint_bezt = bezts + endpoint_offset; /* The first/last keyframe. */
+  const BezTriple *neighbor_bezt = endpoint_bezt +
+                                   direction_to_neighbor; /* The second (to last) keyframe. */
 
   if (endpoint_bezt->ipo == BEZT_IPO_CONST || fcu->extend == FCURVE_EXTRAPOLATE_CONSTANT ||
       (fcu->flag & FCURVE_DISCRETE_VALUES) != 0) {
@@ -1759,7 +1735,7 @@ static float fcurve_eval_keyframes_extrapolate(
       return endpoint_bezt->vec[1][1];
     }
 
-    float dx = endpoint_bezt->vec[1][0] - evaltime;
+    const float dx = endpoint_bezt->vec[1][0] - evaltime;
     float fac = neighbor_bezt->vec[1][0] - endpoint_bezt->vec[1][0];
 
     /* Prevent division by zero. */
@@ -1773,8 +1749,8 @@ static float fcurve_eval_keyframes_extrapolate(
 
   /* Use the gradient of the second handle (later) of neighbor to calculate the gradient and thus
    * the value of the curve at evaluation time. */
-  int handle = direction_to_neighbor > 0 ? 0 : 2;
-  float dx = endpoint_bezt->vec[1][0] - evaltime;
+  const int handle = direction_to_neighbor > 0 ? 0 : 2;
+  const float dx = endpoint_bezt->vec[1][0] - evaltime;
   float fac = endpoint_bezt->vec[1][0] - endpoint_bezt->vec[handle][0];
 
   /* Prevent division by zero. */
@@ -1786,10 +1762,11 @@ static float fcurve_eval_keyframes_extrapolate(
   return endpoint_bezt->vec[1][1] - (fac * dx);
 }
 
-static float fcurve_eval_keyframes_interpolate(FCurve *fcu, BezTriple *bezts, float evaltime)
+static float fcurve_eval_keyframes_interpolate(const FCurve *fcu,
+                                               const BezTriple *bezts,
+                                               float evaltime)
 {
   const float eps = 1.e-8f;
-  BezTriple *bezt, *prevbezt;
   uint a;
 
   /* Evaltime occurs somewhere in the middle of the curve. */
@@ -1806,7 +1783,7 @@ static float fcurve_eval_keyframes_interpolate(FCurve *fcu, BezTriple *bezts, fl
    *   This lower bound was established in b888a32eee8147b028464336ad2404d8155c64dd.
    */
   a = BKE_fcurve_bezt_binarysearch_index_ex(bezts, evaltime, fcu->totvert, 0.0001, &exact);
-  bezt = bezts + a;
+  const BezTriple *bezt = bezts + a;
 
   if (exact) {
     /* Index returned must be interpreted differently when it sits on top of an existing keyframe
@@ -1818,7 +1795,7 @@ static float fcurve_eval_keyframes_interpolate(FCurve *fcu, BezTriple *bezts, fl
   /* Index returned refers to the keyframe that the eval-time occurs *before*
    * - hence, that keyframe marks the start of the segment we're dealing with.
    */
-  prevbezt = (a > 0) ? (bezt - 1) : bezt;
+  const BezTriple *prevbezt = (a > 0) ? (bezt - 1) : bezt;
 
   /* Use if the key is directly on the frame, in rare cases this is needed else we get 0.0 instead.
    * XXX: consult T39207 for examples of files where failure of these checks can cause issues. */
@@ -2054,7 +2031,7 @@ static float fcurve_eval_keyframes(FCurve *fcu, BezTriple *bezts, float evaltime
     return fcurve_eval_keyframes_extrapolate(fcu, bezts, evaltime, 0, +1);
   }
 
-  BezTriple *lastbezt = bezts + fcu->totvert - 1;
+  const BezTriple *lastbezt = bezts + fcu->totvert - 1;
   if (lastbezt->vec[1][0] <= evaltime) {
     return fcurve_eval_keyframes_extrapolate(fcu, bezts, evaltime, fcu->totvert - 1, -1);
   }
@@ -2063,14 +2040,13 @@ static float fcurve_eval_keyframes(FCurve *fcu, BezTriple *bezts, float evaltime
 }
 
 /* Calculate F-Curve value for 'evaltime' using #FPoint samples. */
-static float fcurve_eval_samples(FCurve *fcu, FPoint *fpts, float evaltime)
+static float fcurve_eval_samples(const FCurve *fcu, const FPoint *fpts, float evaltime)
 {
-  FPoint *prevfpt, *lastfpt, *fpt;
   float cvalue = 0.0f;
 
   /* Get pointers. */
-  prevfpt = fpts;
-  lastfpt = prevfpt + fcu->totvert - 1;
+  const FPoint *prevfpt = fpts;
+  const FPoint *lastfpt = prevfpt + fcu->totvert - 1;
 
   /* Evaluation time at or past endpoints? */
   if (prevfpt->vec[0] >= evaltime) {
@@ -2085,10 +2061,10 @@ static float fcurve_eval_samples(FCurve *fcu, FPoint *fpts, float evaltime)
     float t = fabsf(evaltime - floorf(evaltime));
 
     /* Find the one on the right frame (assume that these are spaced on 1-frame intervals). */
-    fpt = prevfpt + ((int)evaltime - (int)prevfpt->vec[0]);
+    const FPoint *fpt = prevfpt + ((int)evaltime - (int)prevfpt->vec[0]);
 
     /* If not exactly on the frame, perform linear interpolation with the next one. */
-    if ((t != 0.0f) && (t < 1.0f)) {
+    if (t != 0.0f && t < 1.0f) {
       cvalue = interpf(fpt->vec[1], (fpt + 1)->vec[1], 1.0f - t);
     }
     else {
@@ -2110,15 +2086,14 @@ static float fcurve_eval_samples(FCurve *fcu, FPoint *fpts, float evaltime)
  */
 static float evaluate_fcurve_ex(FCurve *fcu, float evaltime, float cvalue)
 {
-  float devaltime;
-
   /* Evaluate modifiers which modify time to evaluate the base curve at. */
   FModifiersStackStorage storage;
   storage.modifier_count = BLI_listbase_count(&fcu->modifiers);
   storage.size_per_modifier = evaluate_fmodifiers_storage_size_per_modifier(&fcu->modifiers);
   storage.buffer = alloca(storage.modifier_count * storage.size_per_modifier);
 
-  devaltime = evaluate_time_fmodifiers(&storage, &fcu->modifiers, fcu, cvalue, evaltime);
+  const float devaltime = evaluate_time_fmodifiers(
+      &storage, &fcu->modifiers, fcu, cvalue, evaltime);
 
   /* Evaluate curve-data
    * - 'devaltime' instead of 'evaltime', as this is the time that the last time-modifying
@@ -2177,16 +2152,15 @@ float evaluate_fcurve_driver(PathResolvedRNA *anim_rna,
 
     /* Only do a default 1-1 mapping if it's unlikely that anything else will set a value... */
     if (fcu->totvert == 0) {
-      FModifier *fcm;
       bool do_linear = true;
 
       /* Out-of-range F-Modifiers will block, as will those which just plain overwrite the values
        * XXX: additive is a bit more dicey; it really depends then if things are in range or not...
        */
-      for (fcm = fcu->modifiers.first; fcm; fcm = fcm->next) {
+      LISTBASE_FOREACH (FModifier *, fcm, &fcu->modifiers) {
         /* If there are range-restrictions, we must definitely block T36950. */
         if ((fcm->flag & FMODIFIER_FLAG_RANGERESTRICT) == 0 ||
-            ((fcm->sfra <= evaltime) && (fcm->efra >= evaltime))) {
+            (fcm->sfra <= evaltime && fcm->efra >= evaltime)) {
           /* Within range: here it probably doesn't matter,
            * though we'd want to check on additive. */
         }
@@ -2209,7 +2183,7 @@ float evaluate_fcurve_driver(PathResolvedRNA *anim_rna,
 
 bool BKE_fcurve_is_empty(const FCurve *fcu)
 {
-  return (fcu->totvert == 0) && (fcu->driver == NULL) &&
+  return fcu->totvert == 0 && fcu->driver == NULL &&
          !list_has_suitable_fmodifier(&fcu->modifiers, 0, FMI_TYPE_GENERATE_CURVE);
 }