Remove 'topochange' from EDBM.soc-2014-shapekey

In future, there may be several tools relying on topo hash, each one making
updates to it when it needs. Hence, such thing should absolutely not be held
by mesh data, but (in this case) by Key data.

Also made some reorganization and renaming a bit.

Note: this code compiles, but for now cannot really be tested, since scratch
is not enabled...

7 files changed, 119 insertions, 51 deletions

diff --git a/source/blender/blenkernel/BKE_editmesh.h b/source/blender/blenkernel/BKE_editmesh.h
index e7044a5b7ad..edcca3666b3 100644
--- a/source/blender/blenkernel/BKE_editmesh.h
+++ b/source/blender/blenkernel/BKE_editmesh.h
@@ -37,13 +37,7 @@ struct Mesh;
 struct Scene;
 struct DerivedMesh;
 struct MeshStatVis;
-
-/* struct for storing data to determine if the mesh's topology has changed */
-typedef struct {
-	int totvert, totedge, totloop, totface;
-	/* note the layout of totvert..totface is the same as in a BMesh so we can bulk copy/compare them */
-	int topohash;
-} EMTopoChangeData;
+struct MTopoHash;
 
 /* ok: the EDBM module is for editmode bmesh stuff.  in contrast, the 
  *     BMEdit module is for code shared with blenkernel that concerns
@@ -85,10 +79,6 @@ typedef struct BMEditMesh {
 	/* Object this editmesh came from (if it came from one) */
 	struct Object *ob;
 
-	/* used to know if the editmesh's topology has changed from a certain point in time,
-	 * use BKE_editmesh_topochange_calc/BKE_editmesh_topo_has_changed */
-	EMTopoChangeData topochange;
-
 	/*temp variables for x-mirror editing*/
 	int mirror_cdlayer; /* -1 is invalid */
 } BMEditMesh;
@@ -105,11 +95,9 @@ void        BKE_editmesh_update_linked_customdata(BMEditMesh *em);
 void        BKE_editmesh_color_free(BMEditMesh *em);
 void        BKE_editmesh_color_ensure(BMEditMesh *em, const char htype);
 
-/* calculates a topology-dependent hash to detect if topology has been modified later */
-void        BKE_editmesh_topochange_calc(BMEditMesh *em);
-
-/* checks if the topology has changed since last call to BKE_editmesh_syncdata_calc. Does not recalc! */
-bool        BKE_editmesh_topo_has_changed(BMEditMesh *em);
+/* Topology hash compute/check. */
+void        BKE_editmesh_topohash_compute(BMEditMesh *em, struct MTopoHash **topohash);
+bool        BKE_editmesh_topohash_identity(BMEditMesh *em, struct MTopoHash *topohash, const bool do_update);
 
 /* editderivedmesh.c */
 /* should really be defined in editmesh.c, but they use 'EditDerivedBMesh' */
diff --git a/source/blender/blenkernel/intern/editmesh.c b/source/blender/blenkernel/intern/editmesh.c
index 02cbc435fd0..b8ee402389a 100644
--- a/source/blender/blenkernel/intern/editmesh.c
+++ b/source/blender/blenkernel/intern/editmesh.c
@@ -34,6 +34,7 @@
 #include "DNA_listBase.h"
 #include "DNA_object_types.h"
 #include "DNA_mesh_types.h"
+#include "DNA_meshdata_types.h"
 
 #include "BLI_math.h"
 
@@ -250,24 +251,56 @@ void BKE_editmesh_color_ensure(BMEditMesh *em, const char htype)
 
 /* ==================== topology hashing ======================= */
 
