[Bf-blender-cvs] [fe00175] master: Fix crash happening in Cycles fcurve modifier

Sergey Sharybin noreply at git.blender.org
Wed Jan 1 17:37:56 CET 2014


Commit: fe00175c35a301a68518c583a4fab163ac8f32a0
Author: Sergey Sharybin
Date:   Sun Dec 29 19:15:37 2013 +0600
https://developer.blender.org/rBfe00175c35a301a68518c583a4fab163ac8f32a0

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

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

M	source/blender/blenkernel/BKE_fcurve.h
M	source/blender/blenkernel/intern/anim_sys.c
M	source/blender/blenkernel/intern/fcurve.c
M	source/blender/blenkernel/intern/fmodifier.c
M	source/blender/blenloader/intern/readfile.c
M	source/blender/makesdna/DNA_anim_types.h

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

diff --git a/source/blender/blenkernel/BKE_fcurve.h b/source/blender/blenkernel/BKE_fcurve.h
index c31dd74..5ad378b 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 cb628db..609932a 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 32098c6..04765a7 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 f24edfe..3436728 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 

@@ Diff output truncated at 10240 characters. @@




More information about the Bf-blender-cvs mailing list