Fix T56593: Crash when enabling collection with curves

Was missing update of ID blocks which are becoming visible.

7 files changed, 71 insertions, 56 deletions

diff --git a/source/blender/depsgraph/intern/builder/deg_builder.cc b/source/blender/depsgraph/intern/builder/deg_builder.cc
index aacd106ceed..ecf6403199c 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder.cc
@@ -114,7 +114,7 @@ void deg_graph_build_flush_visibility(Depsgraph *graph)
 
 void deg_graph_build_finalize(Main *bmain, Depsgraph *graph)
 {
-	/* Maker sure dependencies of visible ID datablocks are visible. */
+	/* Make sure dependencies of visible ID datablocks are visible. */
 	deg_graph_build_flush_visibility(graph);
 	/* Re-tag IDs for update if it was tagged before the relations
 	 * update tag.
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index 06e37ab5be7..cbe394a3f0b 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -162,13 +162,17 @@ DepsgraphNodeBuilder::~DepsgraphNodeBuilder()
 IDDepsNode *DepsgraphNodeBuilder::add_id_node(ID *id)
 {
 	IDDepsNode *id_node = NULL;
+	ID *id_cow = NULL;
+	bool is_previous_visible = false;
 	IDInfo *id_info = (IDInfo *)BLI_ghash_lookup(id_info_hash_, id);
-	ID *id_cow = (id_info != NULL) ? id_info->id_cow : NULL;
-	if (id_cow != NULL) {
+	if (id_info != NULL) {
+		id_cow = id_info->id_cow;
+		is_previous_visible= id_info->is_visible;
 		/* Tag ID info to not free the CoW ID pointer. */
 		id_info->id_cow = NULL;
 	}
 	id_node = graph_->add_id_node(id, id_cow);
