Depsgraph: Fix wrong ID remapping when armature object is constructed prior to it's targets

Previously it was possible to run into situation when armature is constructed prior to
objects which are used for it's constraints. This was causing wrong scene evaluation.

Now we create placeholders for objects used by armature in case they don't have ID node
yet, which ensures we have proper mapping from original to copy-on-write ID pointer.

7 files changed, 78 insertions, 31 deletions

diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index b8e64584b14..35542be7aa6 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -521,10 +521,12 @@ void DepsgraphNodeBuilder::build_animdata(ID *id)
 	if (adt == NULL) {
 		return;
 	}
-	ID *id_cow = get_cow_id(id);
 
 	/* animation */
 	if (adt->action || adt->nla_tracks.first || adt->drivers.first) {
+		(void) add_id_node(id);
+		ID *id_cow = get_cow_id(id);
+
 		// XXX: Hook up specific update callbacks for special properties which
 		// may need it...
 
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes_layer.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes_layer.cc
index b6df176545e..861c2377567 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes_layer.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes_layer.cc
@@ -100,7 +100,8 @@ void DepsgraphNodeBuilder::build_scene_layer_collections(Scene *scene)
 	/* Make sure we've got ID node, so we can get pointer to CoW datablock. */
 	IDDepsNode *id_node = add_id_node(&scene->id);
 	Scene *scene_cow = (Scene *)deg_expand_copy_on_write_datablock(m_graph,
-	                                                               id_node);
+	                                                               id_node,
+	                                                               true);
 #else
 	Scene *scene_cow = scene;
 #endif
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes_rig.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes_rig.cc
index 69f02603f40..d567dd02740 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes_rig.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes_rig.cc
@@ -142,10 +142,10 @@ void DepsgraphNodeBuilder::build_rig(Scene *scene, Object *object)
 	Scene *scene_cow = get_cow_datablock(scene);
 	IDDepsNode *object_id_node = add_id_node(&object->id);
 	Object *object_cow = (Object *)deg_expand_copy_on_write_datablock(
-	        m_graph, object_id_node);
+	        m_graph, object_id_node, true);
 	IDDepsNode *armature_id_node = add_id_node(&armature->id);
 	bArmature *armature_cow = (bArmature *)deg_expand_copy_on_write_datablock(
-	        m_graph, armature_id_node);
+	        m_graph, armature_id_node, true);
 #else
 	Scene *scene_cow = scene;
 	Object *object_cow = object;
diff --git a/source/blender/depsgraph/intern/depsgraph.cc b/source/blender/depsgraph/intern/depsgraph.cc
index e36ecf84539..98e476fbecd 100644
--- a/source/blender/depsgraph/intern/depsgraph.cc
+++ b/source/blender/depsgraph/intern/depsgraph.cc
@@ -275,19 +275,23 @@ IDDepsNode *Depsgraph::find_id_node(const ID *id) const
 	return reinterpret_cast<IDDepsNode *>(BLI_ghash_lookup(id_hash, id));
 }
 
-IDDepsNode *Depsgraph::add_id_node(ID *id, const char *name)
+IDDepsNode *Depsgraph::add_id_node(ID *id, const char *name, bool do_tag)
 {
 	IDDepsNode *id_node = find_id_node(id);
 	if (!id_node) {
 		DepsNodeFactory *factory = deg_get_node_factory(DEG_NODE_TYPE_ID_REF);
 		id_node = (IDDepsNode *)factory->create_node(id, "", name);
-		id->tag |= LIB_TAG_DOIT;
+		if (do_tag) {
+			id->tag |= LIB_TAG_DOIT;
+		}
 		/* Register node in ID hash.
 		 *
 		 * NOTE: We address ID nodes by the original ID pointer they are
 		 * referencing to.
 		 */
 		BLI_ghash_insert(id_hash, id, id_node);
+	} else if (do_tag) {
+		id->tag |= LIB_TAG_DOIT;
 	}
 	return id_node;
 }
@@ -389,9 +393,9 @@ DepsRelation::~DepsRelation()
 void Depsgraph::add_entry_tag(OperationDepsNode *node)
 {
 	/* Sanity check. */
-	if (!node)
+	if (node == NULL) {
 		return;
-
+	}
 	/* Add to graph-level set of directly modified nodes to start searching from.
 	 * NOTE: this is necessary since we have several thousand nodes to play with...
 	 */
@@ -427,6 +431,17 @@ ID *Depsgraph::get_cow_id(const ID *id_orig) const
 	return id_node->id_cow;
 }
 
