Skip to content
Permalink
Browse files
Node tool: improvements for deletion of vertices
- fix issue when deleting the repeated first+last vertex of polygon's ring
- make deletion aware of topological editing (also delete other vertices at the same position)
- fix issue with dragging not being stopped when deleting multiple vertices
- make deletion in node editor follow the same code path
- unit tests for topological editing (moving / deleting vertices)
  • Loading branch information
wonder-sk committed Apr 18, 2017
1 parent e6aa44f commit 832d2dd0451f06c6509d07f8ff50317f6c07bf3f
Showing with 149 additions and 101 deletions.
  1. +89 −11 src/app/nodetool/qgsnodetool.cpp
  2. +0 −85 src/app/nodetool/qgsselectedfeature.cpp
  3. +0 −5 src/app/nodetool/qgsselectedfeature.h
  4. +60 −0 tests/src/app/testqgsnodetool.cpp
@@ -945,13 +945,27 @@ void QgsNodeTool::deleteNodeEditorSelection()
if ( firstSelectedIndex == -1 )
return;

mSelectedFeature->deleteSelectedVertexes();

if ( mSelectedFeature->geometry()->isNull() )
{
emit messageEmitted( tr( "Geometry has been cleared. Use the add part tool to set geometry for this feature." ) );
// make a list of selected vertices
QList<Vertex> nodes;
QList<QgsVertexEntry *> &selFeatureVertices = mSelectedFeature->vertexMap();
QgsVectorLayer *layer = mSelectedFeature->vlayer();
QgsFeatureId fid = mSelectedFeature->featureId();
QgsGeometry geometry = cachedGeometry( layer, fid );
Q_FOREACH ( QgsVertexEntry *vertex, selFeatureVertices )
{
if ( vertex->isSelected() )
{
int vertexIndex = geometry.vertexNrFromVertexId( vertex->vertexId() );
if ( vertexIndex != -1 )
nodes.append( Vertex( layer, fid, vertexIndex ) );
}
}
else

// now select the vertices and delete them...
setHighlightedNodes( nodes );
deleteVertex();

if ( !mSelectedFeature->geometry()->isNull() )
{
int nextVertexToSelect = firstSelectedIndex;
if ( mSelectedFeature->geometry()->type() == QgsWkbTypes::LineGeometry )
@@ -1475,31 +1489,90 @@ void QgsNodeTool::applyEditsToLayers( QgsNodeTool::NodeEdits &edits )

void QgsNodeTool::deleteVertex()
{
QList<Vertex> toDelete;
QSet<Vertex> toDelete;
if ( !mSelectedNodes.isEmpty() )
{
toDelete = mSelectedNodes;
toDelete = QSet<Vertex>::fromList( mSelectedNodes );
}
else
{
bool addingVertex = mDraggingVertexType == AddingVertex || mDraggingVertexType == AddingEndpoint;
toDelete << *mDraggingVertex;
toDelete += mDraggingExtraVertices;
stopDragging();
toDelete += QSet<Vertex>::fromList( mDraggingExtraVertices );

if ( addingVertex )
{
stopDragging();
return; // just cancel the vertex
}
}

stopDragging();
setHighlightedNodes( QList<Vertex>() ); // reset selection

if ( QgsProject::instance()->topologicalEditing() )
{
// if topo editing is enabled, delete all the vertices that are on the same location
QSet<Vertex> topoVerticesToDelete;
Q_FOREACH ( const Vertex &vertexToDelete, toDelete )
{
QgsPoint layerPt = cachedGeometryForVertex( vertexToDelete ).vertexAt( vertexToDelete.vertexId );
QgsPoint mapPt = toMapCoordinates( vertexToDelete.layer, layerPt );
Q_FOREACH ( const QgsPointLocator::Match &otherMatch, layerVerticesSnappedToPoint( vertexToDelete.layer, mapPt ) )
{
Vertex otherVertex( otherMatch.layer(), otherMatch.featureId(), otherMatch.vertexIndex() );
if ( toDelete.contains( otherVertex ) || topoVerticesToDelete.contains( otherVertex ) )
continue;

topoVerticesToDelete.insert( otherVertex );
}
}

toDelete.unite( topoVerticesToDelete );
}

// switch from a plain list to dictionary { layer: { fid: [vertexNr1, vertexNr2, ...] } }
QHash<QgsVectorLayer *, QHash<QgsFeatureId, QList<int> > > toDeleteGrouped;
Q_FOREACH ( const Vertex &vertex, toDelete )
{
toDeleteGrouped[vertex.layer][vertex.fid].append( vertex.vertexId );
}

// de-duplicate vertices in linear rings - if there is the first vertex selected,
// then also the last vertex will be selected - but we want just one out of the pair
QHash<QgsVectorLayer *, QHash<QgsFeatureId, QList<int> > >::iterator itX = toDeleteGrouped.begin();
for ( ; itX != toDeleteGrouped.end(); ++itX )
{
QgsVectorLayer *layer = itX.key();
QHash<QgsFeatureId, QList<int> > &featuresDict = itX.value();

QHash<QgsFeatureId, QList<int> >::iterator it2 = featuresDict.begin();
for ( ; it2 != featuresDict.end(); ++it2 )
{
QgsFeatureId fid = it2.key();
QList<int> &vertexIds = it2.value();
if ( vertexIds.count() >= 2 && layer->geometryType() == QgsWkbTypes::PolygonGeometry )
{
QSet<int> duplicateVertexIndices;
QgsGeometry geom = cachedGeometry( layer, fid );
for ( int i = 0; i < vertexIds.count(); ++i )
{
QgsVertexId vid;
geom.vertexIdFromVertexNr( vertexIds[i], vid );
int ringVertexCount = geom.geometry()->vertexCount( vid.part, vid.ring );
if ( vid.vertex == ringVertexCount - 1 )
{
// this is the last vertex of the ring - remove the first vertex from the list
duplicateVertexIndices << geom.vertexNrFromVertexId( QgsVertexId( vid.part, vid.ring, 0 ) );
}
}
// now delete the duplicities
Q_FOREACH ( int duplicateVertexIndex, duplicateVertexIndices )
vertexIds.removeOne( duplicateVertexIndex );
}
}
}

// main for cycle to delete all selected vertices
QHash<QgsVectorLayer *, QHash<QgsFeatureId, QList<int> > >::iterator it = toDeleteGrouped.begin();
for ( ; it != toDeleteGrouped.end(); ++it )
@@ -1528,6 +1601,11 @@ void QgsNodeTool::deleteVertex()
success = false;
}
}

if ( res == QgsVectorLayer::EmptyGeometry )
{
emit messageEmitted( tr( "Geometry has been cleared. Use the add part tool to set geometry for this feature." ) );
}
}

