[Bf-blender-cvs] [5671e7a92c3] master: Cleanup: Fixing anti-patterns in fcurve.c

Colin Basnett noreply at git.blender.org
Fri Nov 11 20:07:36 CET 2022


Commit: 5671e7a92c397e3558f346cc2796da452f016b17
Author: Colin Basnett
Date:   Fri Nov 11 11:07:30 2022 -0800
Branches: master
https://developer.blender.org/rB5671e7a92c397e3558f346cc2796da452f016b17

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

===================================================================

M	source/blender/blenkernel/intern/fcurve.c

===================================================================

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_r

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list