From b2d4fc8803280a090914026f74b29b150e8f28b5 Mon Sep 17 00:00:00 2001 From: JuhoErvasti Date: Mon, 16 Dec 2024 13:22:33 +0200 Subject: [PATCH 1/2] Split features: always give largest geometry to original feature --- src/core/vector/qgsvectorlayereditutils.cpp | 16 +++++ tests/src/app/testqgsmaptoolsplitfeatures.cpp | 60 ++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp index 349065d00fc6..17df3e729fb3 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -447,6 +447,22 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::splitFeatures( const QgsC topologyTestPoints.append( featureTopologyTestPoints ); if ( splitFunctionReturn == Qgis::GeometryOperationResult::Success ) { + //find largest geometry and give that to the original feature + std::function size = mLayer->geometryType() == Qgis::GeometryType::Polygon ? &QgsGeometry::area : &QgsGeometry::length; + double featureGeomSize = size( featureGeom ); + + QVector::iterator largestNewFeature = std::max_element( newGeometries.begin(), newGeometries.end(), [ &size ]( const QgsGeometry & a, const QgsGeometry & b ) -> bool + { + return size( a ) < size( b ); + } ); + + if ( size( *largestNewFeature ) > featureGeomSize ) + { + QgsGeometry copy = *largestNewFeature; + *largestNewFeature = featureGeom; + featureGeom = copy; + } + //change this geometry mLayer->changeGeometry( feat.id(), featureGeom ); diff --git a/tests/src/app/testqgsmaptoolsplitfeatures.cpp b/tests/src/app/testqgsmaptoolsplitfeatures.cpp index 6b6ec31b4cf6..e3119ec22a56 100644 --- a/tests/src/app/testqgsmaptoolsplitfeatures.cpp +++ b/tests/src/app/testqgsmaptoolsplitfeatures.cpp @@ -44,6 +44,8 @@ class TestQgsMapToolSplitFeatures : public QObject void testSplitSomeOfSelectedLines(); // see https://github.com/qgis/QGIS/issues/29270 void testSplitPolygonSnapToSegment(); + void testLargestGeometryToOriginalFeaturePolygon(); + void testLargestGeometryToOriginalFeatureLine(); private: QPoint mapToPoint( double x, double y ); @@ -146,13 +148,17 @@ void TestQgsMapToolSplitFeatures::testSplitPolygon() { mCanvas->setCurrentLayer( mPolygonLayer ); + QSet oldFids = mUtils->existingFeatureIds(); + mUtils->mouseClick( 4, 11, Qt::LeftButton ); mUtils->mouseClick( 4, 3, Qt::LeftButton ); mUtils->mouseClick( 4, 3, Qt::RightButton ); + QgsFeatureId newFid = mUtils->newFeatureId( oldFids ); + QCOMPARE( mPolygonLayer->undoStack()->index(), 1 ); QCOMPARE( mPolygonLayer->featureCount(), 3 ); - QCOMPARE( mPolygonLayer->getFeature( 1 ).geometry().asWkt(), QStringLiteral( "Polygon Z ((4 10 24, 4 5 14, 0 5 10, 0 10 20, 4 10 24))" ) ); + QCOMPARE( mPolygonLayer->getFeature( newFid ).geometry().asWkt(), QStringLiteral( "Polygon Z ((4 10 24, 4 5 14, 0 5 10, 0 10 20, 4 10 24))" ) ); QCOMPARE( mPolygonLayer->getFeature( 2 ).geometry().asWkt(), QStringLiteral( "Polygon Z ((0 0 10, 0 5 20, 10 5 30, 0 0 10))" ) ); // no change to other layers @@ -165,13 +171,17 @@ void TestQgsMapToolSplitFeatures::testSplitPolygonTopologicalEditing() QgsProject::instance()->setTopologicalEditing( true ); mCanvas->setCurrentLayer( mPolygonLayer ); + QSet oldFids = mUtils->existingFeatureIds(); + mUtils->mouseClick( 4, 11, Qt::LeftButton ); mUtils->mouseClick( 4, 3, Qt::LeftButton ); mUtils->mouseClick( 4, 3, Qt::RightButton ); + QgsFeatureId newFid = mUtils->newFeatureId( oldFids ); + QCOMPARE( mPolygonLayer->undoStack()->index(), 1 ); QCOMPARE( mPolygonLayer->featureCount(), 3 ); - QCOMPARE( mPolygonLayer->getFeature( 1 ).geometry().asWkt(), QStringLiteral( "Polygon Z ((4 10 24, 4 5 14, 0 5 10, 0 10 20, 4 10 24))" ) ); + QCOMPARE( mPolygonLayer->getFeature( newFid ).geometry().asWkt(), QStringLiteral( "Polygon Z ((4 10 24, 4 5 14, 0 5 10, 0 10 20, 4 10 24))" ) ); QCOMPARE( mPolygonLayer->getFeature( 2 ).geometry().asWkt(), QStringLiteral( "Polygon Z ((0 0 10, 0 5 20, 4 5 14, 10 5 30, 0 0 10))" ) ); QCOMPARE( mMultiLineStringLayer->undoStack()->index(), 1 ); @@ -252,5 +262,51 @@ void TestQgsMapToolSplitFeatures::testSplitPolygonSnapToSegment() mCanvas->snappingUtils()->setConfig( oldCfg ); } +void TestQgsMapToolSplitFeatures::testLargestGeometryToOriginalFeaturePolygon() +{ + mCanvas->setCurrentLayer( mPolygonLayer ); + + QSet oldFids = mUtils->existingFeatureIds(); + + mUtils->mouseClick( 1, 11, Qt::LeftButton ); + mUtils->mouseClick( 1, 5, Qt::LeftButton ); + mUtils->mouseClick( 1, 5, Qt::RightButton ); + + QgsFeatureId newFid = mUtils->newFeatureId( oldFids ); + + QCOMPARE( mPolygonLayer->featureCount(), 3 ); + + QgsFeature oldFeat = mPolygonLayer->getFeature( 1 ); + QgsFeature newFeat = mPolygonLayer->getFeature( newFid ); + + //larger + QCOMPARE( oldFeat.geometry().asWkt(), QStringLiteral( "Polygon Z ((1 5 10, 1 10 21, 10 10 30, 10 5 20, 1 5 10))" ) ); + + //smaller + QCOMPARE( newFeat.geometry().asWkt(), QStringLiteral( "Polygon Z ((1 10 21, 1 5 10, 0 5 10, 0 10 20, 1 10 21))" ) ); +} + +void TestQgsMapToolSplitFeatures::testLargestGeometryToOriginalFeatureLine() +{ + mCanvas->setCurrentLayer( mMultiLineStringLayer ); + + QSet oldFids = mUtils->existingFeatureIds(); + + mUtils->mouseClick( 4, 1, Qt::LeftButton ); + mUtils->mouseClick( 4, -1, Qt::LeftButton ); + mUtils->mouseClick( 4, -1, Qt::RightButton ); + + QgsFeatureId newFid = mUtils->newFeatureId( oldFids ); + + QgsFeature oldFeat = mMultiLineStringLayer->getFeature( 1 ); + QgsFeature newFeat = mMultiLineStringLayer->getFeature( newFid ); + + //larger + QCOMPARE( oldFeat.geometry().asWkt(), QStringLiteral( "MultiLineString ((4 0, 10 0))" ) ); + + //smaller + QCOMPARE( newFeat.geometry().asWkt(), QStringLiteral( "MultiLineString ((0 0, 4 0))" ) ); +} + QGSTEST_MAIN( TestQgsMapToolSplitFeatures ) #include "testqgsmaptoolsplitfeatures.moc" From c9981ad10257c803e25bee1918749b4e0a2ff95a Mon Sep 17 00:00:00 2001 From: JuhoErvasti Date: Mon, 16 Dec 2024 19:51:06 +0200 Subject: [PATCH 2/2] Fix split policy test - This change was made to match the change in b2d4fc8803280a090914026f74b29b150e8f28b5 where the largest geometry is now inherited by the original feature when splitting features. - This caused the test to fail for the 'field_unset' and 'field_ratio' attributes since the largest feature with the largest 'field_ratio' attribute also had the original 'field_unset' value and the others had an unset attribute --- tests/src/python/test_qgsvectorlayereditutils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/python/test_qgsvectorlayereditutils.py b/tests/src/python/test_qgsvectorlayereditutils.py index 36b691b48f8c..10ffc2b6a04e 100644 --- a/tests/src/python/test_qgsvectorlayereditutils.py +++ b/tests/src/python/test_qgsvectorlayereditutils.py @@ -752,9 +752,9 @@ def test_split_policy_lines(self): self.assertCountEqual( attributes, [ - [None, 3301.0, 302.0, 303.0, 8.664000000000001, 305.0], + [None, 3301.0, 302.0, 303.0, 277.53878260869567, 305.0], + [None, 301, 302.0, QgsUnsetAttributeValue(), 8.664000000000001, 305.0], [None, 301, 302.0, QgsUnsetAttributeValue(), 17.797217391304347, 305.0], - [None, 301, 302.0, QgsUnsetAttributeValue(), 277.53878260869567, 305.0], ], )