Fix T40813: Dissolve verts with adjacent regions, removes the dividing edge

3 files changed, 69 insertions, 107 deletions

diff --git a/source/blender/bmesh/intern/bmesh_error.h b/source/blender/bmesh/intern/bmesh_error.h
index 0f23f87c143..6cd37011814 100644
--- a/source/blender/bmesh/intern/bmesh_error.h
+++ b/source/blender/bmesh/intern/bmesh_error.h
@@ -59,17 +59,20 @@ void BMO_error_clear(BMesh *bm);
 /*------ error code defines -------*/
 
 /*error messages*/
-#define BMERR_SELF_INTERSECTING			1
-#define BMERR_DISSOLVEDISK_FAILED		2
-#define BMERR_CONNECTVERT_FAILED		3
-#define BMERR_WALKER_FAILED				4
-#define BMERR_DISSOLVEFACES_FAILED		5
-#define BMERR_DISSOLVEVERTS_FAILED		6
-#define BMERR_TESSELLATION				7
-#define BMERR_NONMANIFOLD				8
-#define BMERR_INVALID_SELECTION			9
-#define BMERR_MESH_ERROR				10
-#define BMERR_CONVEX_HULL_FAILED		11
+enum {
+	BMERR_SELF_INTERSECTING = 1,
+	BMERR_DISSOLVEDISK_FAILED,
+	BMERR_CONNECTVERT_FAILED,
+	BMERR_WALKER_FAILED,
+	BMERR_DISSOLVEFACES_FAILED,
+	BMERR_TESSELLATION,
+	BMERR_NONMANIFOLD,
+	BMERR_INVALID_SELECTION,
+	BMERR_MESH_ERROR,
+	BMERR_CONVEX_HULL_FAILED,
+
+	BMERR_TOTAL,
+};
 
 /* BMESH_ASSERT */
 #ifdef WITH_ASSERT_ABORT
diff --git a/source/blender/bmesh/intern/bmesh_operators.c b/source/blender/bmesh/intern/bmesh_operators.c
index dc419fab739..7f872613896 100644
--- a/source/blender/bmesh/intern/bmesh_operators.c
+++ b/source/blender/bmesh/intern/bmesh_operators.c
@@ -54,13 +54,13 @@ static const char *bmo_error_messages[] = {
 	N_("Could not connect vertices"),
 	N_("Could not traverse mesh"),
 	N_("Could not dissolve faces"),
-	N_("Could not dissolve vertices"),
 	N_("Tessellation error"),
 	N_("Cannot deal with non-manifold geometry"),
 	N_("Invalid selection"),
 	N_("Internal mesh error"),
 };
 
+BLI_STATIC_ASSERT(ARRAY_SIZE(bmo_error_messages) + 1 == BMERR_TOTAL, "message mismatch");
 
 /* operator slot type information - size of one element of the type given. */
 const int BMO_OPSLOT_TYPEINFO[BMO_OP_SLOT_TOTAL_TYPES] = {
diff --git a/source/blender/bmesh/operators/bmo_dissolve.c b/source/blender/bmesh/operators/bmo_dissolve.c
index 59a78033e04..36a936c6622 100644
--- a/source/blender/bmesh/operators/bmo_dissolve.c
+++ b/source/blender/bmesh/operators/bmo_dissolve.c
@@ -335,126 +335,85 @@ void bmo_dissolve_edges_exec(BMesh *bm, BMOperator *op)
 	}
 }
 
