Fix/enhance BKE_mesh_validate_arrays.

Aside from some minor cleanup, this commit:
* Fixes checking twice for multiple usage of same vert by a same poly.
* Fixes handling of ME_VERT_TMP_TAG vert flag by that check (there was no guaranty
  that flag was cleared for a poly's vertices before we start checking).

1 files changed, 71 insertions, 90 deletions

diff --git a/source/blender/blenkernel/intern/mesh_validate.c b/source/blender/blenkernel/intern/mesh_validate.c
index ab469ac3996..f01e6220abd 100644
--- a/source/blender/blenkernel/intern/mesh_validate.c
+++ b/source/blender/blenkernel/intern/mesh_validate.c
@@ -496,6 +496,7 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
 		SortPoly *sort_polys = MEM_callocN(sizeof(SortPoly) * totpoly, "mesh validate's sort_polys");
 		SortPoly *prev_sp, *sp = sort_polys;
 		int prev_end;
+
 		for (i = 0, mp = mpolys; i < totpoly; i++, mp++, sp++) {
 			sp->index = i;
 
@@ -519,6 +520,14 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
 				sp->numverts = mp->totloop;
 				sp->loopstart = mp->loopstart;
 
+				/* Ideally we would only have to do that once on all vertices before we start checking each poly, but
+				 * several polys can use same vert, so we have to ensure here all verts of current poly are cleared. */
+				for (j = 0, ml = &mloops[sp->loopstart]; j < mp->totloop; j++, ml++) {
+					if (ml->v < totvert) {
+						mverts[ml->v].flag &= ~ME_VERT_TMP_TAG;
+					}
+				}
+
 				/* Test all poly's loops' vert idx. */
 				for (j = 0, ml = &mloops[sp->loopstart]; j < mp->totloop; j++, ml++, v++) {
 					if (ml->v >= totvert) {
@@ -526,24 +535,16 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
 						PRINT_ERR("\tLoop %u has invalid vert reference (%u)\n", sp->loopstart + j, ml->v);
 						sp->invalid = true;
 					}
+					else if (mverts[ml->v].flag & ME_VERT_TMP_TAG) {
+						PRINT_ERR("\tPoly %u has duplicated vert reference at corner (%u)\n", i, j);
+						sp->invalid = true;
+					}
 					else {
 						mverts[ml->v].flag |= ME_VERT_TMP_TAG;
 					}
 					*v = ml->v;
 				}
 
-				/* is the same vertex used more than once */
-				if (!sp->invalid) {
-					v = sp->verts;
-					for (j = 0; j < mp->totloop; j++, v++) {
-						if ((mverts[*v].flag & ME_VERT_TMP_TAG) == 0) {
-							PRINT_ERR("\tPoly %u has duplicate vert reference at corner (%u)\n", i, j);
-							sp->invalid = true;
-						}
-						mverts[*v].flag &= ~ME_VERT_TMP_TAG;
-					}
-				}
-
 				if (sp->invalid)
 					continue;
 
@@ -594,31 +595,10 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
 					}
 				}
 
-				/* Now check that that poly does not use a same vertex more than once! */
 				if (!sp->invalid) {
-					const int *prev_v = v = sp->verts;
-					j = sp->numverts;
-
-					qsort(sp->verts, j, sizeof(int), int_cmp);
-
-					for (j--, v++; j; j--, v++) {
-						if (*v != *prev_v) {
-							int dlt = v - prev_v;
-							if (dlt > 1) {
-								PRINT_ERR("\tPoly %u is invalid, it multi-uses vertex %d (%d times)\n",
-								          sp->index, *prev_v, dlt);
-								sp->invalid = true;
-							}
-							prev_v = v;
-						}
-					}
-					if (v - prev_v > 1) { /* Don't forget final verts! */
-						PRINT_ERR("\tPoly %u is invalid, it multi-uses vertex %d (%d times)\n",
-						          sp->index, *prev_v, (int)(v - prev_v));
-						sp->invalid = true;
-					}
+					/* Needed for checking polys using same verts below. */
+					qsort(sp->verts, sp->numverts, sizeof(int), int_cmp);
 				}
-			
 			}
 		}
 