-void BKE_editmesh_topochange_calc(BMEditMesh *em)
+/**
+ * Compute given \a em's topology signature, and stores it into \a topohash.
+ *
+ * If \a topohash is not yet allocated, will take care of this too, user must take care of freeing.
+ *
+ * \param em The BMEditMesh to check current topology.
+ * \param topohash The topology hash to fill in.
+ */
+void BKE_editmesh_topohash_compute(BMEditMesh *em, MTopoHash **topohash)
 {
-	em->topochange.topohash = BM_mesh_topology_hash(em->bm);
-	em->topochange.totvert = em->bm->totvert;
-	em->topochange.totedge = em->bm->totedge;
-	em->topochange.totloop = em->bm->totloop;
-	em->topochange.totface = em->bm->totface;
+	if (*topohash == NULL) {
+		*topohash = MEM_mallocN(sizeof(**topohash), __func__);
+	}
+
+	(*topohash)->totvert = em->bm->totvert;
+	(*topohash)->totedge = em->bm->totedge;
+	(*topohash)->totloop = em->bm->totloop;
+	(*topohash)->totpoly = em->bm->totface;
+
+	(*topohash)->hash = BM_mesh_topology_hash(em->bm);
 }
 
-bool BKE_editmesh_topo_has_changed(BMEditMesh *em)
+/**
+ * Checks if current \a em topology is identical to the one which generated \a topohash.
+ *
+ * \param em The BMEditMesh to check current topology.
+ * \param topohash The previously computed topology hash.
+ * \param do_update If true, update \a topohash to current \a em's topology 'signature'.
+ *
+ * \return True if topology matches given hash.
+ */
+bool BKE_editmesh_topohash_identity(BMEditMesh *em, MTopoHash *topohash, const bool do_update)
 {
-	if (em->bm->totvert == em->topochange.totvert && em->bm->totedge == em->topochange.totedge &&
-	    em->bm->totloop == em->topochange.totloop && em->bm->totface == em->topochange.totface)
+	if (do_update ||
+	    ((em->bm->totvert == topohash->totvert) && (em->bm->totedge == topohash->totedge) &&
+	     (em->bm->totloop == topohash->totloop) && (em->bm->totface == topohash->totpoly)))
 	{
-		int hash = BM_mesh_topology_hash(em->bm);
-		if (hash == em->topochange.topohash)
+		unsigned int hash = BM_mesh_topology_hash(em->bm);
+
+		if (hash != topohash->hash) {
+			if (do_update) {
+				topohash->totvert = em->bm->totvert;
+				topohash->totedge = em->bm->totedge;
+				topohash->totloop = em->bm->totloop;
+				topohash->totpoly = em->bm->totface;
+				topohash->hash = hash;
+			}
 			return false;
-		else return true;
+		}
+		return true;
 	}
-	return true;
+	return false;
 }
diff --git a/source/blender/blenkernel/intern/key.c b/source/blender/blenkernel/intern/key.c
index 2056adf38d2..2cb3af89e7f 100644
--- a/source/blender/blenkernel/intern/key.c
+++ b/source/blender/blenkernel/intern/key.c
@@ -93,8 +93,8 @@ void BKE_key_free(Key *key)
 		MEM_freeN(kb);
 	}
 
-	if (key->scratch.data)
-		MEM_freeN(key->scratch.data);
+	MEM_SAFE_FREE(key->scratch.data);
+	MEM_SAFE_FREE(key->topohash);
 }
 
 void BKE_key_free_nolib(Key *key)
@@ -107,8 +107,8 @@ void BKE_key_free_nolib(Key *key)
 		MEM_freeN(kb);
 	}
 
-	if (key->scratch.data)
-		MEM_freeN(key->scratch.data);
+	MEM_SAFE_FREE(key->scratch.data);
+	MEM_SAFE_FREE(key->topohash);
 }
 
 Key *BKE_key_add(ID *id)    /* common function */
@@ -122,7 +122,9 @@ Key *BKE_key_add(ID *id)    /* common function */
 	key->from = id;
 
 	key->uidgen = 1;
-	
+
+	key->topohash = NULL;
+
 	/* XXX the code here uses some defines which will soon be deprecated... */
 	switch (GS(id->name)) {
 		case ID_ME:
@@ -181,9 +183,13 @@ Key *BKE_key_copy(Key *key)
 		kbn = kbn->next;
 		kb = kb->next;
 	}
