diff options
author | Sergey Sharybin <sergey.vfx@gmail.com> | 2013-12-29 17:15:37 +0400 |
---|---|---|
committer | Sergey Sharybin <sergey.vfx@gmail.com> | 2014-01-01 20:32:48 +0400 |
commit | fe00175c35a301a68518c583a4fab163ac8f32a0 (patch) | |
tree | 284c8b5a7131571c5b62dde861c0baa968fa41cc | |
parent | 5d701c6d25a1c5cea65b15cbcc88b4c745164cc2 (diff) |
Fix crash happening in Cycles fcurve modifier
Summary:
Crash was happening because of fcurve modifier stack
used modifier's DNA to store temporary data.
Now made it so storage for such a thing is being
allocated locally per object update so multiple objects
which shares the same animation wouldn't run into
threading conflict anymore.
This storage might be a part of EvaluationContext,
but that'd mean passing this context all over in
object_where_is which will clutter API for now without
actual benefit for this.
Optimization notes: storage is only being allocated
if there're Cycles modifier in the stack, so there're
no extra allocations happening in all other cases.
To make code a bit less cluttered with this storage
passing all over the place added extra callbacks to
the FModifier storage which runs evaluation with the
given storage.
Reviewers: brecht, campbellbarton, aligorith
CC: plasmasolutions
Differential Revision: https://developer.blender.org/D147
-rw-r--r-- | source/blender/blenkernel/BKE_fcurve.h | 17 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/anim_sys.c | 11 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/fcurve.c | 12 | ||||
-rw-r--r-- | source/blender/blenkernel/intern/fmodifier.c | 149 | ||||
-rw-r--r-- | source/blender/blenloader/intern/readfile.c | 1 | ||||
-rw-r--r-- | source/blender/makesdna/DNA_anim_types.h | 1 |
6 files changed, 154 insertions, 37 deletions
diff --git a/source/blender/blenkernel/BKE_fcurve.h b/source/blender/blenkernel/BKE_fcurve.h index c31dd745911..5ad378bc331 100644 --- a/source/blender/blenkernel/BKE_fcurve.h +++ b/source/blender/blenkernel/BKE_fcurve.h @@ -99,6 +99,8 @@ float driver_get_variable_value(struct ChannelDriver *driver, struct DriverVar * /* ************** F-Curve Modifiers *************** */ +typedef struct GHash FModifierStackStorage; + /* F-Curve Modifier Type-Info (fmi): * This struct provides function pointers for runtime, so that functions can be * written more generally (with fewer/no special exceptions for various modifiers). @@ -134,6 +136,10 @@ typedef struct FModifierTypeInfo { float (*evaluate_modifier_time)(struct FCurve *fcu, struct FModifier *fcm, float cvalue, float evaltime); /* evaluate the modifier for the given time and 'accumulated' value */ void (*evaluate_modifier)(struct FCurve *fcu, struct FModifier *fcm, float *cvalue, float evaltime); + + /* Same as above but for modifiers which requires storage */ + float (*evaluate_modifier_time_storage)(FModifierStackStorage *storage, struct FCurve *fcu, struct FModifier *fcm, float cvalue, float evaltime); + void (*evaluate_modifier_storage)(FModifierStackStorage *storage, struct FCurve *fcu, struct FModifier *fcm, float *cvalue, float evaltime); } FModifierTypeInfo; /* Values which describe the behavior of a FModifier Type */ @@ -157,7 +163,10 @@ typedef enum eFMI_Requirement_Flags { */ FMI_REQUIRES_NOTHING = (1 << 1), /* refer to modifier instance */ - FMI_REQUIRES_RUNTIME_CHECK = (1 << 2) + FMI_REQUIRES_RUNTIME_CHECK = (1 << 2), + + /* Requires to store data shared between time and valua evaluation */ + FMI_REQUIRES_STORAGE = (1 << 3) } eFMI_Requirement_Flags; /* Function Prototypes for FModifierTypeInfo's */ @@ -177,8 +186,10 @@ void set_active_fmodifier(ListBase *modifiers, struct FModifier *fcm); short list_has_suitable_fmodifier(ListBase *modifiers, int mtype, short acttype); -float evaluate_time_fmodifiers(ListBase *modifiers, struct FCurve *fcu, float cvalue, float evaltime); -void evaluate_value_fmodifiers(ListBase *modifiers, struct FCurve *fcu, float *cvalue, float evaltime); +FModifierStackStorage *evaluate_fmodifiers_storage_new(ListBase *modifiers); +void evaluate_fmodifiers_storage_free(FModifierStackStorage *storage); +float evaluate_time_fmodifiers(FModifierStackStorage *storage, ListBase *modifiers, struct FCurve *fcu, float cvalue, float evaltime); +void evaluate_value_fmodifiers(FModifierStackStorage *storage, ListBase *modifiers, struct FCurve *fcu, float *cvalue, float evaltime); void fcurve_bake_modifiers(struct FCurve *fcu, int start, int end); diff --git a/source/blender/blenkernel/intern/anim_sys.c b/source/blender/blenkernel/intern/anim_sys.c index cb628db8cd2..609932ae5e2 100644 --- a/source/blender/blenkernel/intern/anim_sys.c +++ b/source/blender/blenkernel/intern/anim_sys.c @@ -1981,6 +1981,7 @@ static void nlaeval_fmodifiers_split_stacks(ListBase *list1, ListBase *list2) /* evaluate action-clip strip */ static void nlastrip_evaluate_actionclip(PointerRNA *ptr, ListBase *channels, ListBase *modifiers, NlaEvalStrip *nes) { + FModifierStackStorage *storage; ListBase tmp_modifiers = {NULL, NULL}; NlaStrip *strip = nes->strip; FCurve *fcu; @@ -2001,7 +2002,8 @@ static void nlastrip_evaluate_actionclip(PointerRNA *ptr, ListBase *channels, Li nlaeval_fmodifiers_join_stacks(&tmp_modifiers, &strip->modifiers, modifiers); /* evaluate strip's modifiers which modify time to evaluate the base curves at */ - evaltime = evaluate_time_fmodifiers(&tmp_modifiers, NULL, 0.0f, strip->strip_time); + storage = evaluate_fmodifiers_storage_new(&tmp_modifiers); + evaltime = evaluate_time_fmodifiers(storage, &tmp_modifiers, NULL, 0.0f, strip->strip_time); /* evaluate all the F-Curves in the action, saving the relevant pointers to data that will need to be used */ for (fcu = strip->act->curves.first; fcu; fcu = fcu->next) { @@ -2022,7 +2024,7 @@ static void nlastrip_evaluate_actionclip(PointerRNA *ptr, ListBase *channels, Li /* apply strip's F-Curve Modifiers on this value * NOTE: we apply the strip's original evaluation time not the modified one (as per standard F-Curve eval) */ - evaluate_value_fmodifiers(&tmp_modifiers, fcu, &value, strip->strip_time); + evaluate_value_fmodifiers(storage, &tmp_modifiers, fcu, &value, strip->strip_time); /* get an NLA evaluation channel to work with, and accumulate the evaluated value with the value(s) @@ -2032,7 +2034,10 @@ static void nlastrip_evaluate_actionclip(PointerRNA *ptr, ListBase *channels, Li if (nec) nlaevalchan_accumulate(nec, nes, value); } - + + /* free temporary storage */ + evaluate_fmodifiers_storage_free(storage); + /* unlink this strip's modifiers from the parent's modifiers again */ nlaeval_fmodifiers_split_stacks(&strip->modifiers, modifiers); } diff --git a/source/blender/blenkernel/intern/fcurve.c b/source/blender/blenkernel/intern/fcurve.c index 32098c67ca7..04765a74b77 100644 --- a/source/blender/blenkernel/intern/fcurve.c +++ b/source/blender/blenkernel/intern/fcurve.c @@ -2138,6 +2138,7 @@ static float fcurve_eval_samples(FCurve *fcu, FPoint *fpts, float evaltime) */ float evaluate_fcurve(FCurve *fcu, float evaltime) { + FModifierStackStorage *storage; float cvalue = 0.0f; float devaltime; @@ -2177,9 +2178,10 @@ float evaluate_fcurve(FCurve *fcu, float evaltime) } } } - + /* evaluate modifiers which modify time to evaluate the base curve at */ - devaltime = evaluate_time_fmodifiers(&fcu->modifiers, fcu, cvalue, evaltime); + storage = evaluate_fmodifiers_storage_new(&fcu->modifiers); + 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 @@ -2191,8 +2193,10 @@ float evaluate_fcurve(FCurve *fcu, float evaltime) cvalue = fcurve_eval_samples(fcu, fcu->fpt, devaltime); /* evaluate modifiers */ - evaluate_value_fmodifiers(&fcu->modifiers, fcu, &cvalue, evaltime); - + evaluate_value_fmodifiers(storage, &fcu->modifiers, fcu, &cvalue, evaltime); + + evaluate_fmodifiers_storage_free(storage); + /* if curve can only have integral values, perform truncation (i.e. drop the decimal part) * here so that the curve can be sampled correctly */ diff --git a/source/blender/blenkernel/intern/fmodifier.c b/source/blender/blenkernel/intern/fmodifier.c index f24edfea136..3436728254b 100644 --- a/source/blender/blenkernel/intern/fmodifier.c +++ b/source/blender/blenkernel/intern/fmodifier.c @@ -42,6 +42,7 @@ #include "BLF_translation.h" #include "BLI_blenlib.h" +#include "BLI_ghash.h" #include "BLI_noise.h" #include "BLI_math.h" /* windows needs for M_PI */ #include "BLI_utildefines.h" @@ -51,6 +52,11 @@ /* ******************************** F-Modifiers ********************************* */ +/* Forward declarations. */ +void fmodifiers_storage_put(FModifierStackStorage *storage, FModifier *fcm, void *data); +void fmodifiers_storage_remove(FModifierStackStorage *storage, FModifier *fcm); +void *fmodifiers_storage_get(FModifierStackStorage *storage, FModifier *fcm); + /* Info ------------------------------- */ /* F-Modifiers are modifiers which operate on F-Curves. However, they can also be defined @@ -89,7 +95,9 @@ static FModifierTypeInfo FMI_MODNAME = { fcm_modname_new_data, /* new data */ fcm_modname_verify, /* verify */ fcm_modname_time, /* evaluate time */ - fcm_modname_evaluate /* evaluate */ + fcm_modname_evaluate, /* evaluate */ + fcm_modname_time_storage, /* evaluate time with storage */ + fcm_modname_evaluate_storage /* evaluate with storage */ }; #endif @@ -240,7 +248,9 @@ static FModifierTypeInfo FMI_GENERATOR = { fcm_generator_new_data, /* new data */ fcm_generator_verify, /* verify */ NULL, /* evaluate time */ - fcm_generator_evaluate /* evaluate */ + fcm_generator_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; /* Built-In Function Generator F-Curve Modifier --------------------------- */ @@ -362,7 +372,9 @@ static FModifierTypeInfo FMI_FN_GENERATOR = { fcm_fn_generator_new_data, /* new data */ NULL, /* verify */ NULL, /* evaluate time */ - fcm_fn_generator_evaluate /* evaluate */ + fcm_fn_generator_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; /* Envelope F-Curve Modifier --------------------------- */ @@ -469,7 +481,9 @@ static FModifierTypeInfo FMI_ENVELOPE = { fcm_envelope_new_data, /* new data */ fcm_envelope_verify, /* verify */ NULL, /* evaluate time */ - fcm_envelope_evaluate /* evaluate */ + fcm_envelope_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; /* exported function for finding points */ @@ -585,7 +599,8 @@ static void fcm_cycles_new_data(void *mdata) data->before_mode = data->after_mode = FCM_EXTRAPOLATE_CYCLIC; } -static float fcm_cycles_time(FCurve *fcu, FModifier *fcm, float UNUSED(cvalue), float evaltime) +static float fcm_cycles_time(FModifierStackStorage *storage, FCurve *fcu, FModifier *fcm, + float UNUSED(cvalue), float evaltime) { FMod_Cycles *data = (FMod_Cycles *)fcm->data; float prevkey[2], lastkey[2], cycyofs = 0.0f; @@ -717,18 +732,21 @@ static float fcm_cycles_time(FCurve *fcu, FModifier *fcm, float UNUSED(cvalue), tFCMED_Cycles *edata; /* for now, this is just a float, but we could get more stuff... */ - fcm->edata = edata = MEM_callocN(sizeof(tFCMED_Cycles), "tFCMED_Cycles"); + edata = MEM_callocN(sizeof(tFCMED_Cycles), "tFCMED_Cycles"); edata->cycyofs = cycyofs; + + fmodifiers_storage_put(storage, fcm, edata); } /* return the new frame to evaluate */ return evaltime; } -static void fcm_cycles_evaluate(FCurve *UNUSED(fcu), FModifier *fcm, float *cvalue, float UNUSED(evaltime)) +static void fcm_cycles_evaluate(FModifierStackStorage *storage, FCurve *UNUSED(fcu), + FModifier *fcm, float *cvalue, float UNUSED(evaltime)) { - tFCMED_Cycles *edata = (tFCMED_Cycles *)fcm->edata; - + tFCMED_Cycles *edata = fmodifiers_storage_get(storage, fcm); + /* use temp data */ if (edata) { /* add cyclic offset - no need to check for now, otherwise the data wouldn't exist! */ @@ -736,7 +754,7 @@ static void fcm_cycles_evaluate(FCurve *UNUSED(fcu), FModifier *fcm, float *cval /* free temp data */ MEM_freeN(edata); - fcm->edata = NULL; + fmodifiers_storage_remove(storage, fcm); } } @@ -744,15 +762,17 @@ static FModifierTypeInfo FMI_CYCLES = { FMODIFIER_TYPE_CYCLES, /* type */ sizeof(FMod_Cycles), /* size */ FMI_TYPE_EXTRAPOLATION, /* action type */ - FMI_REQUIRES_ORIGINAL_DATA, /* requirements */ + FMI_REQUIRES_ORIGINAL_DATA | FMI_REQUIRES_STORAGE, /* requirements */ N_("Cycles"), /* name */ "FMod_Cycles", /* struct name */ NULL, /* free data */ NULL, /* copy data */ fcm_cycles_new_data, /* new data */ NULL /*fcm_cycles_verify*/, /* verify */ - fcm_cycles_time, /* evaluate time */ - fcm_cycles_evaluate /* evaluate */ + NULL, /* evaluate time */ + NULL, /* evaluate */ + fcm_cycles_time, /* evaluate time with storage */ + fcm_cycles_evaluate /* evaluate with storage */ }; /* Noise F-Curve Modifier --------------------------- */ @@ -810,7 +830,9 @@ static FModifierTypeInfo FMI_NOISE = { fcm_noise_new_data, /* new data */ NULL /*fcm_noise_verify*/, /* verify */ NULL, /* evaluate time */ - fcm_noise_evaluate /* evaluate */ + fcm_noise_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; /* Filter F-Curve Modifier --------------------------- */ @@ -828,7 +850,9 @@ static FModifierTypeInfo FMI_FILTER = { NULL, /* new data */ NULL /*fcm_filter_verify*/, /* verify */ NULL, /* evlauate time */ - fcm_filter_evaluate /* evaluate */ + fcm_filter_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; #endif // XXX not yet implemented @@ -884,7 +908,9 @@ static FModifierTypeInfo FMI_PYTHON = { fcm_python_new_data, /* new data */ NULL /*fcm_python_verify*/, /* verify */ NULL /*fcm_python_time*/, /* evaluate time */ - fcm_python_evaluate /* evaluate */ + fcm_python_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; @@ -927,7 +953,9 @@ static FModifierTypeInfo FMI_LIMITS = { NULL, /* new data */ NULL, /* verify */ fcm_limits_time, /* evaluate time */ - fcm_limits_evaluate /* evaluate */ + fcm_limits_evaluate, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; /* Stepped F-Curve Modifier --------------------------- */ @@ -980,7 +1008,9 @@ static FModifierTypeInfo FMI_STEPPED = { fcm_stepped_new_data, /* new data */ NULL, /* verify */ fcm_stepped_time, /* evaluate time */ - NULL /* evaluate */ + NULL, /* evaluate */ + NULL, /* evaluate time with storage */ + NULL /* evaluate with storage */ }; /* F-Curve Modifier API --------------------------- */ @@ -1256,6 +1286,60 @@ short list_has_suitable_fmodifier(ListBase *modifiers, int mtype, short acttype) /* Evaluation API --------------------------- */ +FModifierStackStorage *evaluate_fmodifiers_storage_new(ListBase *modifiers) +{ + FModifier *fcm; + + /* Sanity checks. */ + if (ELEM(NULL, modifiers, modifiers->last)) { + return NULL; + } + + for (fcm = modifiers->last; fcm; fcm = fcm->prev) { + FModifierTypeInfo *fmi = fmodifier_get_typeinfo(fcm); + + if (fmi == NULL) { + continue; + } + + if (fmi->requires & FMI_REQUIRES_STORAGE) { + return (FModifierStackStorage *) BLI_ghash_new(BLI_ghashutil_ptrhash, + BLI_ghashutil_ptrcmp, + "fmodifier stack temp storage"); + } + } + + return NULL; +} + +void evaluate_fmodifiers_storage_free(FModifierStackStorage *storage) +{ + if (storage != NULL) { + BLI_ghash_free((GHash *) storage, NULL, NULL); + } +} + +void fmodifiers_storage_put(FModifierStackStorage *storage, FModifier *fcm, void *data) +{ + BLI_assert(storage != NULL); + + BLI_ghash_insert((GHash *) storage, fcm, data); +} + +void fmodifiers_storage_remove(FModifierStackStorage *storage, FModifier *fcm) +{ + BLI_assert(storage != NULL); + + BLI_ghash_remove((GHash *) storage, fcm, NULL, NULL); +} + +void *fmodifiers_storage_get(FModifierStackStorage *storage, FModifier *fcm) +{ + BLI_assert(storage != NULL); + + return BLI_ghash_lookup((GHash *) storage, fcm); +} + /* helper function - calculate influence of FModifier */ static float eval_fmodifier_influence(FModifier *fcm, float evaltime) { @@ -1306,7 +1390,8 @@ static float eval_fmodifier_influence(FModifier *fcm, float evaltime) * so nevaltime gets set to whatever the last time-modifying modifier likes... * - we start from the end of the stack, as only the last one matters for now */ -float evaluate_time_fmodifiers(ListBase *modifiers, FCurve *fcu, float cvalue, float evaltime) +float evaluate_time_fmodifiers(FModifierStackStorage *storage, ListBase *modifiers, + FCurve *fcu, float cvalue, float evaltime) { FModifier *fcm; @@ -1337,10 +1422,17 @@ float evaluate_time_fmodifiers(ListBase *modifiers, FCurve *fcu, float cvalue, f ((fcm->sfra <= evaltime) && (fcm->efra >= evaltime)) ) { /* only evaluate if there's a callback for this */ - if (fmi->evaluate_modifier_time) { + if (fmi->evaluate_modifier_time || fmi->evaluate_modifier_time_storage) { if ((fcm->flag & (FMODIFIER_FLAG_DISABLED | FMODIFIER_FLAG_MUTED)) == 0) { float influence = eval_fmodifier_influence(fcm, evaltime); - float nval = fmi->evaluate_modifier_time(fcu, fcm, cvalue, evaltime); + float nval; + + if ((fmi->requires & FMI_REQUIRES_STORAGE) == 0) { + nval = fmi->evaluate_modifier_time(fcu, fcm, cvalue, evaltime); + } + else { + nval = fmi->evaluate_modifier_time_storage(storage, fcu, fcm, cvalue, evaltime); + } evaltime = interpf(nval, evaltime, influence); } @@ -1355,7 +1447,8 @@ float evaluate_time_fmodifiers(ListBase *modifiers, FCurve *fcu, float cvalue, f /* Evaluates the given set of F-Curve Modifiers using the given data * Should only be called after evaluate_time_fmodifiers() has been called... */ -void evaluate_value_fmodifiers(ListBase *modifiers, FCurve *fcu, float *cvalue, float evaltime) +void evaluate_value_fmodifiers(FModifierStackStorage *storage, ListBase *modifiers, + FCurve *fcu, float *cvalue, float evaltime) { FModifier *fcm; @@ -1374,12 +1467,18 @@ void evaluate_value_fmodifiers(ListBase *modifiers, FCurve *fcu, float *cvalue, if ((fcm->flag & FMODIFIER_FLAG_RANGERESTRICT) == 0 || ((fcm->sfra <= evaltime) && (fcm->efra >= evaltime)) ) { - if (fmi->evaluate_modifier) { + if (fmi->evaluate_modifier || fmi->evaluate_modifier_storage) { if ((fcm->flag & (FMODIFIER_FLAG_DISABLED | FMODIFIER_FLAG_MUTED)) == 0) { float influence = eval_fmodifier_influence(fcm, evaltime); float nval = *cvalue; - - fmi->evaluate_modifier(fcu, fcm, &nval, evaltime); + + if ((fmi->requires & FMI_REQUIRES_STORAGE) == 0) { + fmi->evaluate_modifier(fcu, fcm, &nval, evaltime); + } + else { + fmi->evaluate_modifier_storage(storage, fcu, fcm, &nval, evaltime); + } + *cvalue = interpf(nval, *cvalue, influence); } } diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c index ed501d50b07..81018df7a88 100644 --- a/source/blender/blenloader/intern/readfile.c +++ b/source/blender/blenloader/intern/readfile.c @@ -2028,7 +2028,6 @@ static void direct_link_fmodifiers(FileData *fd, ListBase *list) for (fcm = list->first; fcm; fcm = fcm->next) { /* relink general data */ fcm->data = newdataadr(fd, fcm->data); - fcm->edata = NULL; /* do relinking of data for specific types */ switch (fcm->type) { diff --git a/source/blender/makesdna/DNA_anim_types.h b/source/blender/makesdna/DNA_anim_types.h index 7a7e08138b0..6f5c9254a38 100644 --- a/source/blender/makesdna/DNA_anim_types.h +++ b/source/blender/makesdna/DNA_anim_types.h @@ -53,7 +53,6 @@ typedef struct FModifier { struct FModifier *next, *prev; void *data; /* pointer to modifier data */ - void *edata; /* pointer to temporary data used during evaluation */ char name[64]; /* user-defined description for the modifier */ short type; /* type of f-curve modifier */ |