@@ -630,66 +610,69 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
 		for (i = 1; i < totpoly; i++, sp++) {
 			int p1_nv = sp->numverts, p2_nv = prev_sp->numverts;
 			const int *p1_v = sp->verts, *p2_v = prev_sp->verts;
-			short p1_sub = true, p2_sub = true;
-			if (sp->invalid)
+
+			if (sp->invalid) {
+				/* break, because all known invalid polys have been put at the end by qsort with search_poly_cmp. */
 				break;
+			}
+
 			/* Test same polys. */
 #if 0
-			/* NOTE: This performs a sub-set test. */
-			/* XXX This (and the sort of verts list) is better than systematic
-			 *     search of all verts of one list into the other if lists have
-			 *     a fair amount of elements.
-			 *     Not sure however it's worth it in this case?
-			 *     But as we also need sorted vert list to check verts multi-used
-			 *     (in first pass of checks)... */
-			/* XXX If we consider only "equal" polys (i.e. using exactly same set of verts)
-			 *     as invalid, better to replace this by a simple memory cmp... */
-			while ((p1_nv && p2_nv) && (p1_sub || p2_sub)) {
-				if (*p1_v < *p2_v) {
-					if (p1_sub)
-						p1_sub = false;
-					p1_nv--;
-					p1_v++;
+			{
+				bool p1_sub = true, p2_sub = true;
+
+				/* NOTE: This performs a sub-set test. */
+				/* XXX This (and the sort of verts list) is better than systematic
+				 *     search of all verts of one list into the other if lists have
+				 *     a fair amount of elements.
+				 *     Not sure however it's worth it in this case?
+				 *     But as we also need sorted vert list to check verts multi-used
+				 *     (in first pass of checks)... */
+				/* XXX If we consider only "equal" polys (i.e. using exactly same set of verts)
+				 *     as invalid, better to replace this by a simple memory cmp... */
+				while ((p1_nv && p2_nv) && (p1_sub || p2_sub)) {
+					if (*p1_v < *p2_v) {
+						if (p1_sub)
+							p1_sub = false;
+						p1_nv--;
+						p1_v++;
+					}
+					else if (*p2_v < *p1_v) {
+						if (p2_sub)
+							p2_sub = false;
+						p2_nv--;
+						p2_v++;
+					}
+					else {
+						/* Equality, both next verts. */
+						p1_nv--;
+						p2_nv--;
+						p1_v++;
+						p2_v++;
+					}
 				}
-				else if (*p2_v < *p1_v) {
-					if (p2_sub)
-						p2_sub = false;
-					p2_nv--;
-					p2_v++;
+				if (p1_nv && p1_sub)
+					p1_sub = false;
+				else if (p2_nv && p2_sub)
+					p2_sub = false;
+
+				if (p1_sub && p2_sub) {
+					PRINT("\tPolys %u and %u use same vertices, considering poly %u as invalid.\n",
+						  prev_sp->index, sp->index, sp->index);
+					sp->invalid = true;
 				}
-				else {
-					/* Equality, both next verts. */
-					p1_nv--;
-					p2_nv--;
-					p1_v++;
-					p2_v++;
+				/* XXX In fact, these might be valid? :/ */
+				else if (p1_sub) {
+					PRINT("\t%u is a sub-poly of %u, considering it as invalid.\n", sp->index, prev_sp->index);
+					sp->invalid = true;
+				}
+				else if (p2_sub) {
+					PRINT("\t%u is a sub-poly of %u, considering it as invalid.\n", prev_sp->index, sp->index);
+					prev_sp->invalid = true;
+					prev_sp = sp; /* sp is new reference poly. */
 				}
-			}
-			if (p1_nv && p1_sub)
-				p1_sub = false;
-			else if (p2_nv && p2_sub)
-				p2_sub = false;
-
-			if (p1_sub && p2_sub) {
-				PRINT("\tPolys %u and %u use same vertices, considering poly %u as invalid.\n",
-				      prev_sp->index, sp->index, sp->index);
-				sp->invalid = true;
-			}
-			/* XXX In fact, these might be valid? :/ */
-			else if (p1_sub) {
-				PRINT("\t%u is a sub-poly of %u, considering it as invalid.\n", sp->index, prev_sp->index);
-				sp->invalid = true;
-			}
-			else if (p2_sub) {
-				PRINT("\t%u is a sub-poly of %u, considering it as invalid.\n", prev_sp->index, sp->index);
-				prev_sp->invalid = true;
-				prev_sp = sp; /* sp is new reference poly. */
 			}
 #else
-			if (0) {
-				p1_sub += 0;
-				p2_sub += 0;
-			}
 			if ((p1_nv == p2_nv) && (memcmp(p1_v, p2_v, p1_nv * sizeof(*p1_v)) == 0)) {
 				if (do_verbose) {
 					PRINT_ERR("\tPolys %u and %u use same vertices (%d",
@@ -887,9 +870,7 @@ bool BKE_mesh_validate_arrays(Mesh *mesh,
 
 	*r_changed = (fix_flag.as_flag || free_flag.as_flag || recalc_flag.as_flag);
 
-	if (do_fixes == false) {
-		BLI_assert(*r_changed == false);
-	}
+	BLI_assert((*r_changed == false) || (do_fixes == true));
 
 	return is_valid;
 }