Skip to content
Permalink
Browse files

Vertex tool delete ring / part fix

Don't also delete random vertices when a part or ring gets deleted

Fixes #35428
Fixes #25650
  • Loading branch information
uclaros committed Jun 1, 2020
1 parent 43e3e63 commit 9116b4c42b673e79e79a02d8be2d190dea039b59
Showing with 142 additions and 23 deletions.
  1. +34 −21 src/app/vertextool/qgsvertextool.cpp
  2. +108 −2 tests/src/app/testqgsvertextool.cpp
@@ -2240,37 +2240,50 @@ void QgsVertexTool::deleteVertex()

// 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 )
// also deselect vertices of parts or rings that will be automatically removed
QHash<QgsVectorLayer *, QHash<QgsFeatureId, QList<int> > >::iterator lIt = toDeleteGrouped.begin();
for ( ; lIt != toDeleteGrouped.end(); ++lIt )
{
QgsVectorLayer *layer = itX.key();
QHash<QgsFeatureId, QList<int> > &featuresDict = itX.value();
QgsVectorLayer *layer = lIt.key();
QHash<QgsFeatureId, QList<int> > &featuresDict = lIt.value();

QHash<QgsFeatureId, QList<int> >::iterator it2 = featuresDict.begin();
for ( ; it2 != featuresDict.end(); ++it2 )
QHash<QgsFeatureId, QList<int> >::iterator fIt = featuresDict.begin();
for ( ; fIt != featuresDict.end(); ++fIt )
{
QgsFeatureId fid = it2.key();
QList<int> &vertexIds = it2.value();
if ( vertexIds.count() >= 2 && layer->geometryType() == QgsWkbTypes::PolygonGeometry )
QgsFeatureId fid = fIt.key();
QList<int> &vertexIds = fIt.value();
if ( vertexIds.count() >= 2 && ( layer->geometryType() == QgsWkbTypes::PolygonGeometry ||
layer->geometryType() == QgsWkbTypes::LineGeometry ) )
{
QSet<int> duplicateVertexIndices;
QgsGeometry geom = cachedGeometry( layer, fid );
for ( int i = 0; i < vertexIds.count(); ++i )
std::sort( vertexIds.begin(), vertexIds.end(), std::greater<int>() );
const QgsGeometry geom = cachedGeometry( layer, fid );
const QgsAbstractGeometry *ag = geom.constGet();
QVector<QVector<int>> numberOfVertices;
for ( int p = 0 ; p < ag->partCount() ; ++p )
{
numberOfVertices.append( QVector<int>() );
for ( int r = 0 ; r < ag->ringCount( p ) ; ++r )
{
numberOfVertices[p].append( ag->vertexCount( p, r ) );
}
}
// polygonal rings with less than 4 vertices get deleted automatically
// linear parts with less than 2 vertices get deleted automatically
// let's keep that number and don't remove vertices beyond that point
const int minAllowedVertices = geom.type() == QgsWkbTypes::PolygonGeometry ? 4 : 2;
for ( int i = vertexIds.count() - 1; i >= 0 ; --i )
{
QgsVertexId vid;
if ( geom.vertexIdFromVertexNr( vertexIds[i], vid ) )
{
int ringVertexCount = geom.constGet()->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 ) );
}
// also don't try to delete the first vertex of a ring since we have already deleted the last
if ( numberOfVertices.at( vid.part ).at( vid.ring ) < minAllowedVertices ||
( 0 == vid.vertex && geom.type() == QgsWkbTypes::PolygonGeometry ) )
vertexIds.removeOne( vertexIds.at( i ) );
else
--numberOfVertices[vid.part][vid.ring];
}
}
// now delete the duplicities
for ( int duplicateVertexIndex : qgis::as_const( duplicateVertexIndices ) )
vertexIds.removeOne( duplicateVertexIndex );
}
}
}
@@ -145,16 +145,20 @@ class TestQgsVertexTool : public QObject
QgsAdvancedDigitizingDockWidget *mAdvancedDigitizingDockWidget = nullptr;
QgsVertexTool *mVertexTool = nullptr;
QgsVectorLayer *mLayerLine = nullptr;
QgsVectorLayer *mLayerMultiLine = nullptr;
QgsVectorLayer *mLayerPolygon = nullptr;
QgsVectorLayer *mLayerMultiPolygon = nullptr;
QgsVectorLayer *mLayerPoint = nullptr;
QgsVectorLayer *mLayerLineZ = nullptr;
QgsVectorLayer *mLayerCompoundCurve = nullptr;
QgsVectorLayer *mLayerLineReprojected = nullptr;
QgsFeatureId mFidLineZF1 = 0;
QgsFeatureId mFidLineZF2 = 0;
QgsFeatureId mFidLineF1 = 0;
QgsFeatureId mFidMultiLineF1 = 0;
QgsFeatureId mFidLineF13857 = 0;
QgsFeatureId mFidPolygonF1 = 0;
QgsFeatureId mFidMultiPolygonF1 = 0;
QgsFeatureId mFidPointF1 = 0;
QgsFeatureId mFidCompoundCurveF1 = 0;
QgsFeatureId mFidCompoundCurveF2 = 0;
@@ -186,23 +190,30 @@ void TestQgsVertexTool::initTestCase()
// make testing layers
mLayerLine = new QgsVectorLayer( QStringLiteral( "LineString?crs=EPSG:27700" ), QStringLiteral( "layer line" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerLine->isValid() );
mLayerMultiLine = new QgsVectorLayer( QStringLiteral( "MultiLineString?crs=EPSG:27700" ), QStringLiteral( "layer multiline" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerMultiLine->isValid() );
mLayerLineReprojected = new QgsVectorLayer( QStringLiteral( "LineString?crs=EPSG:3857" ), QStringLiteral( "layer line" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerLineReprojected->isValid() );
mLayerPolygon = new QgsVectorLayer( QStringLiteral( "Polygon?crs=EPSG:27700" ), QStringLiteral( "layer polygon" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerPolygon->isValid() );
mLayerMultiPolygon = new QgsVectorLayer( QStringLiteral( "MultiPolygon?crs=EPSG:27700" ), QStringLiteral( "layer multipolygon" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerMultiPolygon->isValid() );
mLayerPoint = new QgsVectorLayer( QStringLiteral( "Point?crs=EPSG:27700" ), QStringLiteral( "layer point" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerPoint->isValid() );
mLayerLineZ = new QgsVectorLayer( QStringLiteral( "LineStringZ?crs=EPSG:27700" ), QStringLiteral( "layer line" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerLineZ->isValid() );
mLayerCompoundCurve = new QgsVectorLayer( QStringLiteral( "CompoundCurve?crs=27700" ), QStringLiteral( "layer compound curve" ), QStringLiteral( "memory" ) );
QVERIFY( mLayerCompoundCurve->isValid() );
QgsProject::instance()->addMapLayers( QList<QgsMapLayer *>() << mLayerLine << mLayerPolygon << mLayerPoint << mLayerLineZ << mLayerCompoundCurve );
QgsProject::instance()->addMapLayers( QList<QgsMapLayer *>() << mLayerLine << mLayerMultiLine << mLayerPolygon << mLayerMultiPolygon << mLayerPoint << mLayerLineZ << mLayerCompoundCurve );

QgsPolylineXY line1;
line1 << QgsPointXY( 2, 1 ) << QgsPointXY( 1, 1 ) << QgsPointXY( 1, 3 );
QgsFeature lineF1;
lineF1.setGeometry( QgsGeometry::fromPolylineXY( line1 ) );

QgsFeature multiLineF1;
multiLineF1.setGeometry( QgsGeometry::fromWkt( "MultiLineString ((3 1, 3 2),(3 3, 3 4))" ) );

QgsCoordinateTransform ct( mLayerLine->crs(), mLayerLineReprojected->crs(), QgsCoordinateTransformContext() );
QgsGeometry line3857 = lineF1.geometry();
line3857.transform( ct );
@@ -216,6 +227,9 @@ void TestQgsVertexTool::initTestCase()
QgsFeature polygonF1;
polygonF1.setGeometry( QgsGeometry::fromPolygonXY( polygon1 ) );

QgsFeature multiPolygonF1;
multiPolygonF1.setGeometry( QgsGeometry::fromWkt( "MultiPolygon (((1 5, 2 5, 2 6.5, 2 8, 1 8, 1 6.5, 1 5),(1.25 5.5, 1.25 6, 1.75 6, 1.75 5.5, 1.25 5.5),(1.25 7, 1.75 7, 1.75 7.5, 1.25 7.5, 1.25 7)),((3 5, 3 6.5, 3 8, 4 8, 4 6.5, 4 5, 3 5),(3.25 5.5, 3.75 5.5, 3.75 6, 3.25 6, 3.25 5.5),(3.25 7, 3.75 7, 3.75 7.5, 3.25 7.5, 3.25 7)))" ) );

QgsFeature pointF1;
pointF1.setGeometry( QgsGeometry::fromPointXY( QgsPointXY( 2, 3 ) ) );

@@ -246,6 +260,11 @@ void TestQgsVertexTool::initTestCase()
mFidLineF1 = lineF1.id();
QCOMPARE( mLayerLine->featureCount(), ( long )1 );

mLayerMultiLine->startEditing();
mLayerMultiLine->addFeature( multiLineF1 );
mFidMultiLineF1 = multiLineF1.id();
QCOMPARE( mLayerMultiLine->featureCount(), ( long )1 );

mLayerLineReprojected->startEditing();
mLayerLineReprojected->addFeature( lineF13857 );
mFidLineF13857 = lineF13857.id();
@@ -256,6 +275,11 @@ void TestQgsVertexTool::initTestCase()
mFidPolygonF1 = polygonF1.id();
QCOMPARE( mLayerPolygon->featureCount(), ( long )1 );

mLayerMultiPolygon->startEditing();
mLayerMultiPolygon->addFeature( multiPolygonF1 );
mFidMultiPolygonF1 = multiPolygonF1.id();
QCOMPARE( mLayerMultiPolygon->featureCount(), ( long )1 );

mLayerPoint->startEditing();
mLayerPoint->addFeature( pointF1 );
mFidPointF1 = pointF1.id();
@@ -277,7 +301,9 @@ void TestQgsVertexTool::initTestCase()

// just one added feature in each undo stack
QCOMPARE( mLayerLine->undoStack()->index(), 1 );
QCOMPARE( mLayerMultiLine->undoStack()->index(), 1 );
QCOMPARE( mLayerPolygon->undoStack()->index(), 1 );
QCOMPARE( mLayerMultiPolygon->undoStack()->index(), 1 );
QCOMPARE( mLayerPoint->undoStack()->index(), 1 );
// except for layerLineZ
QCOMPARE( mLayerLineZ->undoStack()->index(), 2 );
@@ -291,14 +317,16 @@ void TestQgsVertexTool::initTestCase()
QCOMPARE( mCanvas->mapSettings().outputSize(), QSize( 512, 512 ) );
QCOMPARE( mCanvas->mapSettings().visibleExtent(), QgsRectangle( 0, 0, 8, 8 ) );

mCanvas->setLayers( QList<QgsMapLayer *>() << mLayerLine << mLayerLineReprojected << mLayerPolygon << mLayerPoint << mLayerLineZ << mLayerCompoundCurve );
mCanvas->setLayers( QList<QgsMapLayer *>() << mLayerLine << mLayerMultiLine << mLayerLineReprojected << mLayerPolygon << mLayerMultiPolygon << mLayerPoint << mLayerLineZ << mLayerCompoundCurve );

QgsMapCanvasSnappingUtils *snappingUtils = new QgsMapCanvasSnappingUtils( mCanvas, this );
mCanvas->setSnappingUtils( snappingUtils );

snappingUtils->locatorForLayer( mLayerLine )->init();
snappingUtils->locatorForLayer( mLayerMultiLine )->init();
snappingUtils->locatorForLayer( mLayerLineReprojected )->init();
snappingUtils->locatorForLayer( mLayerPolygon )->init();
snappingUtils->locatorForLayer( mLayerMultiPolygon )->init();
snappingUtils->locatorForLayer( mLayerPoint )->init();
snappingUtils->locatorForLayer( mLayerLineZ )->init();
snappingUtils->locatorForLayer( mLayerCompoundCurve )->init();
@@ -683,6 +711,84 @@ void TestQgsVertexTool::testDeleteVertex()

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

// delete multiline part by dragging

mousePress( 2.5, 0.5, Qt::LeftButton );
mouseMove( 3.5, 2.5 );
mouseRelease( 3.5, 2.5, Qt::LeftButton );
keyClick( Qt::Key_Delete );

QCOMPARE( mLayerMultiLine->undoStack()->index(), 2 );
QCOMPARE( mLayerMultiLine->getFeature( mFidMultiLineF1 ).geometry(), QgsGeometry::fromWkt( "MultiLineString ((3 3, 3 4))" ) );

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

// delete multiline part by dragging and leaving only one vertex to the part

mousePress( 2.5, 0.5, Qt::LeftButton );
mouseMove( 3.5, 1.5 );
mouseRelease( 3.5, 1.5, Qt::LeftButton );
keyClick( Qt::Key_Delete );

QCOMPARE( mLayerMultiLine->undoStack()->index(), 2 );
QCOMPARE( mLayerMultiLine->getFeature( mFidMultiLineF1 ).geometry(), QgsGeometry::fromWkt( "MultiLineString ((3 3, 3 4))" ) );

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

// delete inner ring by dragging

mousePress( 1.1, 5.1, Qt::LeftButton );
mouseMove( 1.8, 6.2 );
mouseRelease( 1.8, 6.2, Qt::LeftButton );
keyClick( Qt::Key_Delete );

QCOMPARE( mLayerMultiPolygon->undoStack()->index(), 2 );
QCOMPARE( mLayerMultiPolygon->getFeature( mFidMultiPolygonF1 ).geometry(), QgsGeometry::fromWkt( "MultiPolygon (((1 5, 2 5, 2 6.5, 2 8, 1 8, 1 6.5, 1 5),(1.25 7, 1.75 7, 1.75 7.5, 1.25 7.5, 1.25 7)),((3 5, 3 6.5, 3 8, 4 8, 4 6.5, 4 5, 3 5),(3.25 5.5, 3.75 5.5, 3.75 6, 3.25 6, 3.25 5.5),(3.25 7, 3.75 7, 3.75 7.5, 3.25 7.5, 3.25 7)))" ) );

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

// delete inner ring by dragging and leaving less than 4 vertices to the ring

mousePress( 1.1, 5.1, Qt::LeftButton );
mouseMove( 1.8, 5.7 );
mouseRelease( 1.8, 5.7, Qt::LeftButton );
keyClick( Qt::Key_Delete );

QCOMPARE( mLayerMultiPolygon->undoStack()->index(), 2 );
QCOMPARE( mLayerMultiPolygon->getFeature( mFidMultiPolygonF1 ).geometry(), QgsGeometry::fromWkt( "MultiPolygon (((1 5, 2 5, 2 6.5, 2 8, 1 8, 1 6.5, 1 5),(1.25 7, 1.75 7, 1.75 7.5, 1.25 7.5, 1.25 7)),((3 5, 3 6.5, 3 8, 4 8, 4 6.5, 4 5, 3 5),(3.25 5.5, 3.75 5.5, 3.75 6, 3.25 6, 3.25 5.5),(3.25 7, 3.75 7, 3.75 7.5, 3.25 7.5, 3.25 7)))" ) );

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

// delete part and rings by dragging

mousePress( 0.5, 4.5, Qt::LeftButton );
mouseMove( 2.5, 8.5 );
mouseRelease( 2.5, 8.5, Qt::LeftButton );
keyClick( Qt::Key_Delete );

QCOMPARE( mLayerMultiPolygon->undoStack()->index(), 2 );
QCOMPARE( mLayerMultiPolygon->getFeature( mFidMultiPolygonF1 ).geometry(), QgsGeometry::fromWkt( "MultiPolygon (((3 5, 3 6.5, 3 8, 4 8, 4 6.5, 4 5, 3 5),(3.25 5.5, 3.75 5.5, 3.75 6, 3.25 6, 3.25 5.5),(3.25 7, 3.75 7, 3.75 7.5, 3.25 7.5, 3.25 7)))" ) );

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

// combined delete rings from different parts by dragging

mousePress( 0.5, 4.5, Qt::LeftButton );
mouseMove( 4.5, 6.2 );
mouseRelease( 4.5, 6.2, Qt::LeftButton );
keyClick( Qt::Key_Delete );

QCOMPARE( mLayerMultiPolygon->undoStack()->index(), 2 );
QCOMPARE( mLayerMultiPolygon->getFeature( mFidMultiPolygonF1 ).geometry(), QgsGeometry::fromWkt( "MultiPolygon (((1 6.5, 2 6.5, 2 8, 1 8, 1 6.5),(1.25 7, 1.75 7, 1.75 7.5, 1.25 7.5, 1.25 7)),((4 6.5, 3 6.5, 3 8, 4 8, 4 6.5),(3.25 7, 3.75 7, 3.75 7.5, 3.25 7.5, 3.25 7)))" ) );

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

// no other unexpected changes happened
QCOMPARE( mLayerLine->undoStack()->index(), 1 );
QCOMPARE( mLayerPolygon->undoStack()->index(), 1 );

0 comments on commit 9116b4c

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