[Bf-blender-cvs] [2b8f50e1d41] blender2.8: Depsgraph: Fix crash happening in copy-on-write of images

Sergey Sharybin noreply at git.blender.org
Thu Jul 27 13:26:28 CEST 2017


Commit: 2b8f50e1d4130c9eba2a896e595d25212a808480
Author: Sergey Sharybin
Date:   Thu Jul 27 13:09:20 2017 +0200
Branches: blender2.8
https://developer.blender.org/rB2b8f50e1d4130c9eba2a896e595d25212a808480

Depsgraph: Fix crash happening in copy-on-write of images

Was a threading conflict or so in the cache limiter, and in fact
we don't even want images to be copied.

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

M	source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.cc
M	source/blender/depsgraph/intern/eval/deg_eval_copy_on_write.h
M	source/blender/depsgraph/intern/nodes/deg_node.cc

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

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 7d8db60f9ba..f74e3deb9fb 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
@@ -361,6 +361,10 @@ static bool check_datablock_expanded_at_construction(const ID *id_orig)
 static bool check_datablocks_copy_on_writable(const ID *id_orig)
 {
 	const short id_type = GS(id_orig->name);
+	/* We shouldn't bother if copied ID is same as original one. */
+	if (!deg_copy_on_write_is_needed(id_orig)) {
+		return false;
+	}
 	return !ELEM(id_type, ID_BR,
 	                      ID_LS,
 	                      ID_AC,
@@ -645,6 +649,12 @@ ID *deg_expand_copy_on_write_datablock(Depsgraph *depsgraph,
 	           check_datablock_expanded_at_construction(id_node->id_orig));
 	const ID *id_orig = id_node->id_orig;
 	ID *id_cow = id_node->id_cow;
+	/* No need to expand such datablocks, their copied ID is same as original
+	 * one already.
+	 */
+	if (!deg_copy_on_write_is_needed(id_orig)) {
+		return id_cow;
+	}
 	DEG_COW_PRINT("Expanding datablock for %s: id_orig=%p id_cow=%p\n",
 	              id_orig->name, id_orig, id_cow);
 	/* Sanity checks. */
@@ -755,6 +765,10 @@ ID *deg_update_copy_on_write_datablock(/*const*/ Depsgraph *depsgraph,
 {
 	const ID *id_orig = id_node->id_orig;
 	ID *id_cow = id_node->id_cow;
+	/* Similar to expansion, no need to do anything here. */
+	if (!deg_copy_on_write_is_needed(id_orig)) {
+		return id_cow;
+	}
 	/* Special case for datablocks which are expanded at the dependency graph
 	 * construction time. This datablocks must never change pointers of their
 	 * nested data since it is used for function bindings.
@@ -897,9 +911,15 @@ void deg_tag_copy_on_write_id(ID *id_cow, const ID *id_orig)
 	id_cow->newid = (ID *)id_orig;
 }
 
-bool deg_copy_on_write_is_expanded(const struct ID *id_cow)
+bool deg_copy_on_write_is_expanded(const ID *id_cow)
 {
 	return check_datablock_expanded(id_cow);
 }
 
+bool deg_copy_on_write_is_needed(const ID *id_orig)
+{
+	const short id_type = GS(id_orig->name);
+	return !ELEM(id_type, ID_IM);
+}
+
 }  // namespace DEG
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 4ae1de3fdbc..52fe58e3391 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
@@ -92,4 +92,13 @@ void deg_tag_copy_on_write_id(struct ID *id_cow, const struct ID *id_orig);
  */
 bool deg_copy_on_write_is_expanded(const struct ID *id_cow);
 
+/* Check whether copy-on-write datablock is needed for given ID.
+ *
+ * There are some exceptions on datablocks which are covered by dependency graph
+ * but which we don't want to start duplicating.
+ *
+ * This includes images.
+ */
+bool deg_copy_on_write_is_needed(const ID *id_orig);
+
 }  // namespace DEG
diff --git a/source/blender/depsgraph/intern/nodes/deg_node.cc b/source/blender/depsgraph/intern/nodes/deg_node.cc
index 29f0a3a05ba..c34189e0ba7 100644
--- a/source/blender/depsgraph/intern/nodes/deg_node.cc
+++ b/source/blender/depsgraph/intern/nodes/deg_node.cc
@@ -165,10 +165,10 @@ static void id_deps_node_hash_value_free(void *value_v)
 /* Initialize 'id' node - from pointer data given. */
 void IDDepsNode::init(const ID *id, const char *UNUSED(subdata))
 {
-	/* Store ID-pointer. */
 	BLI_assert(id != NULL);
-	this->id_orig = (ID *)id;
-	this->eval_flags = 0;
+	/* Store ID-pointer. */
+	id_orig = (ID *)id;
+	eval_flags = 0;
 
 	components = BLI_ghash_new(id_deps_node_hash_key,
 	                           id_deps_node_hash_key_cmp,
@@ -179,10 +179,15 @@ void IDDepsNode::init(const ID *id, const char *UNUSED(subdata))
 	 * bindings. Rest of data we'll be copying to the new datablock when
 	 * it is actually needed.
 	 */
-	id_cow = (ID *)BKE_libblock_alloc_notest(GS(id->name));
-	DEG_COW_PRINT("Create shallow copy for %s: id_orig=%p id_cow=%p\n",
-	              id_orig->name, id_orig, id_cow);
-	deg_tag_copy_on_write_id(id_cow, id_orig);
+	if (deg_copy_on_write_is_needed(id_orig)) {
+		id_cow = (ID *)BKE_libblock_alloc_notest(GS(id->name));
+		DEG_COW_PRINT("Create shallow copy for %s: id_orig=%p id_cow=%p\n",
+		              id_orig->name, id_orig, id_cow);
+		deg_tag_copy_on_write_id(id_cow, id_orig);
+	}
+	else {
+		id_cow = id_orig;
+	}
 #else
 	id_cow = id_orig;
 #endif
@@ -206,10 +211,12 @@ void IDDepsNode::destroy()
 
 #ifdef WITH_COPY_ON_WRITE
 	/* Free memory used by this CoW ID. */
-	deg_free_copy_on_write_datablock(id_cow);
-	MEM_freeN(id_cow);
-	DEG_COW_PRINT("Destroy CoW for %s: id_orig=%p id_cow=%p\n",
-	              id_orig->name, id_orig, id_cow);
+	if (id_cow != id_orig) {
+		deg_free_copy_on_write_datablock(id_cow);
+		MEM_freeN(id_cow);
+		DEG_COW_PRINT("Destroy CoW for %s: id_orig=%p id_cow=%p\n",
+		              id_orig->name, id_orig, id_cow);
+	}
 #endif
 	/* Tag that the node is freed. */
 	id_orig = NULL;




More information about the Bf-blender-cvs mailing list