Fix crash when making objects to share same mesh

Make it more reliable and predictable way of getting pointer to
an original mesh which came from copy-on-write engine.

Related change: made it (hopefully) more clear name for flags.

9 files changed, 51 insertions, 34 deletions

diff --git a/source/blender/blenkernel/intern/DerivedMesh.c b/source/blender/blenkernel/intern/DerivedMesh.c
index c554d5e7b6c..1698946b506 100644
--- a/source/blender/blenkernel/intern/DerivedMesh.c
+++ b/source/blender/blenkernel/intern/DerivedMesh.c
@@ -2790,7 +2790,7 @@ static void editbmesh_calc_modifiers(
 			}
 			else {
 				struct Mesh *mesh = ob->data;
-				if (mesh->id.tag & LIB_TAG_COPY_ON_WRITE) {
+				if (mesh->id.tag & LIB_TAG_COPIED_ON_WRITE) {
 					BKE_mesh_runtime_ensure_edit_data(mesh);
 					mesh->runtime.edit_data->vertexCos = MEM_dupallocN(deformedVerts);
 				}
@@ -2832,7 +2832,7 @@ static void editbmesh_calc_modifiers(
 	else {
 		/* this is just a copy of the editmesh, no need to calc normals */
 		struct Mesh *mesh = ob->data;
-		if (mesh->id.tag & LIB_TAG_COPY_ON_WRITE) {
+		if (mesh->id.tag & LIB_TAG_COPIED_ON_WRITE) {
 			BKE_mesh_runtime_ensure_edit_data(mesh);
 			if (mesh->runtime.edit_data->vertexCos != NULL)
 				MEM_freeN((void *)mesh->runtime.edit_data->vertexCos);
@@ -2943,18 +2943,14 @@ static void mesh_finalize_eval(Object *object)
 	if (mesh_eval->mat != NULL) {
 		MEM_freeN(mesh_eval->mat);
 	}
+	/* Set flag which makes it easier to see what's going on in a debugger. */
+	mesh_eval->id.tag |= LIB_TAG_COPIED_ON_WRITE_EVAL_RESULT;
 	mesh_eval->mat = MEM_dupallocN(mesh->mat);
 	mesh_eval->totcol = mesh->totcol;
 	/* Make evaluated mesh to share same edit mesh pointer as original
 	 * and copied meshes.
 	 */
 	mesh_eval->edit_btmesh = mesh->edit_btmesh;
-	/* Special flags to help debugging and also to allow copy-on-write core
-	 * to understand that on re-evaluation this mesh is to be preserved and
-	 * to be remapped back to copied original mesh when used as object data.
-	 */
-	mesh_eval->id.tag |= LIB_TAG_COPY_ON_WRITE_EVAL;
-	mesh_eval->id.orig_id = &mesh->id;
 	/* Copy autosmooth settings from original mesh.
 	 * This is not done by BKE_mesh_new_nomain_from_template(), so need to take
 	 * extra care here.
@@ -2970,7 +2966,7 @@ static void mesh_finalize_eval(Object *object)
 
 	/* Object is sometimes not evaluated!
 	 * TODO(sergey): BAD TEMPORARY HACK FOR UNTIL WE ARE SMARTER */
-	if (object->id.tag & LIB_TAG_COPY_ON_WRITE) {
+	if (object->id.tag & LIB_TAG_COPIED_ON_WRITE) {
 		object->data = mesh_eval;
 	}
 	else {
diff --git a/source/blender/blenkernel/intern/lattice.c b/source/blender/blenkernel/intern/lattice.c
index 0fa20f00823..b5b62de57ec 100644
--- a/source/blender/blenkernel/intern/lattice.c
+++ b/source/blender/blenkernel/intern/lattice.c
@@ -1057,7 +1057,7 @@ void BKE_lattice_modifiers_calc(struct Depsgraph *depsgraph, Scene *scene, Objec
 		modifier_deformVerts_DM_deprecated(md, &mectx, NULL, vertexCos, numVerts);
 	}
 
-	if (ob->id.tag & LIB_TAG_COPY_ON_WRITE) {
+	if (ob->id.tag & LIB_TAG_COPIED_ON_WRITE) {
 		if (vertexCos) {
 			BKE_lattice_vertexcos_apply(ob, vertexCos);
 			MEM_freeN(vertexCos);
diff --git a/source/blender/blenkernel/intern/object.c b/source/blender/blenkernel/intern/object.c
index 2eb35908d63..9c42cc686ea 100644
--- a/source/blender/blenkernel/intern/object.c
+++ b/source/blender/blenkernel/intern/object.c
@@ -344,7 +344,9 @@ void BKE_object_free_derived_caches(Object *ob)
 	if (ob->runtime.mesh_eval != NULL) {
 		Mesh *mesh_eval = ob->runtime.mesh_eval;
 		/* Restore initial pointer. */
-		ob->data = mesh_eval->id.orig_id;
+		if (ob->data == mesh_eval) {
+			ob->data = ob->runtime.mesh_orig;
+		}
 		/* Evaluated mesh points to edit mesh, but does not own it. */
 		mesh_eval->edit_btmesh = NULL;
 		BKE_mesh_free(mesh_eval);
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index 5867e278c78..f01baed06c3 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -311,7 +311,7 @@ ID *DepsgraphNodeBuilder::get_cow_id(const ID *id_orig) const
 
 ID *DepsgraphNodeBuilder::ensure_cow_id(ID *id_orig)
 {
-	if (id_orig->tag & LIB_TAG_COPY_ON_WRITE) {
+	if (id_orig->tag & LIB_TAG_COPIED_ON_WRITE) {
 		/* ID is already remapped to copy-on-write. */
 		return id_orig;
 	}
diff --git a/source/blender/depsgraph/intern/depsgraph.cc b/source/blender/depsgraph/intern/depsgraph.cc
index b9681805cfb..26a23cff372 100644
--- a/source/blender/depsgraph/intern/depsgraph.cc
+++ b/source/blender/depsgraph/intern/depsgraph.cc
@@ -310,7 +310,7 @@ IDDepsNode *Depsgraph::find_id_node(const ID *id) const
 
 IDDepsNode *Depsgraph::add_id_node(ID *id, ID *id_cow_hint)
 {
-	BLI_assert((id->tag & LIB_TAG_COPY_ON_WRITE) == 0);
+	BLI_assert((id->tag & LIB_TAG_COPIED_ON_WRITE) == 0);
 	IDDepsNode *id_node = find_id_node(id);
 	if (!id_node) {
 		DepsNodeFactory *factory = deg_type_get_factory(DEG_NODE_TYPE_ID_REF);
@@ -500,7 +500,7 @@ ID *Depsgraph::get_cow_id(const ID *id_orig) const
 		 * We try to enforce that in debug builds, for for release we play a bit
 		 * safer game here.
 		 */
-		if ((id_orig->tag & LIB_TAG_COPY_ON_WRITE) == 0) {
+		if ((id_orig->tag & LIB_TAG_COPIED_ON_WRITE) == 0) {
 			/* TODO(sergey): This is nice sanity check to have, but it fails
 			 * in following situations:
 			 *
diff --git a/source/blender/depsgraph/intern/depsgraph_query.cc b/source/blender/depsgraph/intern/depsgraph_query.cc
index 06fbe980620..ca9f32d4d8c 100644
--- a/source/blender/depsgraph/intern/depsgraph_query.cc
+++ b/source/blender/depsgraph/intern/depsgraph_query.cc
@@ -240,6 +240,6 @@ ID *DEG_get_original_id(ID *id)
 	if (id->orig_id == NULL) {
 		return id;
 	}
-	BLI_assert((id->tag & LIB_TAG_COPY_ON_WRITE) != 0);
+	BLI_assert((id->tag & LIB_TAG_COPIED_ON_WRITE) != 0);
 	return (ID *)id->orig_id;
 }
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 2789f189f03..858e366b280 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
@@ -542,6 +542,7 @@ void update_special_pointers(const Depsgraph *depsgraph,
 			BLI_assert(object_cow->derivedDeform == NULL);
 			object_cow->mode = object_orig->mode;
 			object_cow->sculpt = object_orig->sculpt;
+			object_cow->runtime.mesh_orig = (Mesh *)object_cow->data;
 			if (object_cow->type == OB_ARMATURE) {
 				BKE_pose_remap_bone_pointers((bArmature *)object_cow->data,
 				                             object_cow->pose);
@@ -724,12 +725,12 @@ static void deg_backup_object_runtime(
 	Mesh *mesh_eval = object->runtime.mesh_eval;
 	object_runtime_backup->runtime = object->runtime;
 	BKE_object_runtime_reset(object);
-	/* Currently object update will override actual object->data
-	 * to an evaluated version. Need to make sure we don't have
-	 * data set to evaluated one before free anything.
+	/* Object update will override actual object->data to an evaluated version.
+	 * Need to make sure we don't have data set to evaluated one before free
+	 * anything.
 	 */
 	if (mesh_eval != NULL && object->data == mesh_eval) {
-		object->data = mesh_eval->id.orig_id;
+		object->data = object->runtime.mesh_orig;
 	}
 	/* Store curve cache and make sure we don't free it. */
 	object_runtime_backup->curve_cache = object->curve_cache;
@@ -742,20 +743,33 @@ static void deg_restore_object_runtime(
         Object *object,
         const ObjectRuntimeBackup *object_runtime_backup)
 {
+	Mesh *mesh_orig = object->runtime.mesh_orig;
 	object->runtime = object_runtime_backup->runtime;
+	object->runtime.mesh_orig = mesh_orig;
 	if (object->runtime.mesh_eval != NULL) {
-		Mesh *mesh_eval = object->runtime.mesh_eval;
-		/* Do same thing as object update: override actual object data
-		 * pointer with evaluated datablock.
-		 */
-		if (object->type == OB_MESH) {
-			object->data = mesh_eval;
-			/* Evaluated mesh simply copied edit_btmesh pointer from
-			 * original mesh during update, need to make sure no dead
-			 * pointers are left behind.
+		if (object->id.recalc & ID_RECALC_GEOMETRY) {
+			/* If geometry is tagged for update it means, that part of
+			 * evaluated mesh are not valid anymore. In this case we can not
+			 * have any "persistent" pointers to point to an invalid data.
+			 *
+			 * We restore object's data datablock to an original copy of
+			 * that datablock.
+			 */
+			object->data = mesh_orig;
+		}
+		else {
+			Mesh *mesh_eval = object->runtime.mesh_eval;
+			/* Do same thing as object update: override actual object data
+			 * pointer with evaluated datablock.
 			 */
-			Mesh *mesh = ((Mesh *)mesh_eval->id.orig_id);
-			mesh_eval->edit_btmesh = mesh->edit_btmesh;
+			if (object->type == OB_MESH) {
+				object->data = mesh_eval;
+				/* Evaluated mesh simply copied edit_btmesh pointer from
+				 * original mesh during update, need to make sure no dead
+				 * pointers are left behind.
+				*/
+				mesh_eval->edit_btmesh = mesh_orig->edit_btmesh;
+			}
 		}
 	}
 	if (object_runtime_backup->curve_cache != NULL) {
@@ -995,8 +1009,8 @@ bool deg_validate_copy_on_write_datablock(ID *id_cow)
 void deg_tag_copy_on_write_id(ID *id_cow, const ID *id_orig)
 {
 	BLI_assert(id_cow != id_orig);
-	BLI_assert((id_orig->tag & LIB_TAG_COPY_ON_WRITE) == 0);
-	id_cow->tag |= LIB_TAG_COPY_ON_WRITE;
+	BLI_assert((id_orig->tag & LIB_TAG_COPIED_ON_WRITE) == 0);
+	id_cow->tag |= LIB_TAG_COPIED_ON_WRITE;
 	id_cow->orig_id = (ID *)id_orig;
 }
 
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 05e39c6636a..6ce0c94d884 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -467,8 +467,8 @@ enum {
 	LIB_TAG_PRE_EXISTING    = 1 << 11,
 
 	/* The datablock is a copy-on-write/localized version. */
-	LIB_TAG_COPY_ON_WRITE   = 1 << 12,
-	LIB_TAG_COPY_ON_WRITE_EVAL = 1 << 13,
+	LIB_TAG_COPIED_ON_WRITE               = 1 << 12,
+	LIB_TAG_COPIED_ON_WRITE_EVAL_RESULT   = 1 << 13,
 	LIB_TAG_LOCALIZED = 1 << 14,
 
 	/* RESET_NEVER tag datablock for freeing etc. behavior (usually set when copying real one into temp/runtime one). */
diff --git a/source/blender/makesdna/DNA_object_types.h b/source/blender/makesdna/DNA_object_types.h
index daff6ab0baa..c9db5fbbdfb 100644
--- a/source/blender/makesdna/DNA_object_types.h
+++ b/source/blender/makesdna/DNA_object_types.h
@@ -145,6 +145,11 @@ typedef struct ObjectDisplay {
 
 /* Not saved in file! */
 typedef struct Object_Runtime {
+	/* Original mesh pointer, before object->data was changed to point
+	 * to mesh_eval.
+	 * Is assigned by dependency craph's copy-on-write evaluation.
+	 */
+	struct Mesh *mesh_orig;
 	/* Mesh structure created during object evaluation.
 	 * It has all modifiers applied.
 	 */