-	
-	if (key->scratch.data)
+
+	if (key->scratch.data) {
 		keyn->scratch.data = MEM_dupallocN(key->scratch.data);
+	}
+	if (key->topohash) {
+		keyn->topohash = MEM_dupallocN(key->topohash);
+	}
 
 	return keyn;
 }
@@ -197,14 +203,15 @@ void BKE_key_overwrite_data(Key *from, Key *to)
 			MEM_freeN(kbt->data);
 	}
 
-	if (to->scratch.data) {
-		MEM_freeN(to->scratch.data);
-		to->scratch.data = NULL;
-	}
+	MEM_SAFE_FREE(to->scratch.data);
+	MEM_SAFE_FREE(to->topohash);
 
 	if (from->scratch.data) {
 		to->scratch.data = MEM_dupallocN(from->scratch.data);
 	}
+	if (from->topohash) {
+		to->topohash = MEM_dupallocN(from->topohash);
+	}
 
 	BLI_freelistN(&to->block);
 	BLI_duplicatelist(&to->block, &from->block);
@@ -257,9 +264,13 @@ Key *BKE_key_copy_nolib(Key *key)
 		kb = kb->next;
 	}
 
-	if (key->scratch.data)
+	if (key->scratch.data) {
 		keyn->scratch.data = MEM_dupallocN(key->scratch.data);
-	
+	}
+	if (key->topohash) {
+		keyn->topohash = MEM_dupallocN(key->topohash);
+	}
+
 	return keyn;
 }
 
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index 01af82f124b..2be58e353c6 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -3166,7 +3166,9 @@ static void direct_link_key(FileData *fd, Key *key)
 	
 	key->scratch.origin = newdataadr(fd, key->scratch.origin);
 	key->scratch.data = newdataadr(fd, key->scratch.data);
-	
+
+	key->topohash = NULL;
+
 	for (kb = key->block.first; kb; kb = kb->next) {
 		kb->data = newdataadr(fd, kb->data);
 		
diff --git a/source/blender/editors/mesh/editmesh_utils.c b/source/blender/editors/mesh/editmesh_utils.c
index 719fd7350ed..e09dbda19be 100644
--- a/source/blender/editors/mesh/editmesh_utils.c
+++ b/source/blender/editors/mesh/editmesh_utils.c
@@ -33,6 +33,7 @@
 
 #include "DNA_key_types.h"
 #include "DNA_mesh_types.h"
+#include "DNA_meshdata_types.h"
 #include "DNA_object_types.h"
 #include "DNA_key_types.h"
 
@@ -381,9 +382,13 @@ void EDBM_mesh_make(ToolSettings *ts, Object *ob)
 
 	/* we need to flush selection because the mode may have changed from when last in editmode */
 	EDBM_selectmode_flush(me->edit_btmesh);
-	
-	if (BKE_key_from_object(ob) && BKE_keyblock_from_object(ob))
-		BKE_editmesh_topochange_calc(me->edit_btmesh);
+
+	{
+		Key *k = BKE_key_from_object(ob);
+		if (k && BKE_keyblock_from_object(ob)) {
+			BKE_editmesh_topohash_compute(me->edit_btmesh, &k->topohash);
+		}
+	}
 }
 
 void EDBM_mesh_load(Object *ob)
@@ -603,7 +608,10 @@ static void recalc_keyblocks_from_scratch(Object *ob)
 void EDBM_commit_scratch_to_active(Object *ob, Scene *s)
 {
 	BMEditMesh *em = BKE_editmesh_from_object(ob);
-	bool topo_changed = BKE_editmesh_topo_has_changed(em);
+	Key *k = BKE_key_from_object(ob);
+
+	/* XXX Check it is OK to update topohash here (i.e. subsequent calls to this are OK to assume same topo again). */
+	const bool topo_changed = !BKE_editmesh_topohash_identity(em, k->topohash, true);
 
 	if (topo_changed) {
 		EDBM_mesh_load(em->ob);
@@ -619,7 +627,7 @@ void EDBM_commit_scratch_to_active(Object *ob, Scene *s)
 		/* faster keyblock recalc */
 		recalc_keyblocks_from_scratch(ob);
 		/* update shapes customdata on bmesh from recalced keyblocks */
-		update_bmesh_shapes(ob);	
+		update_bmesh_shapes(ob);
 	}
 }
 
