From 5671e7a92c397e3558f346cc2796da452f016b17 Mon Sep 17 00:00:00 2001 From: Colin Basnett Date: Fri, 11 Nov 2022 11:07:30 -0800 Subject: 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 --- source/blender/blenkernel/intern/fcurve.c | 140 ++++++++++++------------------ 1 file 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); } -- cgit v1.2.3