[vertex tool] Respect topo editing when adding a vertex (fixes #18046)

When adding a vertex to a segment that is coincident with some other segments and topological editing is enabled, vertex tool will now correctly add new vertex also the coincident segments to preserve shared borders.
qgis · Nov 4, 2018 · 5dd5664 · 5dd5664
1 parent efbc089
commit 5dd5664
Show file tree

Hide file tree

Showing 3 changed files with 135 additions and 0 deletions.
diff --git a/src/app/vertextool/qgsvertextool.cpp b/src/app/vertextool/qgsvertextool.cpp
@@ -1372,6 +1372,26 @@ QList<QgsPointLocator::Match> QgsVertexTool::layerVerticesSnappedToPoint( QgsVec
   return myfilter.matches;
 }
 
+QList<QgsPointLocator::Match> QgsVertexTool::layerSegmentsSnappedToSegment( QgsVectorLayer *layer, const QgsPointXY &mapPoint1, const QgsPointXY &mapPoint2 )
+{
+  QList<QgsPointLocator::Match> finalMatches;
+  // we want segment matches that have exactly the same vertices as the given segment (mapPoint1, mapPoint2)
+  // so rather than doing nearest edge search which could return any segment within a tolerance,
+  // we first find matches for one endpoint and then see if there is a matching other endpoint.
+  const QList<QgsPointLocator::Match> matches1 = layerVerticesSnappedToPoint( layer, mapPoint1 );
+  for ( const QgsPointLocator::Match &m : matches1 )
+  {
+    QgsGeometry g = cachedGeometry( layer, m.featureId() );
+    int v0, v1;
+    g.adjacentVertices( m.vertexIndex(), v0, v1 );
+    if ( v0 != -1 && QgsPointXY( g.vertexAt( v0 ) ) == mapPoint2 )
+      finalMatches << QgsPointLocator::Match( QgsPointLocator::Edge, layer, m.featureId(), 0, m.point(), v0 );
+    else if ( v1 != -1 && QgsPointXY( g.vertexAt( v1 ) ) == mapPoint2 )
+      finalMatches << QgsPointLocator::Match( QgsPointLocator::Edge, layer, m.featureId(), 0, m.point(), m.vertexIndex() );
+  }
+  return finalMatches;
+}
+
 void QgsVertexTool::startDraggingAddVertex( const QgsPointLocator::Match &m )
 {
   Q_ASSERT( m.hasEdge() );
@@ -1383,6 +1403,7 @@ void QgsVertexTool::startDraggingAddVertex( const QgsPointLocator::Match &m )
   mDraggingVertexType = AddingVertex;
   mDraggingExtraVertices.clear();
   mDraggingExtraVerticesOffset.clear();
+  mDraggingExtraSegments.clear();
 
   QgsGeometry geom = cachedGeometry( m.layer(), m.featureId() );
 
@@ -1398,6 +1419,36 @@ void QgsVertexTool::startDraggingAddVertex( const QgsPointLocator::Match &m )
   if ( v1.x() != 0 || v1.y() != 0 )
     addDragBand( map_v1, m.point() );
 
+  if ( QgsProject::instance()->topologicalEditing() )
+  {
+    // find other segments coincident with the one user just picked and store them in a list
+    // so we can add a new vertex also in those to keep topology correct
+    const auto layers = canvas()->layers();
+    for ( QgsMapLayer *layer : layers )
+    {
+      QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
+      if ( !vlayer || !vlayer->isEditable() )
+        continue;
+
+      if ( vlayer->geometryType() != QgsWkbTypes::LineGeometry && vlayer->geometryType() != QgsWkbTypes::PolygonGeometry )
+        continue;
+
+      QgsPointXY pt1, pt2;
+      m.edgePoints( pt1, pt2 );
+      const auto snappedSegments = layerSegmentsSnappedToSegment( vlayer, pt1, pt2 );
+      for ( const QgsPointLocator::Match &otherMatch : snappedSegments )
+      {
+        if ( otherMatch.layer() == m.layer() &&
+             otherMatch.featureId() == m.featureId() &&
+             otherMatch.vertexIndex() == m.vertexIndex() )
+          continue;
+
+        // start dragging of snapped point of current layer
+        mDraggingExtraSegments << Vertex( otherMatch.layer(), otherMatch.featureId(), otherMatch.vertexIndex() );
+      }
+    }
+  }
+
   cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );
 }
 