if ( success )
@@ -1545,7 +1623,7 @@ void QgsNodeTool::deleteVertex()
// pre-select next node for deletion if we are deleting just one node
if ( toDelete.count() == 1 )
{
const Vertex &vertex = toDelete[0];
const Vertex &vertex = *toDelete.constBegin();
QgsGeometry geom( cachedGeometryForVertex( vertex ) );
int vertexId = vertex.vertexId;

@@ -223,91 +223,6 @@ void QgsSelectedFeature::validationFinished()
sb->showMessage( tr( "Validation finished (%n error(s) found).", "number of geometry errors", mGeomErrorMarkers.size() ) );
}

void QgsSelectedFeature::deleteSelectedVertexes()
{
int nSelected = 0;
Q_FOREACH ( QgsVertexEntry *entry, mVertexMap )
{
if ( entry->isSelected() )
nSelected++;
}

if ( nSelected == 0 )
return;

bool topologicalEditing = QgsProject::instance()->topologicalEditing();
QMultiMap<double, QgsSnappingResult> currentResultList;

mVlayer->beginEditCommand( QObject::tr( "Deleted vertices" ) );

beginGeometryChange();

bool success = false;
QgsVectorLayer::EditResult res = QgsVectorLayer::Success;
for ( int i = mVertexMap.size() - 1; i > -1 && nSelected > 0; i-- )
{
if ( mVertexMap.at( i )->isSelected() )
{
if ( topologicalEditing )
{
// snap from current vertex
currentResultList.clear();
mVlayer->snapWithContext( mVertexMap.at( i )->pointV1(), ZERO_TOLERANCE, currentResultList, QgsSnappingResult::SnapToVertex );
}

// only last update should trigger the geometry update
// as vertex selection gets lost on the update
if ( --nSelected == 0 )
endGeometryChange();

if ( res != QgsVectorLayer::EmptyGeometry )
res = mVlayer->deleteVertex( mFeatureId, i );

if ( res != QgsVectorLayer::Success && res != QgsVectorLayer::EmptyGeometry )
{
success = false;
QgsDebugMsg( QString( "Deleting vertex %1 failed - resetting" ).arg( i ) );
break;
}

success = true;

if ( topologicalEditing )
{
QMultiMap<double, QgsSnappingResult>::iterator resultIt = currentResultList.begin();

for ( ; resultIt != currentResultList.end(); ++resultIt )
{
// move all other
if ( mFeatureId != resultIt.value().snappedAtGeometry )
mVlayer->deleteVertex( resultIt.value().snappedAtGeometry, resultIt.value().snappedVertexNr );
}
}

if ( res == QgsVectorLayer::EmptyGeometry )
{
//geometry has been cleared as a result of deleting vertices (e.g., not enough vertices left to leave a valid geometry),
//so nothing more to do
QgsGeometry empty;
geometryChanged( mFeatureId, empty );
break;
}
}
}

if ( nSelected > 0 )
endGeometryChange();

if ( success )
{
mVlayer->endEditCommand();
}
else
{
mVlayer->destroyEditCommand();
}
}

