Skip to content

Commit 93e737e

Browse files
wonder-skwonder-sk
committedNov 5, 2018
[vertex tool] Fix topo editing when moving vertices/edges (fixes #20158)
- when some "extra" vertices are selected when moving a vertex, their coincident vertices will be also moved (#20158) - when moving an edge, coincident vertices to its endpoints will be also moved - new tests to cover the above scenarios - made the code hopefully easier to read
1 parent 89a80c1 commit 93e737e

File tree

3 files changed

+168
-61
lines changed

3 files changed

+168
-61
lines changed
 

‎src/app/vertextool/qgsvertextool.cpp

Lines changed: 75 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -474,12 +474,9 @@ void QgsVertexTool::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
474474

475475
// for each editable layer, select vertices
476476
const auto layers = canvas()->layers();
477-
for ( QgsMapLayer *layer : layers )
477+
const auto editableLayers = editableVectorLayers();
478+
for ( QgsVectorLayer *vlayer : editableLayers )
478479
{
479-
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
480-
if ( !vlayer || !vlayer->isEditable() || !vlayer->isSpatial() )
481-
continue;
482-
483480
if ( mMode == ActiveLayer && vlayer != currentVectorLayer() )
484481
continue;
485482

@@ -1165,6 +1162,65 @@ void QgsVertexTool::startDragging( QgsMapMouseEvent *e )
11651162
}
11661163
}
11671164

1165+
QList<QgsVectorLayer *> QgsVertexTool::editableVectorLayers()
1166+
{
1167+
QList<QgsVectorLayer *> editableLayers;
1168+
const auto layers = canvas()->layers();
1169+
for ( QgsMapLayer *layer : layers )
1170+
{
1171+
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
1172+
if ( vlayer && vlayer->isEditable() && vlayer->isSpatial() )
1173+
editableLayers << vlayer;
1174+
}
1175+
return editableLayers;
1176+
}
1177+
1178+
QSet<Vertex> QgsVertexTool::findCoincidentVertices( const QSet<Vertex> &vertices )
1179+
{
1180+
QSet<Vertex> topoVertices;
1181+
const auto editableLayers = editableVectorLayers();
1182+
1183+
for ( const Vertex &v : qgis::as_const( vertices ) )
1184+
{
1185+
QgsPointXY origPointV = cachedGeometryForVertex( v ).vertexAt( v.vertexId );
1186+
QgsPointXY mapPointV = toMapCoordinates( v.layer, origPointV );
1187+
for ( QgsVectorLayer *vlayer : editableLayers )
1188+
{
1189+
const auto snappedVertices = layerVerticesSnappedToPoint( vlayer, mapPointV );
1190+
for ( const QgsPointLocator::Match &otherMatch : snappedVertices )
1191+
{
1192+
Vertex otherVertex( otherMatch.layer(), otherMatch.featureId(), otherMatch.vertexIndex() );
1193+
if ( !vertices.contains( otherVertex ) )
1194+
topoVertices << otherVertex;
1195+
}
1196+
}
1197+
}
1198+
return topoVertices;
1199+
}
1200+
1201+
void QgsVertexTool::buildExtraVertices( const QSet<Vertex> &vertices, const QgsPointXY &anchorPoint, QgsVectorLayer *anchorLayer )
1202+
{
1203+
for ( const Vertex &v : qgis::as_const( vertices ) )
1204+
{
1205+
if ( mDraggingVertex && v == *mDraggingVertex )
1206+
continue;
1207+
1208+
QgsPointXY pointV = cachedGeometryForVertex( v ).vertexAt( v.vertexId );
1209+
1210+
// Calculate position of the anchor point in the coordinates of our vertex v
1211+
// Try to avoid reprojection so we do not loose precision when map CRS does not match layer CRS
1212+
QgsPointXY anchorPointLayer;
1213+
if ( v.layer->crs() == anchorLayer->crs() )
1214+
anchorPointLayer = anchorPoint;
1215+
else // reprojection is necessary: ANCHOR -> MAP -> V
1216+
anchorPointLayer = toLayerCoordinates( v.layer, toMapCoordinates( anchorLayer, anchorPoint ) );
1217+
QgsVector offset = pointV - anchorPointLayer;
1218+
1219+
mDraggingExtraVertices << v;
1220+
mDraggingExtraVerticesOffset << offset;
1221+
}
1222+
}
1223+
11681224
void QgsVertexTool::startDraggingMoveVertex( const QgsPointLocator::Match &m )
11691225
{
11701226
Q_ASSERT( m.hasVertex() );
@@ -1179,59 +1235,22 @@ void QgsVertexTool::startDraggingMoveVertex( const QgsPointLocator::Match &m )
11791235

11801236
setHighlightedVerticesVisible( false ); // hide any extra highlight of vertices until we are done with moving
11811237

1182-
QgsPointXY origDraggingVertexPoint = geom.vertexAt( mDraggingVertex->vertexId );
1183-
1184-
// if there are other highlighted vertices, they should be dragged as well with their offset
1238+
QSet<Vertex> movingVertices;
1239+
movingVertices << *mDraggingVertex;
11851240
for ( const Vertex &v : qgis::as_const( mSelectedVertices ) )
1186-
{
1187-
if ( v != *mDraggingVertex )
1188-
{
1189-
QgsPointXY origPointV = cachedGeometryForVertex( v ).vertexAt( v.vertexId );
1190-
QgsPointXY origPointLayer = origDraggingVertexPoint;
1191-
if ( v.layer->crs() != mDraggingVertex->layer->crs() ) // reproject if necessary
1192-
origPointLayer = toLayerCoordinates( v.layer, toMapCoordinates( m.layer(), origDraggingVertexPoint ) );
1193-
QgsVector offset = origPointV - origPointLayer;
1194-
1195-
mDraggingExtraVertices << v;
1196-
mDraggingExtraVerticesOffset << offset;
1197-
}
1198-
}
1199-
1200-
cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );
1241+
movingVertices << v;
12011242

12021243
if ( QgsProject::instance()->topologicalEditing() )
12031244
{
1204-
// support for topo editing - find extra features
1205-
// that have coincident point with the vertex being dragged
1206-
const auto layers = canvas()->layers();
1207-
for ( QgsMapLayer *layer : layers )
1208-
{
1209-
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
1210-
if ( !vlayer || !vlayer->isEditable() )
1211-
continue;
1245+
movingVertices.unite( findCoincidentVertices( movingVertices ) );
1246+
}
12121247

1213-
const auto snappedVertices = layerVerticesSnappedToPoint( vlayer, m.point() );
1214-
for ( const QgsPointLocator::Match &otherMatch : snappedVertices )
1215-
{
1216-
if ( otherMatch.layer() == m.layer() &&
1217-
otherMatch.featureId() == m.featureId() &&
1218-
otherMatch.vertexIndex() == m.vertexIndex() )
1219-
continue;
1248+
buildExtraVertices( movingVertices, geom.vertexAt( m.vertexIndex() ), m.layer() );
12201249

1221-
// start dragging of snapped point of current layer
1222-
mDraggingExtraVertices << Vertex( otherMatch.layer(), otherMatch.featureId(), otherMatch.vertexIndex() );
1223-
mDraggingExtraVerticesOffset << QgsVector(); // topo vertices have the same position
1224-
}
1225-
}
1226-
}
1250+
cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );
12271251

