[Bf-blender-cvs] [5d9a1b440b1] blender2.8: Depsgraph: Fix wrong ID remapping when armature object is constructed prior to it's targets

Sergey Sharybin noreply at git.blender.org
Wed Jul 19 17:34:06 CEST 2017


Commit: 5d9a1b440b13bcd6f4255e7205421c0495e7ea12
Author: Sergey Sharybin
Date:   Wed Jul 19 11:57:20 2017 +0200
Branches: blender2.8
https://developer.blender.org/rB5d9a1b440b13bcd6f4255e7205421c0495e7ea12

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.

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

M	source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
M	source/blender/depsgraph/intern/builder/deg_builder_nodes_layer.cc
M	source/blender/depsgraph/intern/builder/deg_builder_nodes_rig.cc
M	source/blender/depsgraph/intern/depsgraph.cc
M	source/blender/depsgraph/intern/depsgraph.h
M	source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
M	source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h

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

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

@@ Diff output truncated at 10240 characters. @@




More information about the Bf-blender-cvs mailing list