duplicateNonManifoldVertices: construct rings from close faces (#6358)

Author: Fedr

source/MRMesh/MRMeshBuilder.cpp

@@ -559,33 +559,34 @@ class PathOverIncidentVert
     Triangulation& faceToVertices;
     // all iterators in [vertexBegIt, vertexEndIt) must have the same central vertex
     std::vector<IncidentVert>::iterator vertexBegIt, vertexEndIt;
-    size_t lastUnvisitedIndex = 0; // pivot index. [vertexBegIt, vertexBegIt + lastUnvisitedIndex) - unvisited vertices
+    size_t firstUnvisitedIndex = 0; // pivot index. [vertexBegIt + firstUnvistedIndex, vertexBegIt) - unvisited vertices
 
 public:
     PathOverIncidentVert( Triangulation& triangleToVertices,
                 std::vector<IncidentVert>& incidentItemsVector, size_t beg, size_t end )
         : faceToVertices( triangleToVertices )
         , vertexBegIt( incidentItemsVector.begin() + beg )
         , vertexEndIt( incidentItemsVector.begin() + end )
-        , lastUnvisitedIndex( end - beg )
     {}
 
     // false if there are some unvisited vertices
     bool empty() const
     {
-        return lastUnvisitedIndex <= 0;
+        return vertexBegIt + firstUnvisitedIndex >= vertexEndIt;
     }
 
     // first unvisited vertex
     VertId getFirstVertex() const
     {
+        assert( !empty() );
+        const auto first = vertexBegIt + firstUnvisitedIndex;
         // below selection ensures that getNextIncidentVertex( getFirstVertex(), true ) will find nextVertex in the very first triangle
-        const auto & vs = faceToVertices[vertexBegIt->f];
-        if ( vs[0] == vertexBegIt->srcVert )
+        const auto & vs = faceToVertices[first->f];
+        if ( vs[0] == first->srcVert )
             return vs[1];
-        if ( vs[1] == vertexBegIt->srcVert )
+        if ( vs[1] == first->srcVert )
             return vs[2];
-        if ( vs[2] == vertexBegIt->srcVert )
+        if ( vs[2] == first->srcVert )
             return vs[0];
         assert( false );
         return {};
@@ -594,11 +595,11 @@ class PathOverIncidentVert
     // find incident unvisited vertex, in case of several option prefer finding the vertex not equal to preVertex
     VertId getNextIncidentVertex( VertId v, bool triOrientation, VertId prevVertex = {} )
     {
-        if ( lastUnvisitedIndex <= 0 )
+        if ( empty() )
             return VertId( -1 );
 
-        auto prevIt = vertexBegIt + lastUnvisitedIndex;
-        for ( auto it = vertexBegIt; it < vertexBegIt + lastUnvisitedIndex; ++it )
+        auto prevIt = vertexEndIt;
+        for ( auto it = vertexBegIt + firstUnvisitedIndex; it < vertexEndIt; ++it )
         {
             VertId nextVertex;
             const auto & vs = faceToVertices[it->f];
@@ -624,19 +625,21 @@ class PathOverIncidentVert
             {
                 if ( nextVertex != prevVertex )
                 {
-                    --lastUnvisitedIndex;
-                    std::iter_swap( it, vertexBegIt + lastUnvisitedIndex );
+                    if ( it != vertexBegIt + firstUnvisitedIndex )
+                        std::iter_swap( it, vertexBegIt + firstUnvisitedIndex );
+                    ++firstUnvisitedIndex;
                     return nextVertex;
                 }
                 // prevVertex is a possible continuation, store it, and search for other options
                 prevIt = it;
             }
         }
-        if ( prevIt < vertexBegIt + lastUnvisitedIndex )
+        if ( prevIt < vertexEndIt )
         {
             // the only option is return in prevVertex
-            --lastUnvisitedIndex;
-            std::iter_swap( prevIt, vertexBegIt + lastUnvisitedIndex );
+            if ( prevIt != vertexBegIt + firstUnvisitedIndex )
+                std::iter_swap( prevIt, vertexBegIt + firstUnvisitedIndex );
+            ++firstUnvisitedIndex;
             return prevVertex;
         }
         return {};
@@ -654,7 +657,7 @@ class PathOverIncidentVert
 
         for ( size_t i = 1; i < path.size(); ++i )
         {
-            for ( auto it = vertexBegIt + lastUnvisitedIndex; it < vertexEndIt; ++it )
+            for ( auto it = vertexBegIt; it < vertexBegIt + firstUnvisitedIndex; ++it )
             {
                 VertId v1, v2;
                 bool alreadyDuplicted = true;

source/MRTest/MRMeshBuilderTests.cpp

@@ -50,7 +50,10 @@ static void testBuildWithDups( const char * objMesh, int numVerts, int numComps
 
 TEST( MRMesh, MeshBuildWithDups )
 {
-    // this test case passed always
+    // first 4 triangles subdivide a square with center point,
+    // following 4 triangles subdivide the opposite side of same triangle,
+    // all 5 points are shared,
+    // expected that only center vetrex is duplicated and single connected component remains
     testBuildWithDups
     (
         "v 0 0.5 0\n"
@@ -68,7 +71,7 @@ TEST( MRMesh, MeshBuildWithDups )
         "f 1 4 5\n", 6, 1
     );
 
-    // this test start passing only after recent improvements (but will fail in case vertex reordering)
+    // same situation as above with the order of triangles changed
     testBuildWithDups
     (
         "v 0 0.5 0\n"
@@ -85,6 +88,29 @@ TEST( MRMesh, MeshBuildWithDups )
         "f 1 5 4\n"
         "f 3 5 1\n", 6, 1
     );
+
+    // first 4 triangles subdivide a square with center point,
+    // following 4 triangles subdivide same square with center point,
+    // 3 points on one diagonal are shared,
+    // it is divided properly on two components if the rings are computed from the smallest by id next triangle
+    testBuildWithDups
+    (
+        "v -1 0 -1\n"
+        "v  1 0 -1\n"
+        "v -1 0  1\n"
+        "v  1 0  1\n"
+        "v -1 0 -1\n"
+        "v  1 0  1\n"
+        "v  0 0  0\n"
+        "f 7 1 3\n"
+        "f 1 7 2\n"
+        "f 7 3 4\n"
+        "f 2 7 4\n"
+        "f 5 7 2\n"
+        "f 7 3 6\n"
+        "f 2 7 6\n"
+        "f 7 5 3\n", 10, 2
+    );
 }
 
 } //namespace MeshBuilder