Skip to content
Permalink
Browse files

[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

(cherry picked from commit 93e737e)
  • Loading branch information
wonder-sk authored and nyalldawson committed Nov 4, 2018
1 parent d737e56 commit 2f5a87fda95b943c890bea814653f8e8b05cd4d2
Showing with 168 additions and 61 deletions.
  1. +75 −60 src/app/vertextool/qgsvertextool.cpp
  2. +15 −1 src/app/vertextool/qgsvertextool.h
  3. +78 −0 tests/src/app/testqgsvertextool.cpp
@@ -474,12 +474,9 @@ void QgsVertexTool::cadCanvasReleaseEvent( QgsMapMouseEvent *e )

// for each editable layer, select vertices
const auto layers = canvas()->layers();
for ( QgsMapLayer *layer : layers )
const auto editableLayers = editableVectorLayers();
for ( QgsVectorLayer *vlayer : editableLayers )
{
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
if ( !vlayer || !vlayer->isEditable() || !vlayer->isSpatial() )
continue;

if ( mMode == ActiveLayer && vlayer != currentVectorLayer() )
continue;

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

QList<QgsVectorLayer *> QgsVertexTool::editableVectorLayers()
{
QList<QgsVectorLayer *> editableLayers;
const auto layers = canvas()->layers();
for ( QgsMapLayer *layer : layers )
{
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
if ( vlayer && vlayer->isEditable() && vlayer->isSpatial() )
editableLayers << vlayer;
}
return editableLayers;
}

QSet<Vertex> QgsVertexTool::findCoincidentVertices( const QSet<Vertex> &vertices )
{
QSet<Vertex> topoVertices;
const auto editableLayers = editableVectorLayers();

for ( const Vertex &v : qgis::as_const( vertices ) )
{
QgsPointXY origPointV = cachedGeometryForVertex( v ).vertexAt( v.vertexId );
QgsPointXY mapPointV = toMapCoordinates( v.layer, origPointV );
for ( QgsVectorLayer *vlayer : editableLayers )
{
const auto snappedVertices = layerVerticesSnappedToPoint( vlayer, mapPointV );
for ( const QgsPointLocator::Match &otherMatch : snappedVertices )
{
Vertex otherVertex( otherMatch.layer(), otherMatch.featureId(), otherMatch.vertexIndex() );
if ( !vertices.contains( otherVertex ) )
topoVertices << otherVertex;
}
}
}
return topoVertices;
}

void QgsVertexTool::buildExtraVertices( const QSet<Vertex> &vertices, const QgsPointXY &anchorPoint, QgsVectorLayer *anchorLayer )
{
for ( const Vertex &v : qgis::as_const( vertices ) )
{
if ( mDraggingVertex && v == *mDraggingVertex )
continue;

QgsPointXY pointV = cachedGeometryForVertex( v ).vertexAt( v.vertexId );

// Calculate position of the anchor point in the coordinates of our vertex v
// Try to avoid reprojection so we do not loose precision when map CRS does not match layer CRS
QgsPointXY anchorPointLayer;
if ( v.layer->crs() == anchorLayer->crs() )
anchorPointLayer = anchorPoint;
else // reprojection is necessary: ANCHOR -> MAP -> V
anchorPointLayer = toLayerCoordinates( v.layer, toMapCoordinates( anchorLayer, anchorPoint ) );
QgsVector offset = pointV - anchorPointLayer;

mDraggingExtraVertices << v;
mDraggingExtraVerticesOffset << offset;
}
}

void QgsVertexTool::startDraggingMoveVertex( const QgsPointLocator::Match &m )
{
Q_ASSERT( m.hasVertex() );
@@ -1179,59 +1235,22 @@ void QgsVertexTool::startDraggingMoveVertex( const QgsPointLocator::Match &m )

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

QgsPointXY origDraggingVertexPoint = geom.vertexAt( mDraggingVertex->vertexId );

// if there are other highlighted vertices, they should be dragged as well with their offset
QSet<Vertex> movingVertices;
movingVertices << *mDraggingVertex;
for ( const Vertex &v : qgis::as_const( mSelectedVertices ) )
{
if ( v != *mDraggingVertex )
{
QgsPointXY origPointV = cachedGeometryForVertex( v ).vertexAt( v.vertexId );
QgsPointXY origPointLayer = origDraggingVertexPoint;
if ( v.layer->crs() != mDraggingVertex->layer->crs() ) // reproject if necessary
origPointLayer = toLayerCoordinates( v.layer, toMapCoordinates( m.layer(), origDraggingVertexPoint ) );
QgsVector offset = origPointV - origPointLayer;

mDraggingExtraVertices << v;
mDraggingExtraVerticesOffset << offset;
}
}

cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );
movingVertices << v;

if ( QgsProject::instance()->topologicalEditing() )
{
// support for topo editing - find extra features
// that have coincident point with the vertex being dragged
const auto layers = canvas()->layers();
for ( QgsMapLayer *layer : layers )
{
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
if ( !vlayer || !vlayer->isEditable() )
continue;
movingVertices.unite( findCoincidentVertices( movingVertices ) );
}

const auto snappedVertices = layerVerticesSnappedToPoint( vlayer, m.point() );
for ( const QgsPointLocator::Match &otherMatch : snappedVertices )
{
if ( otherMatch.layer() == m.layer() &&
otherMatch.featureId() == m.featureId() &&
otherMatch.vertexIndex() == m.vertexIndex() )
continue;
buildExtraVertices( movingVertices, geom.vertexAt( m.vertexIndex() ), m.layer() );

// start dragging of snapped point of current layer
mDraggingExtraVertices << Vertex( otherMatch.layer(), otherMatch.featureId(), otherMatch.vertexIndex() );
mDraggingExtraVerticesOffset << QgsVector(); // topo vertices have the same position
}
}
}
cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );

// now build drag rubber bands for extra vertices

QSet<Vertex> movingVertices;
movingVertices << *mDraggingVertex;
for ( const Vertex &v : qgis::as_const( mDraggingExtraVertices ) )
movingVertices << v;

QgsPointXY dragVertexMapPoint = m.point();

buildDragBandsForVertices( movingVertices, dragVertexMapPoint );
@@ -1423,13 +1442,9 @@ void QgsVertexTool::startDraggingAddVertex( const QgsPointLocator::Match &m )
{
// 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 )
const auto editableLayers = editableVectorLayers();
for ( QgsVectorLayer *vlayer : editableLayers )
{
QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
if ( !vlayer || !vlayer->isEditable() )
continue;

if ( vlayer->geometryType() != QgsWkbTypes::LineGeometry && vlayer->geometryType() != QgsWkbTypes::PolygonGeometry )
continue;

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

buildDragBandsForVertices( movingVertices, mapPoint );

QgsPointXY layerPoint = toLayerCoordinates( m.layer(), mapPoint );

for ( const Vertex &v : qgis::as_const( movingVertices ) )
if ( QgsProject::instance()->topologicalEditing() )
{
mDraggingExtraVertices << v;
mDraggingExtraVerticesOffset << ( geom.vertexAt( v.vertexId ) - QgsPoint( layerPoint ) );
movingVertices.unite( findCoincidentVertices( movingVertices ) );
}

QgsPointXY layerPoint = toLayerCoordinates( m.layer(), mapPoint );
buildExtraVertices( movingVertices, layerPoint, m.layer() );

cadDockWidget()->setPoints( QList<QgsPointXY>() << m.point() << m.point() );
}

@@ -184,6 +184,20 @@ class APP_EXPORT QgsVertexTool : public QgsMapToolAdvancedDigitizing

void applyEditsToLayers( VertexEdits &edits );

/**
* For the given set of vertices (possibly from multiple layers) find any another vertices that are coincident with
* them and not yet included in the set. This is used for topological editing to find all vertices that should be moved.
*/
QSet<Vertex> findCoincidentVertices( const QSet<Vertex> &vertices );

/**
* For the given set of vertices, prepare mDraggingExtraVertices and mDraggingExtraVerticesOffset arrays.
* The parameters anchorPoint and anchorLayer are used to calculate offsets.
*/
void buildExtraVertices( const QSet<Vertex> &vertices, const QgsPointXY &anchorPoint, QgsVectorLayer *anchorLayer );

//! Returns a list of canvas layers filtered to just editable spatial vector layers
QList<QgsVectorLayer *> editableVectorLayers();

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

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

// support for vertex editor
@@ -62,9 +62,11 @@ class TestQgsVertexTool : public QObject
void testAddVertexAtEndpoint();
void testDeleteVertex();
void testMoveMultipleVertices();
void testMoveMultipleVertices2();
void testMoveVertexTopo();
void testDeleteVertexTopo();
void testAddVertexTopo();
void testMoveEdgeTopo();
void testActiveLayerPriority();
void testSelectedFeaturesPriority();

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

void TestQgsVertexTool::testMoveMultipleVertices2()
{
// this time select two vertices with shift
mouseClick( 1, 1, Qt::LeftButton, Qt::ShiftModifier );
mouseClick( 2, 1, Qt::LeftButton, Qt::ShiftModifier );

// move them by +1, +1
mouseClick( 1, 1, Qt::LeftButton );
mouseClick( 2, 2, Qt::LeftButton );

QCOMPARE( mLayerLine->undoStack()->index(), 2 );
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(3 2, 2 2, 1 3)" ) );

mLayerLine->undoStack()->undo();
QCOMPARE( mLayerLine->undoStack()->index(), 1 );

QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
}

void TestQgsVertexTool::testMoveVertexTopo()
{
// test moving of vertices of two features at once
@@ -564,6 +585,63 @@ void TestQgsVertexTool::testAddVertexTopo()
QgsProject::instance()->setTopologicalEditing( false );
}

void TestQgsVertexTool::testMoveEdgeTopo()
{
// test move of an edge shared with another feature

// 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 );

// move shared segment
mouseClick( 6, 4, Qt::LeftButton );
mouseClick( 6, 5, Qt::LeftButton );

QCOMPARE( mLayerPolygon->undoStack()->index(), 3 );

QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 5, 4 5, 4 1))" ) );
QCOMPARE( mLayerPolygon->getFeature( fTmpId ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 5, 7 5, 7 6, 4 6, 4 5))" ) );

mLayerPolygon->undoStack()->undo();

// another test to move a shared segment - but this time we just pick two individual points of a feature
// and do vertex move

QgsProject::instance()->setTopologicalEditing( false );

// this time select two vertices with shift
mouseClick( 4, 4, Qt::LeftButton, Qt::ShiftModifier );
mouseClick( 7, 4, Qt::LeftButton, Qt::ShiftModifier );

QgsProject::instance()->setTopologicalEditing( true );

// now move the shared segment
mouseClick( 4, 4, Qt::LeftButton );
mouseClick( 4, 3, Qt::LeftButton );

QCOMPARE( mLayerPolygon->undoStack()->index(), 3 );

QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 3, 4 3, 4 1))" ) );
QCOMPARE( mLayerPolygon->getFeature( fTmpId ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 3, 7 3, 7 6, 4 6, 4 3))" ) );

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

0 comments on commit 2f5a87f

Please sign in to comment.
You can’t perform that action at this time.