-static bool test_extra_verts(BMesh *bm, BMVert *v)
-{
-	BMIter fiter, liter, eiter, fiter_sub;
-	BMFace *f;
-	BMLoop *l;
-	BMEdge *e;
-
-	/* test faces around verts for verts that would be wrongly killed
-	 * by dissolve faces. */
-	BM_ITER_ELEM (f, &fiter, v, BM_FACES_OF_VERT) {
-		BM_ITER_ELEM (l, &liter, f, BM_LOOPS_OF_FACE) {
-			if (!BMO_elem_flag_test(bm, l->v, VERT_MARK)) {
-				/* if an edge around a vert is a boundary edge,
-				 * then dissolve faces won't destroy it.
-				 * also if it forms a boundary with one
-				 * of the face region */
-				bool found = false;
-				BM_ITER_ELEM (e, &eiter, l->v, BM_EDGES_OF_VERT) {
-					BMFace *f_iter;
-					if (BM_edge_is_boundary(e)) {
-						found = true;
-					}
-					else {
-						BM_ITER_ELEM (f_iter, &fiter_sub, e, BM_FACES_OF_EDGE) {
-							if (!BMO_elem_flag_test(bm, f_iter, FACE_MARK)) {
-								found = true;
-								break;
-							}
-						}
-					}
-					if (found == true) {
-						break;
-					}
-				}
-				if (found == false) {
-					return false;
-				}
-			}
-		}
-	}
-
-	return true;
-}
 void bmo_dissolve_verts_exec(BMesh *bm, BMOperator *op)
 {
-	BMIter iter, fiter;
+	BMOIter oiter;
+	BMIter iter;
 	BMVert *v, *v_next;
-	BMFace *f;
+	BMEdge *e, *e_next;
+	BMFace *act_face = bm->act_face;
 
 	const bool use_face_split = BMO_slot_bool_get(op->slots_in, "use_face_split");
 
+	BMO_ITER (v, &oiter, op->slots_in, "verts", BM_VERT) {
+		BMIter itersub;
+
+		BMO_elem_flag_enable(bm, v, VERT_MARK | VERT_ISGC);
+
+		BM_ITER_ELEM (e, &itersub, v, BM_EDGES_OF_VERT) {
+			BMO_elem_flag_enable(bm, e, EDGE_ISGC);
+		}
+	}
 
-	BMO_slot_buffer_flag_enable(bm, op->slots_in, "verts", BM_VERT, VERT_MARK);
-	
 	if (use_face_split) {
 		bm_face_split(bm, VERT_MARK);
 	}
 
-	BM_ITER_MESH_MUTABLE (v, v_next, &iter, bm, BM_VERTS_OF_MESH) {
-		if (BMO_elem_flag_test(bm, v, VERT_MARK)) {
-			/* check if it's a two-valence ver */
-			if (BM_vert_edge_count(v) == 2) {
+	BMO_ITER (v, &oiter, op->slots_in, "verts", BM_VERT) {
+		BMIter itersub;
+		BMLoop *l_first;
+		BM_ITER_ELEM (l_first, &itersub, v, BM_LOOPS_OF_VERT) {
+			BMLoop *l_iter;
+			l_iter = l_first;
+			do {
+				BMO_elem_flag_enable(bm, l_iter->v, VERT_ISGC);
+				BMO_elem_flag_enable(bm, l_iter->e, EDGE_ISGC);
+			} while ((l_iter = l_iter->next) != l_first);
+		}
+	}
 
-				/* collapse the ver */
-				/* previously the faces were joined, but collapsing between 2 edges
-				 * gives some advantage/difference in using vertex-dissolve over edge-dissolve */
-#if 0
-				BM_vert_collapse_faces(bm, v->e, v, 1.0f, true, true);
-#else
-				BM_vert_collapse_edge(bm, v->e, v, true);
-#endif
+	BMO_ITER (v, &oiter, op->slots_in, "verts", BM_VERT) {
+		BMIter itersub;
 
-				continue;
-			}
+		if (BM_vert_edge_count(v) != 2) {
+			BM_ITER_ELEM (e, &itersub, v, BM_EDGES_OF_VERT) {
+				BMFace *fa, *fb;
+				if (BM_edge_face_pair(e, &fa, &fb)) {
+					BMFace *f_new;
 
-			BM_ITER_ELEM (f, &fiter, v, BM_FACES_OF_VERT) {
-				BMO_elem_flag_enable(bm, f, FACE_MARK | FACE_ORIG);
-			}
-			
-			/* check if our additions to the input to face dissolve
-			 * will destroy nonmarked vertices. */
-			if (!test_extra_verts(bm, v)) {
-				BM_ITER_ELEM (f, &fiter, v, BM_FACES_OF_VERT) {
-					if (BMO_elem_flag_test(bm, f, FACE_ORIG)) {
-						BMO_elem_flag_disable(bm, f, FACE_MARK | FACE_ORIG);
+					/* join faces */
+					f_new = BM_faces_join_pair(bm, fa, fb, e, false);
+
+					/* maintain active face */
+					if (act_face && bm->act_face == NULL) {
+						bm->act_face = f_new;
 					}
 				}
 			}
-			else {
-				BM_ITER_ELEM (f, &fiter, v, BM_FACES_OF_VERT) {
-					BMO_elem_flag_disable(bm, f, FACE_ORIG);
-				}
-			}
 		}
 	}
 
-	BMO_op_callf(bm, op->flag, "dissolve_faces faces=%ff", FACE_MARK);
-	if (BMO_error_occurred(bm)) {
-		const char *msg;
-
-		BMO_error_get(bm, &msg, NULL);
-		BMO_error_clear(bm);
-		BMO_error_raise(bm, op, BMERR_DISSOLVEVERTS_FAILED, msg);
+	/* Cleanup geometry (#BM_faces_join_pair, but it removes geometry we're looping on)
+	 * so do this in a separate pass instead. */
+	BM_ITER_MESH_MUTABLE (e, e_next, &iter, bm, BM_EDGES_OF_MESH) {
+		if ((e->l == NULL) && BMO_elem_flag_test(bm, e, EDGE_ISGC)) {
+			BM_edge_kill(bm, e);
+		}
 	}
-	
-	/* clean up any remaining */
-	/* note: don't use BM_ITER_MESH_MUTABLE here, even though vertices are removed (T37559) */
-	BM_ITER_MESH (v, &iter, bm, BM_VERTS_OF_MESH) {
+	BM_ITER_MESH_MUTABLE (v, v_next, &iter, bm, BM_VERTS_OF_MESH) {
+		if ((v->e == NULL) && BMO_elem_flag_test(bm, v, VERT_ISGC)) {
+			BM_vert_kill(bm, v);
+		}
+	}
+	/* done with cleanup */
+
+	BM_ITER_MESH_MUTABLE (v, v_next, &iter, bm, BM_VERTS_OF_MESH) {
 		if (BMO_elem_flag_test(bm, v, VERT_MARK)) {
-			if (!BM_vert_dissolve(bm, v)) {
-				BMO_error_raise(bm, op, BMERR_DISSOLVEVERTS_FAILED, NULL);
-				return;
+			if (BM_vert_edge_count(v) == 2) {
+				BM_vert_collapse_edge(bm, v->e, v, true);
 			}
-#ifdef DEBUG
-			/* workaround debug assert */
-			iter.count = bm->totvert;
-#endif
 		}
 	}
-
 }
 
 /* Limited Dissolve */