12281252
// now build drag rubber bands for extra vertices
12291253

1230-
QSet<Vertex> movingVertices;
1231-
movingVertices << *mDraggingVertex;
1232-
for ( const Vertex &v : qgis::as_const( mDraggingExtraVertices ) )
1233-
movingVertices << v;
1234-
12351254
QgsPointXY dragVertexMapPoint = m.point();
12361255

12371256
buildDragBandsForVertices( movingVertices, dragVertexMapPoint );
@@ -1423,13 +1442,9 @@ void QgsVertexTool::startDraggingAddVertex( const QgsPointLocator::Match &m )
14231442
{
14241443
// find other segments coincident with the one user just picked and store them in a list
14251444
// so we can add a new vertex also in those to keep topology correct
1426-
const auto layers = canvas()->layers();
1427-
for ( QgsMapLayer *layer : layers )
1445+
const auto editableLayers = editableVectorLayers();
1446+
for ( QgsVectorLayer *vlayer : editableLayers )
14281447
{
1429-
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
1430-
if ( !vlayer || !vlayer->isEditable() )
1431-
continue;
1432-
14331448
if ( vlayer->geometryType() != QgsWkbTypes::LineGeometry && vlayer->geometryType() != QgsWkbTypes::PolygonGeometry )
14341449
continue;
14351450

@@ -1506,14 +1521,14 @@ void QgsVertexTool::startDraggingEdge( const QgsPointLocator::Match &m, const Qg
15061521

15071522
buildDragBandsForVertices( movingVertices, mapPoint );
15081523

1509-
QgsPointXY layerPoint = toLayerCoordinates( m.layer(), mapPoint );
1510-
1511-
for ( const Vertex &v : qgis::as_const( movingVertices ) )
1524+
if ( QgsProject::instance()->topologicalEditing() )
15121525
{
1513-
mDraggingExtraVertices << v;
1514-
mDraggingExtraVerticesOffset << ( geom.vertexAt( v.vertexId ) - QgsPoint( layerPoint ) );
1526+
movingVertices.unite( findCoincidentVertices( movingVertices ) );
15151527
}
15161528

1529+
QgsPointXY layerPoint = toLayerCoordinates( m.layer(), mapPoint );
1530+
buildExtraVertices( movingVertices, layerPoint, m.layer() );
1531+
15171532
cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );
15181533
}
15191534

‎src/app/vertextool/qgsvertextool.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,20 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing
184184

185185
void applyEditsToLayers( VertexEdits &edits );
186186

187+
/**
188+
* For the given set of vertices (possibly from multiple layers) find any another vertices that are coincident with
189+
* them and not yet included in the set. This is used for topological editing to find all vertices that should be moved.
190+
*/
191+
QSet<Vertex> findCoincidentVertices( const QSet<Vertex> &vertices );
192+
193+
/**
194+
* For the given set of vertices, prepare mDraggingExtraVertices and mDraggingExtraVerticesOffset arrays.
195+
* The parameters anchorPoint and anchorLayer are used to calculate offsets.
196+
*/
197+
void buildExtraVertices( const QSet<Vertex> &vertices, const QgsPointXY &anchorPoint, QgsVectorLayer *anchorLayer );
198+
199+
//! Returns a list of canvas layers filtered to just editable spatial vector layers
200+
QList<QgsVectorLayer *> editableVectorLayers();
187201

188202
enum HighlightMode
189203
{
@@ -382,7 +396,7 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing
382396
*/
383397
std::unique_ptr<QgsPointLocator::Match> mNewVertexFromDoubleClick;
384398

385-
//! Geometry cache for fast access to geometries
399+
//! Geometry cache for fast access to geometries (coordinates are in their layer's CRS)
386400
QHash<const QgsVectorLayer *, QHash<QgsFeatureId, QgsGeometry> > mCache;
387401

388402
// support for vertex editor

‎tests/src/app/testqgsvertextool.cpp

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ class TestQgsVertexTool : public QObject
6262
void testAddVertexAtEndpoint();
6363
void testDeleteVertex();
6464
void testMoveMultipleVertices();
65+
void testMoveMultipleVertices2();
6566
void testMoveVertexTopo();
6667
void testDeleteVertexTopo();
6768
void testAddVertexTopo();
69+
void testMoveEdgeTopo();
6870
void testActiveLayerPriority();
6971
void testSelectedFeaturesPriority();
7072

@@ -475,6 +477,25 @@ void TestQgsVertexTool::testMoveMultipleVertices()
475477
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
476478
}
477479

480+
void TestQgsVertexTool::testMoveMultipleVertices2()
481+
{
482+
// this time select two vertices with shift
483+
mouseClick( 1, 1, Qt::LeftButton, Qt::ShiftModifier );
484+
mouseClick( 2, 1, Qt::LeftButton, Qt::ShiftModifier );
485+
486+
// move them by +1, +1
487+
mouseClick( 1, 1, Qt::LeftButton );
488+
mouseClick( 2, 2, Qt::LeftButton );
489+
490+
QCOMPARE( mLayerLine->undoStack()->index(), 2 );
491+
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(3 2, 2 2, 1 3)" ) );
492+
493+
mLayerLine->undoStack()->undo();
494+
QCOMPARE( mLayerLine->undoStack()->index(), 1 );
495+
496+
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
497+
}
498+
478499
void TestQgsVertexTool::testMoveVertexTopo()
479500
{
480501
// test moving of vertices of two features at once
@@ -564,6 +585,63 @@ void TestQgsVertexTool::testAddVertexTopo()
564585
QgsProject::instance()->setTopologicalEditing( false );
565586
}
566587

588+
void TestQgsVertexTool::testMoveEdgeTopo()
589+
{
590+
// test move of an edge shared with another feature
591+
592+
// add a temporary polygon
593+
QgsFeature fTmp;
594+
fTmp.setGeometry( QgsGeometry::fromWkt( "POLYGON((4 4, 7 4, 7 6, 4 6, 4 4))" ) );
595+
bool resAdd = mLayerPolygon->addFeature( fTmp );
596+
QVERIFY( resAdd );
597+
QgsFeatureId fTmpId = fTmp.id();
598+
599+
QCOMPARE( mLayerPolygon->undoStack()->index(), 2 );
600+
601+
QgsProject::instance()->setTopologicalEditing( true );
602+
603+
// move shared segment
604+
mouseClick( 6, 4, Qt::LeftButton );
605+
mouseClick( 6, 5, Qt::LeftButton );
606+
607+
QCOMPARE( mLayerPolygon->undoStack()->index(), 3 );
608+
609+
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 5, 4 5, 4 1))" ) );
610+
QCOMPARE( mLayerPolygon->getFeature( fTmpId ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 5, 7 5, 7 6, 4 6, 4 5))" ) );
611+
612+
mLayerPolygon->undoStack()->undo();
613+
614+
// another test to move a shared segment - but this time we just pick two individual points of a feature
615+
// and do vertex move
616+
617+
QgsProject::instance()->setTopologicalEditing( false );
618+
619+
// this time select two vertices with shift
620+
mouseClick( 4, 4, Qt::LeftButton, Qt::ShiftModifier );
621+
mouseClick( 7, 4, Qt::LeftButton, Qt::ShiftModifier );
622+
623+
QgsProject::instance()->setTopologicalEditing( true );
624+
625+
// now move the shared segment
626+
mouseClick( 4, 4, Qt::LeftButton );
627+
mouseClick( 4, 3, Qt::LeftButton );
628+
629+
QCOMPARE( mLayerPolygon->undoStack()->index(), 3 );
630+
631+
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 3, 4 3, 4 1))" ) );
632+
QCOMPARE( mLayerPolygon->getFeature( fTmpId ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 3, 7 3, 7 6, 4 6, 4 3))" ) );
633+
634+
mLayerPolygon->undoStack()->undo();
635+
636+
//
637+
638+
mLayerPolygon->undoStack()->undo();
639+
640+
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );
641+
642+
QgsProject::instance()->setTopologicalEditing( false );
643+
}
644+
567645
void TestQgsVertexTool::testActiveLayerPriority()
568646
{
569647
// check that features from current layer get priority when picking points

0 commit comments

Comments
 (0)