From 84d2505df496d6380cf3db565b735439fff3f625 Mon Sep 17 00:00:00 2001 From: Jacky Volpes Date: Thu, 30 May 2024 16:52:18 +0200 Subject: [PATCH] Fix vertex deletion issue on CompoundCurve Issue: when deleting a vertex exactly on the limit between two curves of a CompoundCurve, two vertices appeared deleted. In fact, one was deleted and another was moved to link with the previous or next curve. This "smart" move was problematic when the first or second curve was completely deleted after its vertex deletion. This fix always creates an intermediate LineString when such a vertex is deleted to simply handle every case and curve type. --- src/core/geometry/qgscompoundcurve.cpp | 53 +++++++++++-------- .../core/geometry/testqgscompoundcurve.cpp | 24 +++++++-- 2 files changed, 50 insertions(+), 27 deletions(-) diff --git a/src/core/geometry/qgscompoundcurve.cpp b/src/core/geometry/qgscompoundcurve.cpp index 724c610cd09..af793f5af26 100644 --- a/src/core/geometry/qgscompoundcurve.cpp +++ b/src/core/geometry/qgscompoundcurve.cpp @@ -825,6 +825,7 @@ bool QgsCompoundCurve::deleteVertex( QgsVertexId position ) removeCurve( curveId ); } } + // We are on a vertex that belongs to two curves else if ( curveIds.size() == 2 ) { const int nextCurveId = curveIds.at( 1 ).first; @@ -835,46 +836,51 @@ bool QgsCompoundCurve::deleteVertex( QgsVertexId position ) Q_ASSERT( subVertexId.vertex == curve->numPoints() - 1 ); Q_ASSERT( nextSubVertexId.vertex == 0 ); + // globals start and end points const QgsPoint startPoint = curve->startPoint(); const QgsPoint endPoint = nextCurve->endPoint(); - if ( QgsWkbTypes::flatType( curve->wkbType() ) == Qgis::WkbType::LineString && - QgsWkbTypes::flatType( nextCurve->wkbType() ) == Qgis::WkbType::CircularString && - nextCurve->numPoints() > 3 ) - { - QgsPoint intermediatePoint; - Qgis::VertexType type; - nextCurve->pointAt( 2, intermediatePoint, type ); - curve->moveVertex( QgsVertexId( 0, 0, curve->numPoints() - 1 ), intermediatePoint ); - } - else if ( !curve->deleteVertex( subVertexId ) ) + // delete the vertex on first curve + if ( !curve->deleteVertex( subVertexId ) ) { clearCache(); //bbox may have changed return false; } - if ( QgsWkbTypes::flatType( curve->wkbType() ) == Qgis::WkbType::CircularString && - curve->numPoints() > 0 && - QgsWkbTypes::flatType( nextCurve->wkbType() ) == Qgis::WkbType::LineString ) - { - QgsPoint intermediatePoint = curve->endPoint(); - nextCurve->moveVertex( QgsVertexId( 0, 0, 0 ), intermediatePoint ); - } - else if ( !nextCurve->deleteVertex( nextSubVertexId ) ) + + // delete the vertex on second curve + if ( !nextCurve->deleteVertex( nextSubVertexId ) ) { clearCache(); //bbox may have changed return false; } - if ( curve->numPoints() == 0 && - nextCurve->numPoints() != 0 ) + + // if first curve is now empty and second is not then + // create a LineString to link from the global start point to the + // new start of the second curve and delete the first curve + if ( curve->numPoints() == 0 && nextCurve->numPoints() != 0 ) { - nextCurve->moveVertex( QgsVertexId( 0, 0, 0 ), startPoint ); + QgsPoint startPointOfSecond = nextCurve->startPoint(); removeCurve( curveId ); + QgsLineString *line = new QgsLineString(); + line->insertVertex( QgsVertexId( 0, 0, 0 ), startPoint ); + line->insertVertex( QgsVertexId( 0, 0, 1 ), startPointOfSecond ); + mCurves.insert( curveId, line ); } + // else, if the first curve is not empty and the second is + // then create a LineString to link from the new end of the first curve to the + // global end point and delete the first curve else if ( curve->numPoints() != 0 && nextCurve->numPoints() == 0 ) { - curve->moveVertex( QgsVertexId( 0, 0, curve->numPoints() - 1 ), endPoint ); + QgsPoint endPointOfFirst = curve->endPoint(); removeCurve( nextCurveId ); + QgsLineString *line = new QgsLineString(); + line->insertVertex( QgsVertexId( 0, 0, 0 ), endPointOfFirst ); + line->insertVertex( QgsVertexId( 0, 0, 1 ), endPoint ); + mCurves.insert( nextCurveId, line ); } + // else, if both curves are empty then + // remove both curves and create a LineString to link + // the curves before and the curves after the whole geometry else if ( curve->numPoints() == 0 && nextCurve->numPoints() == 0 ) { @@ -885,6 +891,8 @@ bool QgsCompoundCurve::deleteVertex( QgsVertexId position ) line->insertVertex( QgsVertexId( 0, 0, 1 ), endPoint ); mCurves.insert( curveId, line ); } + // else, both curves still have vertices, create a LineString to link + // the curves if needed else { QgsPoint endPointOfFirst = curve->endPoint(); @@ -897,6 +905,7 @@ bool QgsCompoundCurve::deleteVertex( QgsVertexId position ) mCurves.insert( nextCurveId, line ); } } + condenseCurves(); // We merge consecutive LineStrings and CircularStrings } bool success = !curveIds.isEmpty(); diff --git a/tests/src/core/geometry/testqgscompoundcurve.cpp b/tests/src/core/geometry/testqgscompoundcurve.cpp index 9659863c2ea..25f44dcb69d 100644 --- a/tests/src/core/geometry/testqgscompoundcurve.cpp +++ b/tests/src/core/geometry/testqgscompoundcurve.cpp @@ -1488,7 +1488,7 @@ void TestQgsCompoundCurve::deleteVertex() cc.deleteVertex( QgsVertexId( 0, 0, 2 ) ); QCOMPARE( cc.numPoints(), 0 ); - // two lines + // two lines, small line first and long line second QgsLineString ls; ls.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 1, 2, 2, 3 ) << QgsPoint( Qgis::WkbType::PointZM, 11, 12, 4, 5 ) ); @@ -1506,19 +1506,20 @@ void TestQgsCompoundCurve::deleteVertex() const QgsLineString *lsPtr = dynamic_cast< const QgsLineString * >( cc.curveAt( 0 ) ); - QCOMPARE( lsPtr->numPoints(), 2 ); + QCOMPARE( lsPtr->numPoints(), 3 ); QCOMPARE( lsPtr->startPoint(), QgsPoint( Qgis::WkbType::PointZM, 1, 2, 2, 3 ) ); QCOMPARE( lsPtr->endPoint(), QgsPoint( Qgis::WkbType::PointZM, 31, 42, 4, 5 ) ); //add vertex at the end of linestring - QVERIFY( cc.insertVertex( QgsVertexId( 0, 0, 2 ), QgsPoint( Qgis::WkbType::PointZM, 35, 43, 4, 5 ) ) ); + QVERIFY( cc.insertVertex( QgsVertexId( 0, 0, 3 ), QgsPoint( Qgis::WkbType::PointZM, 35, 43, 4, 5 ) ) ); lsPtr = dynamic_cast< const QgsLineString * >( cc.curveAt( 0 ) ); - QCOMPARE( lsPtr->numPoints(), 3 ); + QCOMPARE( lsPtr->numPoints(), 4 ); QCOMPARE( lsPtr->startPoint(), QgsPoint( Qgis::WkbType::PointZM, 1, 2, 2, 3 ) ); QCOMPARE( lsPtr->endPoint(), QgsPoint( Qgis::WkbType::PointZM, 35, 43, 4, 5 ) ); + // two lines, long line first and small line second ls.setPoints( QgsPointSequence() << QgsPoint( Qgis::WkbType::PointZM, 1, 2, 2, 3 ) << QgsPoint( Qgis::WkbType::PointZM, 11, 12, 4, 5 ) << QgsPoint( Qgis::WkbType::PointZM, 21, 32, 4, 5 ) ); @@ -1535,9 +1536,22 @@ void TestQgsCompoundCurve::deleteVertex() lsPtr = dynamic_cast< const QgsLineString * >( cc.curveAt( 0 ) ); - QCOMPARE( lsPtr->numPoints(), 2 ); + QCOMPARE( lsPtr->numPoints(), 3 ); QCOMPARE( lsPtr->startPoint(), QgsPoint( Qgis::WkbType::PointZM, 1, 2, 2, 3 ) ); QCOMPARE( lsPtr->endPoint(), QgsPoint( Qgis::WkbType::PointZM, 31, 42, 4, 5 ) ); + + // small ("one-curve" i.e. 3 vertices total) CircularString followed by LineString + cc.clear(); + cs.setPoints( QgsPointSequence() << QgsPoint( 0, 0 ) << QgsPoint( 1, 1 ) << QgsPoint( 0, 2 ) ); + ls.setPoints( QgsPointSequence() << QgsPoint( 0, 2 ) << QgsPoint( 0, 3 ) << QgsPoint( 0, 4 ) ); + cc.addCurve( cs.clone() ); + cc.addCurve( ls.clone() ); + + QCOMPARE( cc.nCurves(), 2 ); + QCOMPARE( cc.numPoints(), 5 ); + QVERIFY( cc.deleteVertex( QgsVertexId( 0, 0, 2 ) ) ); + QCOMPARE( cc.nCurves(), 1 ); + QCOMPARE( cc.numPoints(), 3 ); } void TestQgsCompoundCurve::filterVertices()