From 83f8f44791374dd051728e44d89fbdeee15c60aa Mon Sep 17 00:00:00 2001 From: Brecht Van Lommel Date: Tue, 29 Jan 2019 11:59:17 +0100 Subject: Fix T59495, T59992, T59904, T59178, T60598: broken keyframed value editing. This removes a bunch of animation/driver evaluations and recalc flags that should be redundant in the new depsgraph, and were incorrectly affecting the evaluated scene in a permanent way. Still two cases that could be removed if the depsgraph is improved, in BKE_object_handle_data_update and BKE_cachefile_update_frame. For physics subframe interpolation there are also still calls to BKE_object_where_is_calc that should ideally be removed as well, though they are not known to cause keyframing bugs. Differential Revision: https://developer.blender.org/D4274 --- source/blender/blenkernel/BKE_animsys.h | 7 ++++++ source/blender/blenkernel/BKE_object.h | 3 --- source/blender/blenkernel/intern/action.c | 1 - source/blender/blenkernel/intern/anim_sys.c | 28 ++++------------------ source/blender/blenkernel/intern/cachefile.c | 3 ++- source/blender/blenkernel/intern/dynamicpaint.c | 2 -- source/blender/blenkernel/intern/object.c | 22 ++++++++--------- source/blender/blenkernel/intern/object_update.c | 15 +++--------- source/blender/blenkernel/intern/particle_system.c | 5 ---- source/blender/blenkernel/intern/rigidbody.c | 6 +++-- source/blender/blenkernel/intern/scene.c | 2 +- 11 files changed, 33 insertions(+), 61 deletions(-) (limited to 'source/blender/blenkernel') diff --git a/source/blender/blenkernel/BKE_animsys.h b/source/blender/blenkernel/BKE_animsys.h index 3790230153a..618d6523b0c 100644 --- a/source/blender/blenkernel/BKE_animsys.h +++ b/source/blender/blenkernel/BKE_animsys.h @@ -186,6 +186,13 @@ void BKE_animsys_free_nla_keyframing_context_cache(struct ListBase *cache); /* ------------- Main API -------------------- */ /* In general, these ones should be called to do all animation evaluation */ +/* Flags for recalc parameter, indicating which part to recalculate. */ +typedef enum eAnimData_Recalc { + ADT_RECALC_DRIVERS = (1 << 0), + ADT_RECALC_ANIM = (1 << 1), + ADT_RECALC_ALL = (ADT_RECALC_DRIVERS | ADT_RECALC_ANIM), +} eAnimData_Recalc; + /* Evaluation loop for evaluating animation data */ void BKE_animsys_evaluate_animdata(struct Depsgraph *depsgraph, struct Scene *scene, struct ID *id, struct AnimData *adt, float ctime, short recalc); diff --git a/source/blender/blenkernel/BKE_object.h b/source/blender/blenkernel/BKE_object.h index c7d34480a20..7117d8942f4 100644 --- a/source/blender/blenkernel/BKE_object.h +++ b/source/blender/blenkernel/BKE_object.h @@ -166,9 +166,6 @@ void BKE_object_where_is_calc_ex( struct Object *ob, float r_originmat[3][3]); void BKE_object_where_is_calc_time( struct Depsgraph *depsgraph, struct Scene *scene, struct Object *ob, float ctime); -void BKE_object_where_is_calc_time_ex( - struct Depsgraph *depsgraph, struct Scene *scene, struct Object *ob, float ctime, - struct RigidBodyWorld *rbw, float r_originmat[3][3]); void BKE_object_where_is_calc_mat4(struct Object *ob, float obmat[4][4]); /* possibly belong in own moduke? */ diff --git a/source/blender/blenkernel/intern/action.c b/source/blender/blenkernel/intern/action.c index 10ceee487fb..13bcfaf4f32 100644 --- a/source/blender/blenkernel/intern/action.c +++ b/source/blender/blenkernel/intern/action.c @@ -1470,7 +1470,6 @@ void what_does_obaction(Object *ob, Object *workob, bPose *pose, bAction *act, c /* init animdata, and attach to workob */ workob->adt = &adt; - adt.recalc = ADT_RECALC_ANIM; adt.action = act; /* execute effects of Action on to workob (or it's PoseChannels) */ diff --git a/source/blender/blenkernel/intern/anim_sys.c b/source/blender/blenkernel/intern/anim_sys.c index 9aff54ea3bc..5be89212004 100644 --- a/source/blender/blenkernel/intern/anim_sys.c +++ b/source/blender/blenkernel/intern/anim_sys.c @@ -1764,7 +1764,7 @@ static void animsys_evaluate_drivers(PointerRNA *ptr, AnimData *adt, float ctime if ((fcu->flag & (FCURVE_MUTED | FCURVE_DISABLED)) == 0) { /* check if driver itself is tagged for recalculation */ /* XXX driver recalc flag is not set yet by depsgraph! */ - if ((driver) && !(driver->flag & DRIVER_FLAG_INVALID) /*&& (driver->flag & DRIVER_FLAG_RECALC)*/) { + if ((driver) && !(driver->flag & DRIVER_FLAG_INVALID)) { /* evaluate this using values set already in other places * NOTE: for 'layering' option later on, we should check if we should remove old value before adding * new to only be done when drivers only changed */ @@ -1775,9 +1775,6 @@ static void animsys_evaluate_drivers(PointerRNA *ptr, AnimData *adt, float ctime ok = animsys_write_rna_setting(&anim_rna, curval); } - /* clear recalc flag */ - driver->flag &= ~DRIVER_FLAG_RECALC; - /* set error-flag if evaluation failed */ if (ok == 0) driver->flag |= DRIVER_FLAG_INVALID; @@ -3513,7 +3510,7 @@ void BKE_animsys_evaluate_animdata(Depsgraph *depsgraph, Scene *scene, ID *id, A * that overrides 'rough' work in NLA */ /* TODO: need to double check that this all works correctly */ - if ((recalc & ADT_RECALC_ANIM) || (adt->recalc & ADT_RECALC_ANIM)) { + if (recalc & ADT_RECALC_ANIM) { /* evaluate NLA data */ if ((adt->nla_tracks.first) && !(adt->flag & ADT_NLA_EVAL_OFF)) { /* evaluate NLA-stack @@ -3524,9 +3521,6 @@ void BKE_animsys_evaluate_animdata(Depsgraph *depsgraph, Scene *scene, ID *id, A /* evaluate Active Action only */ else if (adt->action) animsys_evaluate_action_ex(depsgraph, &id_ptr, adt->action, ctime); - - /* reset tag */ - adt->recalc &= ~ADT_RECALC_ANIM; } /* recalculate drivers @@ -3534,10 +3528,7 @@ void BKE_animsys_evaluate_animdata(Depsgraph *depsgraph, Scene *scene, ID *id, A * or be layered on top of existing animation data. * - Drivers should be in the appropriate order to be evaluated without problems... */ - if ((recalc & ADT_RECALC_DRIVERS) - /* XXX for now, don't check yet, as depsgraph hasn't been updated */ - /* && (adt->recalc & ADT_RECALC_DRIVERS)*/) - { + if (recalc & ADT_RECALC_DRIVERS) { animsys_evaluate_drivers(&id_ptr, adt, ctime); } @@ -3555,9 +3546,6 @@ void BKE_animsys_evaluate_animdata(Depsgraph *depsgraph, Scene *scene, ID *id, A RNA_property_update_cache_flush(bmain, scene); RNA_property_update_cache_free(); } - - /* clear recalc flag now */ - adt->recalc = 0; } /* Evaluation of all ID-blocks with Animation Data blocks - Animation Data Only @@ -3705,8 +3693,7 @@ void BKE_animsys_eval_animdata(Depsgraph *depsgraph, ID *id) * which should get handled as part of the dependency graph instead... */ DEG_debug_print_eval_time(depsgraph, __func__, id->name, id, ctime); - short recalc = ADT_RECALC_ANIM; - BKE_animsys_evaluate_animdata(depsgraph, scene, id, adt, ctime, recalc); + BKE_animsys_evaluate_animdata(depsgraph, scene, id, adt, ctime, ADT_RECALC_ANIM); } void BKE_animsys_update_driver_array(ID *id) @@ -3758,7 +3745,7 @@ void BKE_animsys_eval_driver(Depsgraph *depsgraph, if ((fcu->flag & (FCURVE_MUTED | FCURVE_DISABLED)) == 0) { /* check if driver itself is tagged for recalculation */ /* XXX driver recalc flag is not set yet by depsgraph! */ - if ((driver_orig) && !(driver_orig->flag & DRIVER_FLAG_INVALID) /*&& (driver_orig->flag & DRIVER_FLAG_RECALC)*/) { + if ((driver_orig) && !(driver_orig->flag & DRIVER_FLAG_INVALID)) { /* evaluate this using values set already in other places * NOTE: for 'layering' option later on, we should check if we should remove old value before adding * new to only be done when drivers only changed */ @@ -3797,11 +3784,6 @@ void BKE_animsys_eval_driver(Depsgraph *depsgraph, } } - //printf("\tnew val = %f\n", fcu->curval); - - /* clear recalc flag */ - driver_orig->flag &= ~DRIVER_FLAG_RECALC; - /* set error-flag if evaluation failed */ if (ok == 0) { printf("invalid driver - %s[%d]\n", fcu->rna_path, fcu->array_index); diff --git a/source/blender/blenkernel/intern/cachefile.c b/source/blender/blenkernel/intern/cachefile.c index 6fcfe468e98..128608b3b51 100644 --- a/source/blender/blenkernel/intern/cachefile.c +++ b/source/blender/blenkernel/intern/cachefile.c @@ -184,7 +184,8 @@ void BKE_cachefile_update_frame( char filename[FILE_MAX]; for (cache_file = bmain->cachefiles.first; cache_file; cache_file = cache_file->id.next) { - /* Execute drivers only, as animation has already been done. */ + /* TODO: dependency graph should be updated to do drivers on cachefile. + * Execute drivers only, as animation has already been done. */ BKE_animsys_evaluate_animdata(depsgraph, scene, &cache_file->id, cache_file->adt, ctime, ADT_RECALC_DRIVERS); if (!cache_file->is_sequence) { diff --git a/source/blender/blenkernel/intern/dynamicpaint.c b/source/blender/blenkernel/intern/dynamicpaint.c index 2cb9bd5c619..74f8f27b601 100644 --- a/source/blender/blenkernel/intern/dynamicpaint.c +++ b/source/blender/blenkernel/intern/dynamicpaint.c @@ -5962,8 +5962,6 @@ static int dynamicPaint_doStep( psys_check_enabled(brushObj, brush->psys, for_render)) { /* Paint a particle system */ - BKE_animsys_evaluate_animdata(depsgraph, scene, &brush->psys->part->id, brush->psys->part->adt, - BKE_scene_frame_get(scene), ADT_RECALC_ANIM); dynamicPaint_paintParticles(surface, brush->psys, brush, timescale); } /* Object center distance: */ diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c index 341a15268ef..f3ff601b7e0 100644 --- a/source/blender/blenkernel/intern/object.c +++ b/source/blender/blenkernel/intern/object.c @@ -2210,15 +2210,10 @@ static void solve_parenting(Object *ob, Object *par, float obmat[4][4], } /* note, scene is the active scene while actual_scene is the scene the object resides in */ -void BKE_object_where_is_calc_time_ex( +static void object_where_is_calc_ex( Depsgraph *depsgraph, Scene *scene, Object *ob, float ctime, RigidBodyWorld *rbw, float r_originmat[3][3]) { - if (ob == NULL) return; - - /* execute drivers only, as animation has already been done */ - BKE_animsys_evaluate_animdata(depsgraph, scene, &ob->id, ob->adt, ctime, ADT_RECALC_DRIVERS); - if (ob->parent) { Object *par = ob->parent; @@ -2249,7 +2244,9 @@ void BKE_object_where_is_calc_time_ex( void BKE_object_where_is_calc_time(Depsgraph *depsgraph, Scene *scene, Object *ob, float ctime) { - BKE_object_where_is_calc_time_ex(depsgraph, scene, ob, ctime, NULL, NULL); + /* Execute drivers and animation. */ + BKE_animsys_evaluate_animdata(depsgraph, scene, &ob->id, ob->adt, ctime, ADT_RECALC_ALL); + object_where_is_calc_ex(depsgraph, scene, ob, ctime, NULL, NULL); } /* get object transformation matrix without recalculating dependencies and @@ -2269,11 +2266,13 @@ void BKE_object_where_is_calc_mat4(Object *ob, float obmat[4][4]) void BKE_object_where_is_calc_ex(Depsgraph *depsgraph, Scene *scene, RigidBodyWorld *rbw, Object *ob, float r_originmat[3][3]) { - BKE_object_where_is_calc_time_ex(depsgraph, scene, ob, DEG_get_ctime(depsgraph), rbw, r_originmat); + float ctime = DEG_get_ctime(depsgraph); + object_where_is_calc_ex(depsgraph, scene, ob, ctime, rbw, r_originmat); } void BKE_object_where_is_calc(Depsgraph *depsgraph, Scene *scene, Object *ob) { - BKE_object_where_is_calc_time_ex(depsgraph, scene, ob, DEG_get_ctime(depsgraph), NULL, NULL); + float ctime = DEG_get_ctime(depsgraph); + object_where_is_calc_ex(depsgraph, scene, ob, ctime, NULL, NULL); } /** @@ -4013,16 +4012,17 @@ bool BKE_object_modifier_update_subframe( /* was originally ID_RECALC_ALL - TODO - which flags are really needed??? */ /* TODO(sergey): What about animation? */ ob->id.recalc |= ID_RECALC_ALL; - BKE_animsys_evaluate_animdata(depsgraph, scene, &ob->id, ob->adt, frame, ADT_RECALC_ANIM); if (update_mesh) { + BKE_animsys_evaluate_animdata(depsgraph, scene, &ob->id, ob->adt, frame, ADT_RECALC_ANIM); /* ignore cache clear during subframe updates * to not mess up cache validity */ object_cacheIgnoreClear(ob, 1); BKE_object_handle_update(depsgraph, scene, ob); object_cacheIgnoreClear(ob, 0); } - else + else { BKE_object_where_is_calc_time(depsgraph, scene, ob, frame); + } /* for curve following objects, parented curve has to be updated too */ if (ob->type == OB_CURVE) { diff --git a/source/blender/blenkernel/intern/object_update.c b/source/blender/blenkernel/intern/object_update.c index 5a25880a7d1..1b071785684 100644 --- a/source/blender/blenkernel/intern/object_update.c +++ b/source/blender/blenkernel/intern/object_update.c @@ -158,22 +158,13 @@ void BKE_object_handle_data_update( Scene *scene, Object *ob) { - ID *data_id = (ID *)ob->data; - AnimData *adt = BKE_animdata_from_id(data_id); - Key *key; float ctime = BKE_scene_frame_get(scene); DEG_debug_print_eval(depsgraph, __func__, ob->id.name, ob); - /* TODO(sergey): Only used by legacy depsgraph. */ - if (adt) { - /* evaluate drivers - datalevel */ - /* XXX: for mesh types, should we push this to evaluated mesh instead? */ - BKE_animsys_evaluate_animdata(depsgraph, scene, data_id, adt, ctime, ADT_RECALC_DRIVERS); - } - - /* TODO(sergey): Only used by legacy depsgraph. */ - key = BKE_key_from_object(ob); + /* TODO: only here to evaluate drivers twice to fix dependency graph + * not handling shape key values that depend on each other. */ + Key *key = BKE_key_from_object(ob); if (key && key->block.first) { if (!(ob->shapeflag & OB_SHAPE_LOCK)) BKE_animsys_evaluate_animdata(depsgraph, scene, &key->id, key->adt, ctime, ADT_RECALC_DRIVERS); diff --git a/source/blender/blenkernel/intern/particle_system.c b/source/blender/blenkernel/intern/particle_system.c index 59eb431b44b..9d04960e77d 100644 --- a/source/blender/blenkernel/intern/particle_system.c +++ b/source/blender/blenkernel/intern/particle_system.c @@ -993,8 +993,6 @@ static void evaluate_emitter_anim(struct Depsgraph *depsgraph, Scene *scene, Obj if (ob->parent) evaluate_emitter_anim(depsgraph, scene, ob->parent, cfra); - /* we have to force RECALC_ANIM here since where_is_objec_time only does drivers */ - BKE_animsys_evaluate_animdata(depsgraph, scene, &ob->id, ob->adt, cfra, ADT_RECALC_ANIM); BKE_object_where_is_calc_time(depsgraph, scene, ob, cfra); } @@ -4239,9 +4237,6 @@ void particle_system_update(struct Depsgraph *depsgraph, Scene *scene, Object *o BKE_mesh_tessface_ensure(sim.psmd->mesh_final); } - /* execute drivers only, as animation has already been done */ - BKE_animsys_evaluate_animdata(depsgraph, scene, &part->id, part->adt, cfra, ADT_RECALC_DRIVERS); - /* to verify if we need to restore object afterwards */ psys->flag &= ~PSYS_OB_ANIM_RESTORE; diff --git a/source/blender/blenkernel/intern/rigidbody.c b/source/blender/blenkernel/intern/rigidbody.c index 67e0834e088..ee9c9be6e7f 100644 --- a/source/blender/blenkernel/intern/rigidbody.c +++ b/source/blender/blenkernel/intern/rigidbody.c @@ -1412,6 +1412,8 @@ static void rigidbody_update_sim_ob(Depsgraph *depsgraph, Scene *scene, RigidBod */ static void rigidbody_update_simulation(Depsgraph *depsgraph, Scene *scene, RigidBodyWorld *rbw, bool rebuild) { + float ctime = DEG_get_ctime(depsgraph); + /* update world */ if (rebuild) BKE_rigidbody_validate_sim_world(scene, rbw, true); @@ -1443,7 +1445,7 @@ static void rigidbody_update_simulation(Depsgraph *depsgraph, Scene *scene, Rigi /* validate that we've got valid object set up here... */ RigidBodyOb *rbo = ob->rigidbody_object; /* update transformation matrix of the object so we don't get a frame of lag for simple animations */ - BKE_object_where_is_calc(depsgraph, scene, ob); + BKE_object_where_is_calc_time(depsgraph, scene, ob, ctime); /* TODO remove this whole block once we are sure we never get NULL rbo here anymore. */ /* This cannot be done in CoW evaluation context anymore... */ @@ -1497,7 +1499,7 @@ static void rigidbody_update_simulation(Depsgraph *depsgraph, Scene *scene, Rigi /* validate that we've got valid object set up here... */ RigidBodyCon *rbc = ob->rigidbody_constraint; /* update transformation matrix of the object so we don't get a frame of lag for simple animations */ - BKE_object_where_is_calc(depsgraph, scene, ob); + BKE_object_where_is_calc_time(depsgraph, scene, ob, ctime); /* TODO remove this whole block once we are sure we never get NULL rbo here anymore. */ /* This cannot be done in CoW evaluation context anymore... */ diff --git a/source/blender/blenkernel/intern/scene.c b/source/blender/blenkernel/intern/scene.c index 126a4dfed47..a3faa3d375c 100644 --- a/source/blender/blenkernel/intern/scene.c +++ b/source/blender/blenkernel/intern/scene.c @@ -1402,7 +1402,7 @@ static void scene_armature_depsgraph_workaround(Main *bmain, Depsgraph *depsgrap return; } for (ob = bmain->object.first; ob; ob = ob->id.next) { - if (ob->type == OB_ARMATURE && ob->adt && ob->adt->recalc & ADT_RECALC_ANIM) { + if (ob->type == OB_ARMATURE && ob->adt) { if (ob->pose == NULL || (ob->pose->flag & POSE_RECALC)) { BKE_pose_rebuild(bmain, ob, ob->data, true); } -- cgit v1.2.3