[vertex tool] Fix vertex addition to polygon's first segment (fixes #20774)

wonder-sk · wonder-sk · commit 3769faa86b89 · 2019-01-30T10:23:36.000+01:00
With topo editing mode enabled, addition of extra points to keep the topology
correct wasn't working correctly because for the first segment we were getting
two matches due to duplicated first and last vertex in the ring. The fix
ensures that only one match will be returned for the first duplicated vertex.
diff --git a/src/app/vertextool/qgsvertextool.cpp b/src/app/vertextool/qgsvertextool.cpp
@@ -172,18 +172,35 @@ class MatchCollectingFilter : public QgsPointLocator::MatchFilter
     {
       if ( match.distance() > 0 )
         return false;
-      matches.append( match );
 
       // there may be multiple points at the same location, but we get only one
       // result... the locator API needs a new method verticesInRect()
       QgsGeometry matchGeom = vertextool->cachedGeometry( match.layer(), match.featureId() );
+      bool isPolygon = matchGeom.type() == QgsWkbTypes::PolygonGeometry;
+      QgsVertexId polygonRingVid;
       QgsVertexId vid;
       QgsPoint pt;
       while ( matchGeom.constGet()->nextVertex( vid, pt ) )
       {
         int vindex = matchGeom.vertexNrFromVertexId( vid );
-        if ( pt.x() == match.point().x() && pt.y() == match.point().y() && vindex != match.vertexIndex() )
+        if ( pt.x() == match.point().x() && pt.y() == match.point().y() )
         {
+          if ( isPolygon )
+          {
+            // for polygons we need to handle the case where the first vertex is matching because the
+            // last point will have the same coordinates and we would have a duplicate match which
+            // would make subsequent code behave incorrectly (topology editing mode would add a single
+            // vertex twice)
+            if ( vid.vertex == 0 )
+            {
+              polygonRingVid = vid;
+            }
+            else if ( vid.ringEqual( polygonRingVid ) && vid.vertex == matchGeom.constGet()->vertexCount( vid.part, vid.ring ) - 1 )
+            {
+              continue;
+            }
+          }
+
           QgsPointLocator::Match extra_match( match.type(), match.layer(), match.featureId(),
                                               0, match.point(), vindex );
           matches.append( extra_match );
diff --git a/tests/src/app/testqgsvertextool.cpp b/tests/src/app/testqgsvertextool.cpp
@@ -67,6 +67,7 @@ class TestQgsVertexTool : public QObject
     void testDeleteVertexTopo();
     void testAddVertexTopo();
     void testMoveEdgeTopo();
+    void testAddVertexTopoFirstSegment();
     void testActiveLayerPriority();
     void testSelectedFeaturesPriority();
 
@@ -642,6 +643,27 @@ void TestQgsVertexTool::testMoveEdgeTopo()
   QgsProject::instance()->setTopologicalEditing( false );
 }
 
+void TestQgsVertexTool::testAddVertexTopoFirstSegment()
+{
+  // check that when adding a vertex to the first segment of a polygon's ring with topo editing
+  // enabled, the geometry does not get corrupted (#20774)
+
+  QgsProject::instance()->setTopologicalEditing( true );
+
+  mouseClick( 5.5, 1, Qt::LeftButton );
+  mouseClick( 5, 2, Qt::LeftButton );
+
+  QCOMPARE( mLayerPolygon->undoStack()->index(), 2 );
+
+  QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 5 2, 7 1, 7 4, 4 4, 4 1))" ) );
+
+  mLayerPolygon->undoStack()->undo();
+
+  QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );
+
+  QgsProject::instance()->setTopologicalEditing( false );
+}
+
 void TestQgsVertexTool::testActiveLayerPriority()
 {
   // check that features from current layer get priority when picking points