@@ -673,8 +681,9 @@ bool EDBM_mesh_from_editmesh(Object *obedit, bool do_free)
 		return false;
 	}
 
-	if (me->key && BKE_keyblock_from_object(obedit)) {
-		if (!BKE_editmesh_topo_has_changed(em)) {
+	if (k && BKE_keyblock_from_object(obedit)) {
+		/* XXX Check it is OK to update topohash here (i.e. subsequent calls to this are OK to assume same topo again). */
+		if (BKE_editmesh_topohash_identity(em, k->topohash, true)) {
 			BKE_key_editdata_to_scratch(obedit, true);
 			recalc_keyblocks_from_scratch(obedit);
 			update_bmesh_shapes(obedit);
diff --git a/source/blender/makesdna/DNA_key_types.h b/source/blender/makesdna/DNA_key_types.h
index ae8c81db1c7..7f6b27eca79 100644
--- a/source/blender/makesdna/DNA_key_types.h
+++ b/source/blender/makesdna/DNA_key_types.h
@@ -37,6 +37,7 @@
 
 #include "DNA_defs.h"
 #include "DNA_listBase.h"
+#include "DNA_meshdata_types.h"
 #include "DNA_ID.h"
 
 struct AnimData;
@@ -95,9 +96,12 @@ typedef struct Key {
 	 * perhaps later on it could be defined as some other KeyBlock - campbell */
 	KeyBlock *refkey;
 	
-	/* Runtime KeyBlock where the 'real' data editing happens. Gets committed to its origin. */
+	/* 'Runtime' KeyBlock where the 'real' data editing happens. Gets committed to its origin (saved in .blend). */
 	ScratchKeyBlock scratch;
 
+	/* Strictly-runtime topology hash, to detect changes in topology of skeyed mesh. */
+	MTopoHash *topohash;
+
 	/* this is not a regular string, although it is \0 terminated
 	 * this is an array of (element_array_size, element_type) pairs
 	 * (each one char) used for calculating shape key-blocks */
diff --git a/source/blender/makesdna/DNA_meshdata_types.h b/source/blender/makesdna/DNA_meshdata_types.h
index 3304980f964..749c854ffb3 100644
--- a/source/blender/makesdna/DNA_meshdata_types.h
+++ b/source/blender/makesdna/DNA_meshdata_types.h
@@ -38,6 +38,27 @@
 struct Bone;
 struct Image;
 
+/* Generic mesh topology hash.
+ * Note we store also numbers of verts/edges/loops/polys, since checking those is much cheaper than recomputing
+ * the real hash, and should be enough to detect 99% of topology changes!
+ * Strictly runtime, never to be saved, so tag it as DNA-ignored.
+ */
+/* Note: only half-convinced we really need numbers of verts/edges/loops/polys here. Thing is, they are only useful
+ *       to detect topo *changes*, if topology remains the same we have to recompute the hash every time anyway,
+ *       and unchanged topology is the most common expected case... On the other hand, storing numbers here
+ *       does not add much overhead, and maybe we'll be happy to have this 'is changed' quick check in some cases?
+ */
+#
+#
+typedef struct MTopoHash {
+	int totvert;
+	int totedge;
+	int totloop;
+	int totpoly;
+
+	unsigned int hash;
+} MTopoHash;
+
 /*tessellation face, see MLoop/MPoly for the real face data*/
 typedef struct MFace {
 	unsigned int v1, v2, v3, v4;