blender/blender
170
529
Fork 806
Code Issues 6.5k Pull Requests 968 Projects 21 Wiki Activity

Refactor: make evaluate_fcurve() take its FCurve as const instead of mutable #121788

Merged
Nathan Vegdahl merged 3 commits from nathanvegdahl/blender:const_fcurve_functions into main 2024-05-16 17:13:53 +02:00
Conversation 3 Commits 3 Files Changed 3 +67 -43
3 changed files with 67 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
source/blender/blenkernel/BKE_fcurve.hh
View File

@ -76,10 +76,10 @@ struct FModifierTypeInfo {
/* evaluation */
/** Evaluate time that the modifier requires the F-Curve to be evaluated at */
float (*evaluate_modifier_time)(
FCurve *fcu, FModifier *fcm, float cvalue, float evaltime, void *storage);
const FCurve *fcu, const FModifier *fcm, float cvalue, float evaltime, void *storage);
/** Evaluate the modifier for the given time and 'accumulated' value */
void (*evaluate_modifier)(
FCurve *fcu, FModifier *fcm, float *cvalue, float evaltime, void *storage);
const FCurve *fcu, const FModifier *fcm, float *cvalue, float evaltime, void *storage);
};
/* Values which describe the behavior of a FModifier Type */
@ -165,7 +165,7 @@ struct FModifiersStackStorage {
void *buffer;
};
uint evaluate_fmodifiers_storage_size_per_modifier(ListBase *modifiers);
uint evaluate_fmodifiers_storage_size_per_modifier(const ListBase *modifiers);
/**
* Evaluate time modifications imposed by some F-Curve Modifiers.
*
@ -180,8 +180,8 @@ uint evaluate_fmodifiers_storage_size_per_modifier(ListBase *modifiers);
* \param fcu: Can be NULL.
*/
float evaluate_time_fmodifiers(FModifiersStackStorage *storage,
ListBase *modifiers,
FCurve *fcu,
const ListBase *modifiers,
const FCurve *fcu,
float cvalue,
float evaltime);
/**
@ -189,8 +189,8 @@ float evaluate_time_fmodifiers(FModifiersStackStorage *storage,
* Should only be called after evaluate_time_fmodifiers() has been called.
*/
void evaluate_value_fmodifiers(FModifiersStackStorage *storage,
ListBase *modifiers,
FCurve *fcu,
const ListBase *modifiers,
const FCurve *fcu,
float *cvalue,
float evaltime);
@ -598,7 +598,7 @@ void BKE_fcurve_correct_bezpart(const float v1[2], float v2[2], float v3[2], con
/* -------- Evaluation -------- */
/* evaluate fcurve */
float evaluate_fcurve(FCurve *fcu, float evaltime);
float evaluate_fcurve(const FCurve *fcu, float evaltime);
float evaluate_fcurve_only_curve(FCurve *fcu, float evaltime);
float evaluate_fcurve_driver(PathResolvedRNA *anim_rna,
FCurve *fcu,

13
source/blender/blenkernel/intern/fcurve.cc
View File

@ -2032,8 +2032,11 @@ void BKE_fcurve_deduplicate_keys(FCurve *fcu)
/** \name F-Curve Evaluation
* \{ */
static float fcurve_eval_keyframes_extrapolate(
FCurve *fcu, BezTriple *bezts, float evaltime, int endpoint_offset, int direction_to_neighbor)
static float fcurve_eval_keyframes_extrapolate(const FCurve *fcu,
const BezTriple *bezts,
float evaltime,
int endpoint_offset,
int direction_to_neighbor)
{
/* The first/last keyframe. */
const BezTriple *endpoint_bezt = bezts + endpoint_offset;
@ -2345,7 +2348,7 @@ static float fcurve_eval_keyframes_interpolate(const FCurve *fcu,
}
/* Calculate F-Curve value for 'evaltime' using #BezTriple keyframes. */
static float fcurve_eval_keyframes(FCurve *fcu, BezTriple *bezts, float evaltime)
static float fcurve_eval_keyframes(const FCurve *fcu, const BezTriple *bezts, float evaltime)
{
if (evaltime <= bezts->vec[1][0]) {
return fcurve_eval_keyframes_extrapolate(fcu, bezts, evaltime, 0, +1);
@ -2404,7 +2407,7 @@ static float fcurve_eval_samples(const FCurve *fcu, const FPoint *fpts, float ev
/* Evaluate and return the value of the given F-Curve at the specified frame ("evaltime")
* NOTE: this is also used for drivers.
*/
static float evaluate_fcurve_ex(FCurve *fcu, float evaltime, float cvalue)
static float evaluate_fcurve_ex(const FCurve *fcu, float evaltime, float cvalue)
{
/* Evaluate modifiers which modify time to evaluate the base curve at. */
FModifiersStackStorage storage;
@ -2439,7 +2442,7 @@ static float evaluate_fcurve_ex(FCurve *fcu, float evaltime, float cvalue)
return cvalue;
}
float evaluate_fcurve(FCurve *fcu, float evaltime)
float evaluate_fcurve(const FCurve *fcu, float evaltime)
{
BLI_assert(fcu->driver == nullptr);

81
source/blender/blenkernel/intern/fmodifier.cc
View File

@ -155,8 +155,11 @@ static void fcm_generator_verify(FModifier *fcm)
}
}
static void fcm_generator_evaluate(
FCurve * /*fcu*/, FModifier *fcm, float *cvalue, float evaltime, void * /*storage*/)
static void fcm_generator_evaluate(const FCurve * /*fcu*/,
const FModifier *fcm,
float *cvalue,
float evaltime,
void * /*storage*/)
{
FMod_Generator *data = (FMod_Generator *)fcm->data;
@ -280,8 +283,11 @@ static double sinc(double x)
return sin(M_PI * x) / (M_PI * x);
}
static void fcm_fn_generator_evaluate(
FCurve * /*fcu*/, FModifier *fcm, float *cvalue, float evaltime, void * /*storage*/)
static void fcm_fn_generator_evaluate(const FCurve * /*fcu*/,
const FModifier *fcm,
float *cvalue,
float evaltime,
void * /*storage*/)
{
FMod_FunctionGenerator *data = (FMod_FunctionGenerator *)fcm->data;
double arg = data->phase_multiplier * evaltime + data->phase_offset;
@ -418,8 +424,11 @@ static void fcm_envelope_verify(FModifier *fcm)
}
}
static void fcm_envelope_evaluate(
FCurve * /*fcu*/, FModifier *fcm, float *cvalue, float evaltime, void * /*storage*/)
static void fcm_envelope_evaluate(const FCurve * /*fcu*/,
const FModifier *fcm,
float *cvalue,
float evaltime,
void * /*storage*/)
{
FMod_Envelope *env = (FMod_Envelope *)fcm->data;
FCM_EnvelopeData *fed, *prevfed, *lastfed;
@ -611,7 +620,7 @@ static void fcm_cycles_new_data(void *mdata)
}
static float fcm_cycles_time(
FCurve *fcu, FModifier *fcm, float /*cvalue*/, float evaltime, void *storage_)
const FCurve *fcu, const FModifier *fcm, float /*cvalue*/, float evaltime, void *storage_)
{
const FMod_Cycles *data = (FMod_Cycles *)fcm->data;
tFCMED_Cycles *storage = static_cast<tFCMED_Cycles *>(storage_);
@ -623,12 +632,9 @@ static float fcm_cycles_time(
/* Initialize storage. */
storage->cycyofs = 0;
/* check if modifier is first in stack, otherwise disable ourself... */
/* FIXME... */
if (fcm->prev) {
fcm->flag |= FMODIFIER_FLAG_DISABLED;
return evaltime;
}
/* It shouldn't be possible for this modifier type to be anywhere other than
* the top of the stack. If it is, something's wrong. */
BLI_assert(fcm->prev == nullptr);
nathanvegdahl marked this conversation as resolved Outdated
Nathan Vegdahl commented 2024-05-14 15:43:00 +02:00
Outdated
Review
Copy Link

Here is the (likely bogus) change that's otherwise blocking evaluate_fcurve() from being const.

Here is the (likely bogus) change that's otherwise blocking `evaluate_fcurve()` from being const.
Falk David commented 2024-05-14 16:14:11 +02:00
Outdated
Review
Copy Link

Looks like this logic would have to be pulled out and put into evaluate_time_fmodifiers directly (that's where this code likely should have been from the start).

Looks like this logic would have to be pulled out and put into `evaluate_time_fmodifiers` directly (that's where this code likely should have been from the start).
Falk David commented 2024-05-14 16:20:00 +02:00
Outdated
Review
Copy Link
diff --git a/source/blender/blenkernel/intern/fmodifier.cc b/source/blender/blenkernel/intern/fmodifier.cc
index 025c4851733..7e49d71edee 100644
--- a/source/blender/blenkernel/intern/fmodifier.cc
+++ b/source/blender/blenkernel/intern/fmodifier.cc
@@ -623,13 +623,6 @@ static float fcm_cycles_time(
   /* Initialize storage. */
   storage->cycyofs = 0;
 
-  /* check if modifier is first in stack, otherwise disable ourself... */
-  /* FIXME... */
-  if (fcm->prev) {
-    fcm->flag |= FMODIFIER_FLAG_DISABLED;
-    return evaltime;
-  }
-
   if (fcu == nullptr || (fcu->bezt == nullptr && fcu->fpt == nullptr)) {
     return evaltime;
   }
@@ -1399,6 +1392,19 @@ float evaluate_time_fmodifiers(FModifiersStackStorage *storage,
     return evaltime;
   }
 
+  /**
+   * Disable all cycles modifiers that are not first in the stack.
+   */
+  LISTBASE_FOREACH (FModifier *, fcm, modifiers) {
+    const FModifierTypeInfo *fmi = fmodifier_get_typeinfo(fcm);
+    if (fmi == nullptr || fmi->type != FMODIFIER_TYPE_CYCLES) {
+      continue;
+    }
+    if (fcm->prev) {
+      fcm->flag |= FMODIFIER_FLAG_DISABLED;
+    }
+  }
+
   /* Starting from the end of the stack, calculate the time effects of various stacked modifiers
    * on the time the F-Curve should be evaluated at.
    *
```diff diff --git a/source/blender/blenkernel/intern/fmodifier.cc b/source/blender/blenkernel/intern/fmodifier.cc index 025c4851733..7e49d71edee 100644 --- a/source/blender/blenkernel/intern/fmodifier.cc +++ b/source/blender/blenkernel/intern/fmodifier.cc @@ -623,13 +623,6 @@ static float fcm_cycles_time( /* Initialize storage. */ storage->cycyofs = 0; - /* check if modifier is first in stack, otherwise disable ourself... */ - /* FIXME... */ - if (fcm->prev) { - fcm->flag |= FMODIFIER_FLAG_DISABLED; - return evaltime; - } - if (fcu == nullptr || (fcu->bezt == nullptr && fcu->fpt == nullptr)) { return evaltime; } @@ -1399,6 +1392,19 @@ float evaluate_time_fmodifiers(FModifiersStackStorage *storage, return evaltime; } + /** + * Disable all cycles modifiers that are not first in the stack. + */ + LISTBASE_FOREACH (FModifier *, fcm, modifiers) { + const FModifierTypeInfo *fmi = fmodifier_get_typeinfo(fcm); + if (fmi == nullptr || fmi->type != FMODIFIER_TYPE_CYCLES) { + continue; + } + if (fcm->prev) { + fcm->flag |= FMODIFIER_FLAG_DISABLED; + } + } + /* Starting from the end of the stack, calculate the time effects of various stacked modifiers * on the time the F-Curve should be evaluated at. * ```
Nathan Vegdahl commented 2024-05-14 16:30:15 +02:00
Outdated
Review
Copy Link

Thanks! That may indeed be the case.

However, that doesn't solve the problem because evaluate_time_fmodifiers() also needs to take those parameters as const for this PR. So it might be a good refactor, but isn't what's needed for this PR.

Thanks! That may indeed be the case. However, that doesn't solve the problem because `evaluate_time_fmodifiers()` *also* needs to take those parameters as const for this PR. So it might be a good refactor, but isn't what's needed for this PR.
Falk David commented 2024-05-14 17:50:51 +02:00
Outdated
Review
Copy Link

Ah you're right indeed.
Looking at add_fmodifier, it seems that it's already not possible to add a FMODIFIER_TYPE_CYCLES after the first one. Maybe this just needs to be an assert instead? Is that code actually ever hit? 😅

Ah you're right indeed. Looking at `add_fmodifier`, it seems that it's already not possible to add a `FMODIFIER_TYPE_CYCLES` after the first one. Maybe this just needs to be an assert instead? Is that code actually ever hit? 😅
Christoph Lendenfeld commented 2024-05-16 12:10:20 +02:00
Outdated
Review
Copy Link

I agree with @filedescriptor here. I don't think you can have a cycles modifier at any other place than the first one. Changing that out with an assert is a valid option. And in the long run we can maybe remove that limitation altogether.

I agree with @filedescriptor here. I don't think you can have a cycles modifier at any other place than the first one. Changing that out with an assert is a valid option. And in the long run we can maybe remove that limitation altogether.
👍 1
Falk David commented 2024-05-16 12:16:21 +02:00
Outdated
Review
Copy Link

Yes, also from what I can tell, all codepaths that add an fcurve modifier go through add_fmodifier. Even the Python API uses it (see rna_FCurve_modifiers_new).

Yes, also from what I can tell, all codepaths that add an fcurve modifier go through `add_fmodifier`. Even the Python API uses it (see `rna_FCurve_modifiers_new`).
👍 1
if (fcu == nullptr || (fcu->bezt == nullptr && fcu->fpt == nullptr)) {
return evaltime;
@ -762,8 +768,11 @@ static float fcm_cycles_time(
return evaltime;
}
static void fcm_cycles_evaluate(
FCurve * /*fcu*/, FModifier * /*fcm*/, float *cvalue, float /*evaltime*/, void *storage_)
static void fcm_cycles_evaluate(const FCurve * /*fcu*/,
const FModifier * /*fcm*/,
float *cvalue,
float /*evaltime*/,
void *storage_)
{
tFCMED_Cycles *storage = static_cast<tFCMED_Cycles *>(storage_);
*cvalue += storage->cycyofs;
@ -800,8 +809,11 @@ static void fcm_noise_new_data(void *mdata)
data->modification = FCM_NOISE_MODIF_REPLACE;
}
static void fcm_noise_evaluate(
FCurve * /*fcu*/, FModifier *fcm, float *cvalue, float evaltime, void * /*storage*/)
static void fcm_noise_evaluate(const FCurve * /*fcu*/,
const FModifier *fcm,
float *cvalue,
float evaltime,
void * /*storage*/)
{
FMod_Noise *data = (FMod_Noise *)fcm->data;
float noise;
@ -874,8 +886,8 @@ static void fcm_python_copy(FModifier *fcm, const FModifier *src)
pymod->prop = IDP_CopyProperty(opymod->prop);
}
static void fcm_python_evaluate(FCurve * /*fcu*/,
FModifier * /*fcm*/,
static void fcm_python_evaluate(const FCurve * /*fcu*/,
const FModifier * /*fcm*/,
float * /*cvalue*/,
float /*evaltime*/,
void * /*storage*/)
@ -907,8 +919,11 @@ static FModifierTypeInfo FMI_PYTHON = {
/* Limits F-Curve Modifier --------------------------- */
static float fcm_limits_time(
FCurve * /*fcu*/, FModifier *fcm, float /*cvalue*/, float evaltime, void * /*storage*/)
static float fcm_limits_time(const FCurve * /*fcu*/,
const FModifier *fcm,
float /*cvalue*/,
float evaltime,
void * /*storage*/)
{
FMod_Limits *data = (FMod_Limits *)fcm->data;
@ -924,8 +939,11 @@ static float fcm_limits_time(
return evaltime;
}
static void fcm_limits_evaluate(
FCurve * /*fcu*/, FModifier *fcm, float *cvalue, float /*evaltime*/, void * /*storage*/)
static void fcm_limits_evaluate(const FCurve * /*fcu*/,
const FModifier *fcm,
float *cvalue,
float /*evaltime*/,
void * /*storage*/)
{
FMod_Limits *data = (FMod_Limits *)fcm->data;
@ -965,8 +983,11 @@ static void fcm_stepped_new_data(void *mdata)
data->step_size = 2.0f;
}
static float fcm_stepped_time(
FCurve * /*fcu*/, FModifier *fcm, float /*cvalue*/, float evaltime, void * /*storage*/)
static float fcm_stepped_time(const FCurve * /*fcu*/,
const FModifier *fcm,
float /*cvalue*/,
float evaltime,
void * /*storage*/)
{
FMod_Stepped *data = (FMod_Stepped *)fcm->data;
int snapblock;
@ -1312,7 +1333,7 @@ bool list_has_suitable_fmodifier(const ListBase *modifiers, int mtype, short act
/* Evaluation API --------------------------- */
uint evaluate_fmodifiers_storage_size_per_modifier(ListBase *modifiers)
uint evaluate_fmodifiers_storage_size_per_modifier(const ListBase *modifiers)
{
/* Sanity checks. */
if (ELEM(nullptr, modifiers, modifiers->first)) {
@ -1385,8 +1406,8 @@ static float eval_fmodifier_influence(FModifier *fcm, float evaltime)
}
float evaluate_time_fmodifiers(FModifiersStackStorage *storage,
ListBase *modifiers,
FCurve *fcu,
const ListBase *modifiers,
const FCurve *fcu,
float cvalue,
float evaltime)
{
@ -1445,8 +1466,8 @@ float evaluate_time_fmodifiers(FModifiersStackStorage *storage,
}
void evaluate_value_fmodifiers(FModifiersStackStorage *storage,
ListBase *modifiers,
FCurve *fcu,
const ListBase *modifiers,
const FCurve *fcu,
float *cvalue,
float evaltime)
{