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

11 files changed, 33 insertions, 61 deletions

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);
 			}