+ID *Depsgraph::ensure_cow_id(ID *id_orig)
+{
+	if (id_orig->tag & LIB_TAG_COPY_ON_WRITE) {
+		/* ID is already remapped to copy-on-write. */
+		return id_orig;
+	}
+	/* TODO()sergey): What name we should use here? */
+	IDDepsNode *id_node = add_id_node(id_orig, id_orig->name, false);
+	return id_node->id_cow;
+}
+
 void deg_editors_id_update(Main *bmain, ID *id)
 {
 	if (deg_editor_update_id_cb != NULL) {
diff --git a/source/blender/depsgraph/intern/depsgraph.h b/source/blender/depsgraph/intern/depsgraph.h
index 50c55bc0cf9..9d94fe387a2 100644
--- a/source/blender/depsgraph/intern/depsgraph.h
+++ b/source/blender/depsgraph/intern/depsgraph.h
@@ -102,7 +102,8 @@ struct Depsgraph {
 	 * Convenience wrapper to find node given just pointer + property.
 	 *
 	 * \param ptr: pointer to the data that node will represent
-	 * \param prop: optional property affected - providing this effectively results in inner nodes being returned
+	 * \param prop: optional property affected - providing this effectively
+	 *              results in inner nodes being returned
 	 *
 	 * \return A node matching the required characteristics if it exists
 	 * or NULL if no such node exists in the graph
@@ -113,7 +114,7 @@ struct Depsgraph {
 	TimeSourceDepsNode *find_time_source() const;
 
 	IDDepsNode *find_id_node(const ID *id) const;
-	IDDepsNode *add_id_node(ID *id, const char *name = "");
+	IDDepsNode *add_id_node(ID *id, const char *name = "", bool do_tag = true);
 	void clear_id_nodes();
 
 	/* Add new relationship between two nodes. */
@@ -136,6 +137,11 @@ struct Depsgraph {
 	/* For given original ID get ID which is created by CoW system. */
 	ID *get_cow_id(const ID *id_orig) const;
 
+	/* Similar to above, but for the cases when there is no ID node we create
+	 * one.
+	 */
+	ID *ensure_cow_id(ID *id_orig);
+
 	/* Core Graph Functionality ........... */
 
 	/* <ID : IDDepsNode> mapping from ID blocks to nodes representing these blocks
diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
index ed9ea9e4d47..23d61e5c2c8 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
+++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
@@ -321,7 +321,7 @@ static bool check_datablocks_copy_on_writable(const ID *id_orig)
 
 struct RemapCallbackUserData {
 	/* Dependency graph for which remapping is happening. */
-	const Depsgraph *depsgraph;
+	Depsgraph *depsgraph;
 	/* Temporarily allocated memory for copying purposes. This ID will
 	 * be discarded after expanding is done, so need to make sure temp_id
 	 * is replaced with proper real_id.
@@ -333,6 +333,12 @@ struct RemapCallbackUserData {
 	 */
 	const ID *temp_id;
 	ID *real_id;
+	/* Create placeholder for ID nodes for cases when we need to remap original
+	 * ID to it[s CoW version but we don't have required ID node yet.
+	 *
+	 * This happens when expansion happens a ta construction time.
+	 */
+	bool create_placeholders;
 };
 
 int foreach_libblock_remap_callback(void *user_data_v,
@@ -341,16 +347,22 @@ int foreach_libblock_remap_callback(void *user_data_v,
                                     int /*cb_flag*/)
 {
 	RemapCallbackUserData *user_data = (RemapCallbackUserData *)user_data_v;
-	const Depsgraph *depsgraph = user_data->depsgraph;
+	Depsgraph *depsgraph = user_data->depsgraph;
 	if (*id_p != NULL) {
-		const ID *id_orig = *id_p;
+		ID *id_orig = *id_p;
 		if (id_orig == user_data->temp_id) {
 			DEG_COW_PRINT("    Remapping datablock for %s: id_temp=%p id_cow=%p\n",
 			              id_orig->name, id_orig, user_data->real_id);
 			*id_p = user_data->real_id;
 		}
 		else if (check_datablocks_copy_on_writable(id_orig)) {
-			ID *id_cow = depsgraph->get_cow_id(id_orig);
+			ID *id_cow;
+			if (user_data->create_placeholders) {
+				id_cow = depsgraph->ensure_cow_id(id_orig);
+			}
+			else {
+				id_cow = depsgraph->get_cow_id(id_orig);
+			}
 			BLI_assert(id_cow != NULL);
 			DEG_COW_PRINT("    Remapping datablock for %s: id_orig=%p id_cow=%p\n",
 			              id_orig->name, id_orig, id_cow);
@@ -498,9 +510,12 @@ int foreach_libblock_validate_callback(void *user_data,
  *
  * NOTE: Expects that CoW datablock is empty.
  */
-ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph,
-                                       const IDDepsNode *id_node)
+ID *deg_expand_copy_on_write_datablock(Depsgraph *depsgraph,
+                                       const IDDepsNode *id_node,
+                                       bool create_placeholders)
 {
+	BLI_assert(!create_placeholders ||
+	           check_datablock_expanded_at_construction(id_node->id_orig));
 	const ID *id_orig = id_node->id_orig;
 	ID *id_cow = id_node->id_cow;
 	DEG_COW_PRINT("Expanding datablock for %s: id_orig=%p id_cow=%p\n",
@@ -575,6 +590,7 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph,
 	user_data.depsgraph = depsgraph;
 	user_data.temp_id = newid;
 	user_data.real_id = id_cow;
+	user_data.create_placeholders = create_placeholders;
 	BKE_library_foreach_ID_link(NULL,
 	                            id_cow,
 	                            foreach_libblock_remap_callback,
@@ -593,14 +609,18 @@ ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph,
 }
 
 /* NOTE: Depsgraph is supposed to have ID node already. */
-ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph, ID *id_orig)
+ID *deg_expand_copy_on_write_datablock(Depsgraph *depsgraph,
+                                       ID *id_orig,
+                                       bool create_placeholders)
 {
 	DEG::IDDepsNode *id_node = depsgraph->find_id_node(id_orig);
 	BLI_assert(id_node != NULL);
-	return deg_expand_copy_on_write_datablock(depsgraph, id_node);
+	return deg_expand_copy_on_write_datablock(depsgraph,
+	                                          id_node,
+	                                          create_placeholders);
 }
 
-ID *deg_update_copy_on_write_datablock(const Depsgraph *depsgraph,
+ID *deg_update_copy_on_write_datablock(/*const*/ Depsgraph *depsgraph,
                                        const IDDepsNode *id_node)
 {
 	const ID *id_orig = id_node->id_orig;
@@ -619,12 +639,13 @@ ID *deg_update_copy_on_write_datablock(const Depsgraph *depsgraph,
 	 * - Perform full datablock copy.
 	 */
 	deg_free_copy_on_write_datablock(id_cow);
-	deg_expand_copy_on_write_datablock(depsgraph, id_node);
+	deg_expand_copy_on_write_datablock(depsgraph, id_node, false);
 	return id_cow;
 }
 
 /* NOTE: Depsgraph is supposed to have ID node already. */
-ID *deg_update_copy_on_write_datablock(const Depsgraph *depsgraph, ID *id_orig)
+ID *deg_update_copy_on_write_datablock(/*const*/ Depsgraph *depsgraph,
+                                       ID *id_orig)
 {
 	DEG::IDDepsNode *id_node = depsgraph->find_id_node(id_orig);
 	BLI_assert(id_node != NULL);
@@ -686,8 +707,8 @@ void deg_free_copy_on_write_datablock(ID *id_cow)
 	id_cow->name[0] = '\0';
 }
 
-void deg_evaluate_copy_on_write(EvaluationContext * /*eval_ctx*/,
-                                const Depsgraph *depsgraph,
+void deg_evaluate_copy_on_write(const EvaluationContext * /*eval_ctx*/,
+                                /*const*/ Depsgraph *depsgraph,
                                 const IDDepsNode *id_node)
 {
 	DEBUG_PRINT("%s on %s\n", __func__, id_node->id_orig->name);
diff --git a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h
index 0e1b6642002..78f6f7d926e 100644
--- a/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h
+++ b/source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h
@@ -53,17 +53,19 @@ struct IDDepsNode;
 /* Get fully expanded (ready for use) copy-on-write datablock for the given
  * original datablock.
  */
-ID *deg_expand_copy_on_write_datablock(const Depsgraph *depsgraph,
-                                       const IDDepsNode *id_node);
-ID *deg_expand_copy_on_write_datablock(const struct Depsgraph *depsgraph,
-                                       struct ID *id_orig);
+ID *deg_expand_copy_on_write_datablock(struct Depsgraph *depsgraph,
+                                       const IDDepsNode *id_node,
+                                       bool create_placeholders);
+ID *deg_expand_copy_on_write_datablock(struct Depsgraph *depsgraph,
+                                       struct ID *id_orig,
+                                       bool create_placeholders);
 
 /* Makes sure given CoW datablock is brought back to state of the original
  * datablock.
  */
-ID *deg_update_copy_on_write_datablock(const Depsgraph *depsgraph,
+ID *deg_update_copy_on_write_datablock(/*const*/ struct Depsgraph *depsgraph,
                                        const IDDepsNode *id_node);
-ID *deg_update_copy_on_write_datablock(const struct Depsgraph *depsgraph,
+ID *deg_update_copy_on_write_datablock(/*const*/ struct Depsgraph *depsgraph,
                                        struct ID *id_orig);
 
 /* Helper function which frees memory used by copy-on-written databnlock. */
@@ -72,8 +74,8 @@ void deg_free_copy_on_write_datablock(struct ID *id_cow);
 /* Callback function for depsgraph operation node which ensures copy-on-write
  * datablock is ready for use by further evaluation routines.
  */
-void deg_evaluate_copy_on_write(struct EvaluationContext *eval_ctx,
-                                const struct Depsgraph *depsgraph,
+void deg_evaluate_copy_on_write(const struct EvaluationContext *eval_ctx,
+                                /*const*/ struct Depsgraph *depsgraph,
                                 const struct IDDepsNode *id_node);
 
 /* Check that given ID is propely expanded and does not have any shallow