@@ -1563,6 +1614,13 @@ void QgsVertexTool::moveVertex( const QgsPointXY &mapPoint, const QgsPointLocato
 
   addExtraVerticesToEdits( edits, mapPoint, dragLayer, layerPoint );
 
+  if ( addingVertex && !addingAtEndpoint && QgsProject::instance()->topologicalEditing() )
+  {
+    // topo editing: when adding a vertex to an existing segment, there may be other coincident segments
+    // that also need adding the same vertex
+    addExtraSegmentsToEdits( edits, mapPoint, dragLayer, layerPoint );
+  }
+
   applyEditsToLayers( edits );
 
   if ( QgsProject::instance()->topologicalEditing() && mapPointMatch->hasEdge() && mapPointMatch->layer() )
@@ -1625,6 +1683,40 @@ void QgsVertexTool::addExtraVerticesToEdits( QgsVertexTool::VertexEdits &edits,
 }
 
 
+void QgsVertexTool::addExtraSegmentsToEdits( QgsVertexTool::VertexEdits &edits, const QgsPointXY &mapPoint, QgsVectorLayer *dragLayer, const QgsPointXY &layerPoint )
+{
+  // insert new vertex also to other geometries/layers
+  for ( int i = 0; i < mDraggingExtraSegments.count(); ++i )
+  {
+    const Vertex &topo = mDraggingExtraSegments[i];
+
+    QHash<QgsFeatureId, QgsGeometry> &layerEdits = edits[topo.layer];
+    QgsGeometry topoGeom;
+    if ( layerEdits.contains( topo.fid ) )
+      topoGeom = QgsGeometry( edits[topo.layer][topo.fid] );
+    else
+      topoGeom = QgsGeometry( cachedGeometryForVertex( topo ) );
+
+    QgsPointXY point;
+    if ( dragLayer && topo.layer->crs() == dragLayer->crs() )
+      point = layerPoint;  // this point may come from exact match so it may be more precise
+    else
+      point = toLayerCoordinates( topo.layer, mapPoint );
+
+    QgsPoint pt( point );
+    if ( QgsWkbTypes::hasZ( topo.layer->wkbType() ) )
+      pt.addZValue( defaultZValue() );
+
+    if ( !topoGeom.insertVertex( pt, topo.vertexId + 1 ) )
+    {
+      QgsDebugMsg( QStringLiteral( "[topo] segment insert vertex failed!" ) );
+      continue;
+    }
+    edits[topo.layer][topo.fid] = topoGeom;
+  }
+}
+
+
 void QgsVertexTool::applyEditsToLayers( QgsVertexTool::VertexEdits &edits )
 {
   QHash<QgsVectorLayer *, QHash<QgsFeatureId, QgsGeometry> >::iterator it = edits.begin();

diff --git a/src/app/vertextool/qgsvertextool.h b/src/app/vertextool/qgsvertextool.h
@@ -156,6 +156,9 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing
     //! Gets list of matches of all vertices of a layer exactly snapped to a map point
     QList<QgsPointLocator::Match> layerVerticesSnappedToPoint( QgsVectorLayer *layer, const QgsPointXY &mapPoint );
 
+    //! Gets list of matches of all segments of a layer coincident with the given segment
+    QList<QgsPointLocator::Match> layerSegmentsSnappedToSegment( QgsVectorLayer *layer, const QgsPointXY &mapPoint1, const QgsPointXY &mapPoint2 );
+
     void startDraggingAddVertex( const QgsPointLocator::Match &m );
 
     void startDraggingAddVertexAtEndpoint( const QgsPointXY &mapPoint );
@@ -177,6 +180,8 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing
 
     void addExtraVerticesToEdits( VertexEdits &edits, const QgsPointXY &mapPoint, QgsVectorLayer *dragLayer = nullptr, const QgsPointXY &layerPoint = QgsPointXY() );
 
+    void addExtraSegmentsToEdits( QgsVertexTool::VertexEdits &edits, const QgsPointXY &mapPoint, QgsVectorLayer *dragLayer, const QgsPointXY &layerPoint );
+
     void applyEditsToLayers( VertexEdits &edits );
 
 
@@ -330,6 +335,12 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing
      */
     QList<QgsVector> mDraggingExtraVerticesOffset;
 
+    /**
+     * list of Vertex instances identifying segments (by their first vertex index) that should
+     * also get a new vertex: this is used for topo editing when adding a vertex to existing segment
+     */
+    QList<Vertex> mDraggingExtraSegments;
+
     // members for selection handling
 
     //! list of Vertex instances of vertices that are selected

diff --git a/tests/src/app/testqgsvertextool.cpp b/tests/src/app/testqgsvertextool.cpp
@@ -64,6 +64,7 @@ class TestQgsVertexTool : public QObject
     void testMoveMultipleVertices();
     void testMoveVertexTopo();
     void testDeleteVertexTopo();
+    void testAddVertexTopo();
     void testActiveLayerPriority();
     void testSelectedFeaturesPriority();
 
@@ -532,6 +533,37 @@ void TestQgsVertexTool::testDeleteVertexTopo()
   QgsProject::instance()->setTopologicalEditing( false );
 }
 
+void TestQgsVertexTool::testAddVertexTopo()
+{
+  // test addition of a vertex on a segment shared with another geometry
+
+  // add a temporary polygon
+  QgsFeature fTmp;
+  fTmp.setGeometry( QgsGeometry::fromWkt( "POLYGON((4 4, 7 4, 7 6, 4 6, 4 4))" ) );
+  bool resAdd = mLayerPolygon->addFeature( fTmp );
+  QVERIFY( resAdd );
+  QgsFeatureId fTmpId = fTmp.id();
+
+  QCOMPARE( mLayerPolygon->undoStack()->index(), 2 );
+
+  QgsProject::instance()->setTopologicalEditing( true );
+
+  mouseClick( 5.5, 4, Qt::LeftButton );
+  mouseClick( 5, 5, Qt::LeftButton );
+
+  QCOMPARE( mLayerPolygon->undoStack()->index(), 3 );
+
+  QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 5 5, 4 4, 4 1))" ) );
+  QCOMPARE( mLayerPolygon->getFeature( fTmpId ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 4, 5 5, 7 4, 7 6, 4 6, 4 4))" ) );
+
+  mLayerPolygon->undoStack()->undo();
+  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