void QgsSelectedFeature::replaceVertexMap()
{
// delete old map
@@ -71,11 +71,6 @@ class QgsSelectedFeature: public QObject
*/
void deselectAllVertexes();

/**
* Deletes all selected vertexes
*/
void deleteSelectedVertexes();

/**
* Inverts selection of vertex with number
* \param vertexNr number of vertex which is to be inverted
@@ -61,6 +61,8 @@ class TestQgsNodeTool : public QObject
void testAddVertexAtEndpoint();
void testDeleteVertex();
void testMoveMultipleVertices();
void testMoveVertexTopo();
void testDeleteVertexTopo();

private:
QPoint mapToScreen( double mapX, double mapY )
@@ -472,5 +474,63 @@ void TestQgsNodeTool::testMoveMultipleVertices()
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
}

void TestQgsNodeTool::testMoveVertexTopo()
{
// test moving of vertices of two features at once

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

// connect linestring with polygon at point (2, 1)
mouseClick( 4, 1, Qt::LeftButton );
mouseClick( 2, 1, Qt::LeftButton );

// move shared node of linestring and polygon
mouseClick( 2, 1, Qt::LeftButton );
mouseClick( 3, 3, Qt::LeftButton );

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

QCOMPARE( mLayerLine->undoStack()->index(), 2 );
QCOMPARE( mLayerPolygon->undoStack()->index(), 3 ); // one more move of node from earlier
mLayerLine->undoStack()->undo();
mLayerPolygon->undoStack()->undo();
mLayerPolygon->undoStack()->undo();

QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );

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

void TestQgsNodeTool::testDeleteVertexTopo()
{
// test deletion of vertices with topological editing enabled

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

// connect linestring with polygon at point (2, 1)
mouseClick( 4, 1, Qt::LeftButton );
mouseClick( 2, 1, Qt::LeftButton );

// move shared node of linestring and polygon
mouseClick( 2, 1, Qt::LeftButton );
keyClick( Qt::Key_Delete );

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

QCOMPARE( mLayerLine->undoStack()->index(), 2 );
QCOMPARE( mLayerPolygon->undoStack()->index(), 3 ); // one more move of node from earlier
mLayerLine->undoStack()->undo();
mLayerPolygon->undoStack()->undo();
mLayerPolygon->undoStack()->undo();

QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );

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

QGSTEST_MAIN( TestQgsNodeTool )
#include "testqgsnodetool.moc"

0 comments on commit 832d2dd

Please sign in to comment.