+	id_node->is_previous_visible = is_previous_visible;
 	/* Currently all ID nodes are supposed to have copy-on-write logic.
 	 *
 	 * NOTE: Zero number of components indicates that ID node was just created.
@@ -338,15 +342,17 @@ void DepsgraphNodeBuilder::begin_build()
 	 */
 	id_info_hash_ = BLI_ghash_ptr_new("Depsgraph id hash");
 	foreach (IDDepsNode *id_node, graph_->id_nodes) {
-		if (!deg_copy_on_write_is_expanded(id_node->id_cow)) {
-			continue;
-		}
-		if (id_node->id_orig == id_node->id_cow) {
-			continue;
-		}
 		IDInfo *id_info = (IDInfo *)MEM_mallocN(
 		        sizeof(IDInfo), "depsgraph id info");
-		id_info->id_cow = id_node->id_cow;
+		if (deg_copy_on_write_is_expanded(id_node->id_cow) &&
+		    id_node->id_orig != id_node->id_cow)
+		{
+			id_info->id_cow = id_node->id_cow;
+		}
+		else {
+			id_info->id_cow = NULL;
+		}
+		id_info->is_visible = id_node->is_visible;
 		BLI_ghash_insert(id_info_hash_, id_node->id_orig, id_info);
 		id_node->id_cow = NULL;
 	}
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.h b/source/blender/depsgraph/intern/builder/deg_builder_nodes.h
index b35e07cc333..fcceec99b06 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.h
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.h
@@ -221,6 +221,10 @@ struct DepsgraphNodeBuilder {
 	struct IDInfo {
 		/* Copy-on-written pointer of the corresponding ID. */
 		ID *id_cow;
+		/* State of the is_visible from ID node from previous state of the
+		 * dependency graph.
+		 */
+		bool is_visible;
 	};
 
 protected:
diff --git a/source/blender/depsgraph/intern/depsgraph_build.cc b/source/blender/depsgraph/intern/depsgraph_build.cc
index fccb5808711..1425c1bc453 100644
--- a/source/blender/depsgraph/intern/depsgraph_build.cc
+++ b/source/blender/depsgraph/intern/depsgraph_build.cc
@@ -198,69 +198,49 @@ void DEG_graph_build_from_view_layer(Depsgraph *graph,
 	if (G.debug & G_DEBUG_DEPSGRAPH_BUILD) {
 		start_time = PIL_check_seconds_timer();
 	}
-
 	DEG::Depsgraph *deg_graph = reinterpret_cast<DEG::Depsgraph *>(graph);
+	/* Perform sanity checks. */
 	BLI_assert(BLI_findindex(&scene->view_layers, view_layer) != -1);
-
 	BLI_assert(deg_graph->scene == scene);
 	BLI_assert(deg_graph->view_layer == view_layer);
-
-	/* TODO(sergey): This is a bit tricky, but ensures that all the data
-	 * is evaluated properly when depsgraph is becoming "visible".
-	 *
-	 * This now could happen for both visible scene is changed and extra
-	 * dependency graph was created for render engine.
-	 */
-	const bool need_on_visible_update = (deg_graph->id_nodes.size() == 0);
-
-	/* 1) Generate all the nodes in the graph first */
+	/* Generate all the nodes in the graph first */
 	DEG::DepsgraphNodeBuilder node_builder(bmain, deg_graph);
 	node_builder.begin_build();
 	node_builder.build_view_layer(scene,
 	                               view_layer,
 	                               DEG::DEG_ID_LINKED_DIRECTLY);
 	node_builder.end_build();
-
-	/* 2) Hook up relationships between operations - to determine evaluation
-	 *    order.
+	/* Hook up relationships between operations - to determine evaluation
+	 * order.
 	 */
 	DEG::DepsgraphRelationBuilder relation_builder(bmain, deg_graph);
 	relation_builder.begin_build();
 	relation_builder.build_view_layer(scene, view_layer);
 	relation_builder.build_copy_on_write_relations();
-
 	/* Detect and solve cycles. */
 	DEG::deg_graph_detect_cycles(deg_graph);
-
-	/* 3) Simplify the graph by removing redundant relations (to optimize
-	 *    traversal later). */
+	/* Simplify the graph by removing redundant relations (to optimize
+	 * traversal later). */
 	/* TODO: it would be useful to have an option to disable this in cases where
 	 *       it is causing trouble.
 	 */
 	if (G.debug_value == 799) {
 		DEG::deg_graph_transitive_reduction(deg_graph);
 	}
-
-	/* 4) Flush visibility layer and re-schedule nodes for update. */
+	/* Store pointers to commonly used valuated datablocks. */
+	deg_graph->scene_cow = (Scene *)deg_graph->get_cow_id(&deg_graph->scene->id);
+	/* Flush visibility layer and re-schedule nodes for update. */
 	DEG::deg_graph_build_finalize(bmain, deg_graph);
-
+	DEG_graph_on_visible_update(bmain, graph);
 #if 0
 	if (!DEG_debug_consistency_check(deg_graph)) {
 		printf("Consistency validation failed, ABORTING!\n");
 		abort();
 	}
 #endif
-
 	/* Relations are up to date. */
 	deg_graph->need_update = false;
-
-	/* Store pointers to commonly used valuated datablocks. */
-	deg_graph->scene_cow = (Scene *)deg_graph->get_cow_id(&deg_graph->scene->id);
-
-	if (need_on_visible_update) {
-		DEG_graph_on_visible_update(bmain, graph);
-	}
-
+	/* Finish statistics. */
 	if (G.debug & G_DEBUG_DEPSGRAPH_BUILD) {
 		printf("Depsgraph built in %f seconds.\n",
 		       PIL_check_seconds_timer() - start_time);
diff --git a/source/blender/depsgraph/intern/depsgraph_tag.cc b/source/blender/depsgraph/intern/depsgraph_tag.cc
index e2c28f0d1e1..42db6af797b 100644
--- a/source/blender/depsgraph/intern/depsgraph_tag.cc
+++ b/source/blender/depsgraph/intern/depsgraph_tag.cc
@@ -67,6 +67,7 @@ extern "C" {
 #include "DEG_depsgraph_query.h"
 
 #include "intern/builder/deg_builder.h"
+#include "intern/eval/deg_eval_copy_on_write.h"
 #include "intern/eval/deg_eval_flush.h"
 #include "intern/nodes/deg_node.h"
 #include "intern/nodes/deg_node_component.h"
@@ -519,33 +520,49 @@ void deg_id_tag_update(Main *bmain, ID *id, int flag)
 
 void deg_graph_on_visible_update(Main *bmain, Depsgraph *graph)
 {
-	/* Make sure objects are up to date. */
 	foreach (DEG::IDDepsNode *id_node, graph->id_nodes) {
-		const ID_Type id_type = GS(id_node->id_orig->name);
-		int flag = DEG_TAG_COPY_ON_WRITE;
+		if (!id_node->is_visible) {
+			/* ID is not visible within the current dependency graph, no need
+			 * botherwith it to tag or anything.
+			 */
+			continue;
+		}
+		if (id_node->is_previous_visible) {
+			/* The ID was already visible and evaluated, all the subsequent
+			 * updates and tags are to be done explicitly.
+			 */
+			continue;
+		}
+		int flag = 0;
+		if (!DEG::deg_copy_on_write_is_expanded(id_node->id_cow)) {
+			flag |= DEG_TAG_COPY_ON_WRITE;
+		}
 		/* We only tag components which needs an update. Tagging everything is
 		 * not a good idea because that might reset particles cache (or any
 		 * other type of cache).
 		 *
 		 * TODO(sergey): Need to generalize this somehow.
 		 */
+		const ID_Type id_type = GS(id_node->id_orig->name);
 		if (id_type == ID_OB) {
 			flag |= OB_RECALC_OB | OB_RECALC_DATA;
 		}
 		deg_graph_id_tag_update(bmain, graph, id_node->id_orig, flag);
-	}
-	/* Make sure collection properties are up to date. */
-	for (Scene *scene_iter = graph->scene;
-	     scene_iter != NULL;
-	     scene_iter = scene_iter->set)
-	{
-		IDDepsNode *scene_id_node = graph->find_id_node(&scene_iter->id);
-		if (scene_id_node != NULL) {
-			scene_id_node->tag_update(graph);
-		}
-		else {
-			BLI_assert(graph->need_update);
+		if (id_type == ID_SCE) {
+			/* Make sure collection properties are up to date. */
+			id_node->tag_update(graph);
 		}
+		/* Now when ID is updated to the new visibility state, prevent it from
+		 * being re-tagged again. Simplest way to do so is to pretend that it
+		 * was already updated by the "previous" dependency graph.
+		 *
+		 * NOTE: Even if the on_visible_update() is called from the state when
+		 * dependency graph is tagged for relations update, it will be fine:
+		 * since dependency graph builder re-schedules entry tags, all the
+		 * tags we request from here will be applied in the updated state of
+		 * dependency graph.
+		 */
+		id_node->is_previous_visible = true;
 	}
 }
 
diff --git a/source/blender/depsgraph/intern/nodes/deg_node_id.cc b/source/blender/depsgraph/intern/nodes/deg_node_id.cc
index f2464cf98bd..b522efa6d0d 100644
--- a/source/blender/depsgraph/intern/nodes/deg_node_id.cc
+++ b/source/blender/depsgraph/intern/nodes/deg_node_id.cc
@@ -105,6 +105,7 @@ void IDDepsNode::init(const ID *id, const char *UNUSED(subdata))
 	eval_flags = 0;
 	linked_state = DEG_ID_LINKED_INDIRECTLY;
 	is_visible = true;
+	is_previous_visible = false;
 
 	components = BLI_ghash_new(id_deps_node_hash_key,
 	                           id_deps_node_hash_key_cmp,
diff --git a/source/blender/depsgraph/intern/nodes/deg_node_id.h b/source/blender/depsgraph/intern/nodes/deg_node_id.h
index 8af7b80973b..572abb90fab 100644
--- a/source/blender/depsgraph/intern/nodes/deg_node_id.h
+++ b/source/blender/depsgraph/intern/nodes/deg_node_id.h
@@ -81,6 +81,13 @@ struct IDDepsNode : public DepsNode {
 	 * the dependency graph.
 	 */
 	bool is_visible;
+	/* Is used to detect when ID becomes visible within a dependency graph,
+	 * this value equals to:
+	 *   - False if the ID was never inside of the dependency graph.
+	 *   - Value if is_visible of ID node from the previous state of the
+	 *     dependency graph.
+	 */
+	bool is_previous_visible;
 
 	DEG_DEPSNODE_DECLARE;
 };