From b09c2d113232ca1b4c5cc1e6be8cd5ad372f5019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Mon, 3 Oct 2022 15:16:33 +0200 Subject: [PATCH 1/9] QgsVectorLayer: add addMultiRing and use it in QgsMapToolAddRing The `QgsMapToolAddRing` adds a ring through the `addRing` method of `QgsVectorLayerEditUtils` called by this of `QgsVectorLayer`. The bug described in #23113 indicates that only one of the entities receives the ring addition when using the map tool. This is consistent with the documentation of the `addRing` methods. So, it is by design that the tool works like this. However, as stated in the ticket, it is best to add the ring to all entities. In order to avoid an api break, I added a new addMultiRing method that adds the ring on all entities. Fixes #23113 --- .../vector/qgsvectorlayer.sip.in | 31 ++++++++ .../vector/qgsvectorlayereditutils.sip.in | 12 ++++ src/app/qgsmaptooladdring.cpp | 2 +- src/core/vector/qgsvectorlayer.cpp | 38 ++++++++++ src/core/vector/qgsvectorlayer.h | 23 ++++++ src/core/vector/qgsvectorlayereditutils.cpp | 71 +++++++++++++------ src/core/vector/qgsvectorlayereditutils.h | 10 +++ 7 files changed, 163 insertions(+), 24 deletions(-) diff --git a/python/core/auto_generated/vector/qgsvectorlayer.sip.in b/python/core/auto_generated/vector/qgsvectorlayer.sip.in index be89cf3e2404..030d469e7bcb 100644 --- a/python/core/auto_generated/vector/qgsvectorlayer.sip.in +++ b/python/core/auto_generated/vector/qgsvectorlayer.sip.in @@ -1384,6 +1384,37 @@ Adds a ring to polygon/multipolygon features (takes ownership) changes can be discarded by calling :py:func:`~QgsVectorLayer.rollBack`. %End + Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring /Transfer/, QgsFeatureIds *featureIds = 0 ) /PyName=addMultiCurvedRing/; +%Docstring +Adds a ring to polygon/multipolygon features (takes ownership) + +:param ring: ring to add +:param featureIds: if specified, features ID for features ring was added to will be stored in this parameter + +:return: Qgis.GeometryOperationResult + + - Success + - LayerNotEditable + - AddRingNotInExistingFeature + - InvalidInputGeometryType + - AddRingNotClosed + - AddRingNotValid + - AddRingCrossesExistingRings + +.. note:: + + available in Python as addMultiCurvedRing + +.. note:: + + Calls to :py:func:`~QgsVectorLayer.addMultiRing` are only valid for layers in which edits have been enabled + by a call to :py:func:`~QgsVectorLayer.startEditing`. Changes made to features using this method are not committed + to the underlying data provider until a :py:func:`~QgsVectorLayer.commitChanges` call is made. Any uncommitted + changes can be discarded by calling :py:func:`~QgsVectorLayer.rollBack`. + +.. seealso:: :py:func:`addRing` +%End + Qgis::GeometryOperationResult addPart( const QList &ring ) /Deprecated/; %Docstring Adds a new part polygon to a multipart feature diff --git a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in index 17bd71e297e4..af0c2469baf4 100644 --- a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in +++ b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in @@ -84,6 +84,18 @@ Adds a ring to polygon/multipolygon features all intersecting features are tested and the ring is added to the first valid feature. :param modifiedFeatureId: if specified, feature ID for feature that ring was added to will be stored in this parameter +:return: OperationResult result code: success or reason of failure +%End + + Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds = 0 ); +%Docstring +Adds a ring to polygon/multipolygon features + +:param ring: ring to add +:param targetFeatureIds: if specified, only these features will be the candidates for adding a ring. Otherwise + all intersecting features are tested and the ring is added to all valid features. +:param modifiedFeatureIds: if specified, feature IDS for features that ring was added to will be stored in this parameter + :return: OperationResult result code: success or reason of failure %End diff --git a/src/app/qgsmaptooladdring.cpp b/src/app/qgsmaptooladdring.cpp index bbb39558973e..86796c5223ff 100644 --- a/src/app/qgsmaptooladdring.cpp +++ b/src/app/qgsmaptooladdring.cpp @@ -68,7 +68,7 @@ void QgsMapToolAddRing::polygonCaptured( const QgsCurvePolygon *polygon ) return; vlayer->beginEditCommand( tr( "Ring added" ) ); - const Qgis::GeometryOperationResult addRingReturnCode = vlayer->addRing( polygon->exteriorRing()->clone() ); + const Qgis::GeometryOperationResult addRingReturnCode = vlayer->addMultiRing( polygon->exteriorRing()->clone() ); QString errorMessage; switch ( addRingReturnCode ) { diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp index 593bc382ece3..9a40d8abd23f 100644 --- a/src/core/vector/qgsvectorlayer.cpp +++ b/src/core/vector/qgsvectorlayer.cpp @@ -1323,6 +1323,44 @@ Qgis::GeometryOperationResult QgsVectorLayer::addRing( QgsCurve *ring, QgsFeatur return result; } +Qgis::GeometryOperationResult QgsVectorLayer::addMultiRing( QgsCurve *ring, QgsFeatureIds *featureIds ) +{ + if ( !isValid() || !mEditBuffer || !mDataProvider ) + { + delete ring; + return Qgis::GeometryOperationResult::LayerNotEditable; + } + + if ( !ring ) + { + return Qgis::GeometryOperationResult::InvalidInputGeometryType; + } + + if ( !ring->isClosed() ) + { + delete ring; + return Qgis::GeometryOperationResult::AddRingNotClosed; + } + + QgsVectorLayerEditUtils utils( this ); + Qgis::GeometryOperationResult result = Qgis::GeometryOperationResult::AddRingNotInExistingFeature; + + //first try with selected features + if ( !mSelectedFeatureIds.isEmpty() ) + { + result = utils.addMultiRing( static_cast< QgsCurve * >( ring->clone() ), mSelectedFeatureIds, featureIds ); + } + + if ( result != Qgis::GeometryOperationResult::Success ) + { + //try with all intersecting features + result = utils.addMultiRing( static_cast< QgsCurve * >( ring->clone() ), QgsFeatureIds(), featureIds ); + } + + delete ring; + return result; +} + Qgis::GeometryOperationResult QgsVectorLayer::addPart( const QList &points ) { QgsPointSequence pts; diff --git a/src/core/vector/qgsvectorlayer.h b/src/core/vector/qgsvectorlayer.h index b60315b62369..2120ddc2fab8 100644 --- a/src/core/vector/qgsvectorlayer.h +++ b/src/core/vector/qgsvectorlayer.h @@ -1391,6 +1391,29 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte */ Q_INVOKABLE Qgis::GeometryOperationResult addRing( QgsCurve *ring SIP_TRANSFER, QgsFeatureId *featureId = nullptr ) SIP_PYNAME( addCurvedRing ); + /** + * Adds a ring to polygon/multipolygon features (takes ownership) + * \param ring ring to add + * \param featureIds if specified, features ID for features ring was added to will be stored in this parameter + * \returns Qgis::GeometryOperationResult + * + * - Success + * - LayerNotEditable + * - AddRingNotInExistingFeature + * - InvalidInputGeometryType + * - AddRingNotClosed + * - AddRingNotValid + * - AddRingCrossesExistingRings + * + * \note available in Python as addMultiCurvedRing + * \note Calls to addMultiRing() are only valid for layers in which edits have been enabled + * by a call to startEditing(). Changes made to features using this method are not committed + * to the underlying data provider until a commitChanges() call is made. Any uncommitted + * changes can be discarded by calling rollBack(). + * \see addRing + */ + Q_INVOKABLE Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring SIP_TRANSFER, QgsFeatureIds *featureIds = nullptr ) SIP_PYNAME( addMultiCurvedRing ); + /** * Adds a new part polygon to a multipart feature * \returns Qgis::GeometryOperationResult diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp index a532eeba75d2..9d1cd0b26927 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -118,25 +118,12 @@ Qgis::VectorEditResult QgsVectorLayerEditUtils::deleteVertex( QgsFeatureId featu return !geometry.isNull() ? Qgis::VectorEditResult::Success : Qgis::VectorEditResult::EmptyGeometry; } -Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QVector &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) -{ - QgsPointSequence l; - for ( QVector::const_iterator it = ring.constBegin(); it != ring.constEnd(); ++it ) - { - l << QgsPoint( *it ); - } - return addRing( l, targetFeatureIds, modifiedFeatureId ); -} -Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QgsPointSequence &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) +static +Qgis::GeometryOperationResult _addRing( QgsVectorLayer *layer, QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true ) { - QgsLineString *ringLine = new QgsLineString( ring ); - return addRing( ringLine, targetFeatureIds, modifiedFeatureId ); -} -Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) -{ - if ( !mLayer->isSpatial() ) + if ( !layer || !layer->isSpatial() ) { delete ring; return Qgis::GeometryOperationResult::AddRingNotInExistingFeature; @@ -149,13 +136,13 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, if ( !targetFeatureIds.isEmpty() ) { //check only specified features - fit = mLayer->getFeatures( QgsFeatureRequest().setFilterFids( targetFeatureIds ) ); + fit = layer->getFeatures( QgsFeatureRequest().setFilterFids( targetFeatureIds ) ); } else { //check all intersecting features QgsRectangle bBox = ring->boundingBox(); - fit = mLayer->getFeatures( QgsFeatureRequest().setFilterRect( bBox ).setFlags( QgsFeatureRequest::ExactIntersect ) ); + fit = layer->getFeatures( QgsFeatureRequest().setFilterRect( bBox ).setFlags( QgsFeatureRequest::ExactIntersect ) ); } //find first valid feature we can add the ring to @@ -170,18 +157,56 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, addRingReturnCode = g.addRing( static_cast< QgsCurve * >( ring->clone() ) ); if ( addRingReturnCode == Qgis::GeometryOperationResult::Success ) { - mLayer->changeGeometry( f.id(), g ); - if ( modifiedFeatureId ) - *modifiedFeatureId = f.id(); + layer->changeGeometry( f.id(), g ); + if ( modifiedFeatureIds ) + { + modifiedFeatureIds->insert( f.id() ); + if ( firstOne ) + { + break; + } + } - //setModified( true, true ); - break; } } delete ring; return addRingReturnCode; } +Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QVector &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) +{ + QgsPointSequence l; + for ( QVector::const_iterator it = ring.constBegin(); it != ring.constEnd(); ++it ) + { + l << QgsPoint( *it ); + } + return addRing( l, targetFeatureIds, modifiedFeatureId ); +} + +Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QgsPointSequence &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) +{ + QgsLineString *ringLine = new QgsLineString( ring ); + return addRing( ringLine, targetFeatureIds, modifiedFeatureId ); +} + +Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) +{ + if ( modifiedFeatureId ) + { + QgsFeatureIds *modifiedFeatureIds = new QgsFeatureIds; + Qgis::GeometryOperationResult result = _addRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, true ); + *modifiedFeatureId = *modifiedFeatureIds->begin(); + return result; + } + return _addRing( mLayer, ring, targetFeatureIds, nullptr, true ); +} + +Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds ) +{ + return _addRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, false ); +} + + Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addPart( const QVector &points, QgsFeatureId featureId ) { diff --git a/src/core/vector/qgsvectorlayereditutils.h b/src/core/vector/qgsvectorlayereditutils.h index 2cfa26dd52a8..797698a6692d 100644 --- a/src/core/vector/qgsvectorlayereditutils.h +++ b/src/core/vector/qgsvectorlayereditutils.h @@ -91,6 +91,16 @@ class CORE_EXPORT QgsVectorLayerEditUtils */ Qgis::GeometryOperationResult addRing( const QgsPointSequence &ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureId *modifiedFeatureId = nullptr ); + /** + * Adds a ring to polygon/multipolygon features + * \param ring ring to add + * \param targetFeatureIds if specified, only these features will be the candidates for adding a ring. Otherwise + * all intersecting features are tested and the ring is added to all valid features. + * \param modifiedFeatureIds if specified, feature IDS for features that ring was added to will be stored in this parameter + * \return OperationResult result code: success or reason of failure + */ + Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds = nullptr ); + /** * Adds a ring to polygon/multipolygon features * \param ring ring to add From 6ebb15db9c937b73b46727fe98e31903ffd360a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Thu, 13 Oct 2022 07:11:00 +0200 Subject: [PATCH 2/9] qgsvectorlayereditutils.cpp: replace _addRing by staticAddRing, pet clang-tidy --- src/core/vector/qgsvectorlayereditutils.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp index 9d1cd0b26927..abd5a13a84ea 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -120,7 +120,7 @@ Qgis::VectorEditResult QgsVectorLayerEditUtils::deleteVertex( QgsFeatureId featu static -Qgis::GeometryOperationResult _addRing( QgsVectorLayer *layer, QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true ) +Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true ) { if ( !layer || !layer->isSpatial() ) @@ -194,16 +194,16 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, if ( modifiedFeatureId ) { QgsFeatureIds *modifiedFeatureIds = new QgsFeatureIds; - Qgis::GeometryOperationResult result = _addRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, true ); + Qgis::GeometryOperationResult result = staticAddRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, true ); *modifiedFeatureId = *modifiedFeatureIds->begin(); return result; } - return _addRing( mLayer, ring, targetFeatureIds, nullptr, true ); + return staticAddRing( mLayer, ring, targetFeatureIds, nullptr, true ); } Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds ) { - return _addRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, false ); + return staticAddRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, false ); } From 992e2bf1dfec13291f13b59a266dd54b41e5d05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Mon, 17 Oct 2022 10:40:13 +0200 Subject: [PATCH 3/9] Remove addMultiRing method in QgsVectorLayer. QgsMapToolAddRing can use the one from QgsVectorLayerEditUtils. All checks are made inside QgsVectorLayerEditUtils. --- .../vector/qgsvectorlayer.sip.in | 31 --------------- src/app/qgsmaptooladdring.cpp | 4 +- src/core/vector/qgsvectorlayer.cpp | 38 ------------------- src/core/vector/qgsvectorlayer.h | 23 ----------- src/core/vector/qgsvectorlayereditutils.cpp | 18 +++++++++ 5 files changed, 21 insertions(+), 93 deletions(-) diff --git a/python/core/auto_generated/vector/qgsvectorlayer.sip.in b/python/core/auto_generated/vector/qgsvectorlayer.sip.in index 030d469e7bcb..be89cf3e2404 100644 --- a/python/core/auto_generated/vector/qgsvectorlayer.sip.in +++ b/python/core/auto_generated/vector/qgsvectorlayer.sip.in @@ -1384,37 +1384,6 @@ Adds a ring to polygon/multipolygon features (takes ownership) changes can be discarded by calling :py:func:`~QgsVectorLayer.rollBack`. %End - Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring /Transfer/, QgsFeatureIds *featureIds = 0 ) /PyName=addMultiCurvedRing/; -%Docstring -Adds a ring to polygon/multipolygon features (takes ownership) - -:param ring: ring to add -:param featureIds: if specified, features ID for features ring was added to will be stored in this parameter - -:return: Qgis.GeometryOperationResult - - - Success - - LayerNotEditable - - AddRingNotInExistingFeature - - InvalidInputGeometryType - - AddRingNotClosed - - AddRingNotValid - - AddRingCrossesExistingRings - -.. note:: - - available in Python as addMultiCurvedRing - -.. note:: - - Calls to :py:func:`~QgsVectorLayer.addMultiRing` are only valid for layers in which edits have been enabled - by a call to :py:func:`~QgsVectorLayer.startEditing`. Changes made to features using this method are not committed - to the underlying data provider until a :py:func:`~QgsVectorLayer.commitChanges` call is made. Any uncommitted - changes can be discarded by calling :py:func:`~QgsVectorLayer.rollBack`. - -.. seealso:: :py:func:`addRing` -%End - Qgis::GeometryOperationResult addPart( const QList &ring ) /Deprecated/; %Docstring Adds a new part polygon to a multipart feature diff --git a/src/app/qgsmaptooladdring.cpp b/src/app/qgsmaptooladdring.cpp index 86796c5223ff..d971c5bcabd6 100644 --- a/src/app/qgsmaptooladdring.cpp +++ b/src/app/qgsmaptooladdring.cpp @@ -21,6 +21,7 @@ #include "qgsproject.h" #include "qgscurvepolygon.h" #include "qgsvectorlayer.h" +#include "qgsvectorlayereditutils.h" #include "qgisapp.h" #include "qgsmapmouseevent.h" @@ -68,7 +69,8 @@ void QgsMapToolAddRing::polygonCaptured( const QgsCurvePolygon *polygon ) return; vlayer->beginEditCommand( tr( "Ring added" ) ); - const Qgis::GeometryOperationResult addRingReturnCode = vlayer->addMultiRing( polygon->exteriorRing()->clone() ); + QgsVectorLayerEditUtils utils( vlayer ); + const Qgis::GeometryOperationResult addRingReturnCode = utils.addMultiRing( polygon->exteriorRing()->clone() ); QString errorMessage; switch ( addRingReturnCode ) { diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp index 9a40d8abd23f..593bc382ece3 100644 --- a/src/core/vector/qgsvectorlayer.cpp +++ b/src/core/vector/qgsvectorlayer.cpp @@ -1323,44 +1323,6 @@ Qgis::GeometryOperationResult QgsVectorLayer::addRing( QgsCurve *ring, QgsFeatur return result; } -Qgis::GeometryOperationResult QgsVectorLayer::addMultiRing( QgsCurve *ring, QgsFeatureIds *featureIds ) -{ - if ( !isValid() || !mEditBuffer || !mDataProvider ) - { - delete ring; - return Qgis::GeometryOperationResult::LayerNotEditable; - } - - if ( !ring ) - { - return Qgis::GeometryOperationResult::InvalidInputGeometryType; - } - - if ( !ring->isClosed() ) - { - delete ring; - return Qgis::GeometryOperationResult::AddRingNotClosed; - } - - QgsVectorLayerEditUtils utils( this ); - Qgis::GeometryOperationResult result = Qgis::GeometryOperationResult::AddRingNotInExistingFeature; - - //first try with selected features - if ( !mSelectedFeatureIds.isEmpty() ) - { - result = utils.addMultiRing( static_cast< QgsCurve * >( ring->clone() ), mSelectedFeatureIds, featureIds ); - } - - if ( result != Qgis::GeometryOperationResult::Success ) - { - //try with all intersecting features - result = utils.addMultiRing( static_cast< QgsCurve * >( ring->clone() ), QgsFeatureIds(), featureIds ); - } - - delete ring; - return result; -} - Qgis::GeometryOperationResult QgsVectorLayer::addPart( const QList &points ) { QgsPointSequence pts; diff --git a/src/core/vector/qgsvectorlayer.h b/src/core/vector/qgsvectorlayer.h index 2120ddc2fab8..b60315b62369 100644 --- a/src/core/vector/qgsvectorlayer.h +++ b/src/core/vector/qgsvectorlayer.h @@ -1391,29 +1391,6 @@ class CORE_EXPORT QgsVectorLayer : public QgsMapLayer, public QgsExpressionConte */ Q_INVOKABLE Qgis::GeometryOperationResult addRing( QgsCurve *ring SIP_TRANSFER, QgsFeatureId *featureId = nullptr ) SIP_PYNAME( addCurvedRing ); - /** - * Adds a ring to polygon/multipolygon features (takes ownership) - * \param ring ring to add - * \param featureIds if specified, features ID for features ring was added to will be stored in this parameter - * \returns Qgis::GeometryOperationResult - * - * - Success - * - LayerNotEditable - * - AddRingNotInExistingFeature - * - InvalidInputGeometryType - * - AddRingNotClosed - * - AddRingNotValid - * - AddRingCrossesExistingRings - * - * \note available in Python as addMultiCurvedRing - * \note Calls to addMultiRing() are only valid for layers in which edits have been enabled - * by a call to startEditing(). Changes made to features using this method are not committed - * to the underlying data provider until a commitChanges() call is made. Any uncommitted - * changes can be discarded by calling rollBack(). - * \see addRing - */ - Q_INVOKABLE Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring SIP_TRANSFER, QgsFeatureIds *featureIds = nullptr ) SIP_PYNAME( addMultiCurvedRing ); - /** * Adds a new part polygon to a multipart feature * \returns Qgis::GeometryOperationResult diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp index abd5a13a84ea..a12fdff5ee58 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -129,6 +129,23 @@ Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, QgsCurve *ri return Qgis::GeometryOperationResult::AddRingNotInExistingFeature; } + if ( !ring ) + { + return Qgis::GeometryOperationResult::InvalidInputGeometryType; + } + + if ( !ring->isClosed() ) + { + delete ring; + return Qgis::GeometryOperationResult::AddRingNotClosed; + } + + if ( !layer->isValid() || !layer->editBuffer() || !layer->dataProvider() ) + { + delete ring; + return Qgis::GeometryOperationResult::LayerNotEditable; + } + Qgis::GeometryOperationResult addRingReturnCode = Qgis::GeometryOperationResult::AddRingNotInExistingFeature; //default: return code for 'ring not inserted' QgsFeature f; @@ -203,6 +220,7 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds ) { + return staticAddRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, false ); } From 5d7a0253040243f85c5f6e17b6bf40dea758c69c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Mon, 17 Oct 2022 10:44:48 +0200 Subject: [PATCH 4/9] qgsvectorlayereditutils.h: add \since tag --- .../core/auto_generated/vector/qgsvectorlayereditutils.sip.in | 2 ++ src/core/vector/qgsvectorlayereditutils.h | 1 + 2 files changed, 3 insertions(+) diff --git a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in index af0c2469baf4..2fb416199c7a 100644 --- a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in +++ b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in @@ -97,6 +97,8 @@ Adds a ring to polygon/multipolygon features :param modifiedFeatureIds: if specified, feature IDS for features that ring was added to will be stored in this parameter :return: OperationResult result code: success or reason of failure + +.. versionadded:: 3.28 %End Qgis::GeometryOperationResult addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureId *modifiedFeatureId = 0 ) /PyName=addCurvedRing/; diff --git a/src/core/vector/qgsvectorlayereditutils.h b/src/core/vector/qgsvectorlayereditutils.h index 797698a6692d..99ef62c88996 100644 --- a/src/core/vector/qgsvectorlayereditutils.h +++ b/src/core/vector/qgsvectorlayereditutils.h @@ -98,6 +98,7 @@ class CORE_EXPORT QgsVectorLayerEditUtils * all intersecting features are tested and the ring is added to all valid features. * \param modifiedFeatureIds if specified, feature IDS for features that ring was added to will be stored in this parameter * \return OperationResult result code: success or reason of failure + * \since QGIS 3.28 */ Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds = nullptr ); From 6a1222f187f68e250139cbf44add069ae4077cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Tue, 18 Oct 2022 07:17:45 +0200 Subject: [PATCH 5/9] apply nyall's review: use unique_ptr and sip things --- .../vector/qgsvectorlayereditutils.sip.in | 8 ++++---- src/core/vector/qgsvectorlayereditutils.cpp | 15 +++++++-------- src/core/vector/qgsvectorlayereditutils.h | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in index 2fb416199c7a..447750e48852 100644 --- a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in +++ b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in @@ -87,16 +87,16 @@ Adds a ring to polygon/multipolygon features :return: OperationResult result code: success or reason of failure %End - Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds = 0 ); + Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring /Transfer/, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds /Out/ = 0 ); %Docstring Adds a ring to polygon/multipolygon features -:param ring: ring to add +:param ring: ring to add (ownership is transferred) :param targetFeatureIds: if specified, only these features will be the candidates for adding a ring. Otherwise all intersecting features are tested and the ring is added to all valid features. -:param modifiedFeatureIds: if specified, feature IDS for features that ring was added to will be stored in this parameter -:return: OperationResult result code: success or reason of failure +:return: - OperationResult result code: success or reason of failure + - modifiedFeatureIds: feature IDS for features that ring was added to will be stored in this parameter .. versionadded:: 3.28 %End diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp index a12fdff5ee58..afc8a176bacc 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -120,12 +120,11 @@ Qgis::VectorEditResult QgsVectorLayerEditUtils::deleteVertex( QgsFeatureId featu static -Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true ) +Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, std::unique_ptr< QgsCurve > &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true ) { if ( !layer || !layer->isSpatial() ) { - delete ring; return Qgis::GeometryOperationResult::AddRingNotInExistingFeature; } @@ -136,13 +135,11 @@ Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, QgsCurve *ri if ( !ring->isClosed() ) { - delete ring; return Qgis::GeometryOperationResult::AddRingNotClosed; } if ( !layer->isValid() || !layer->editBuffer() || !layer->dataProvider() ) { - delete ring; return Qgis::GeometryOperationResult::LayerNotEditable; } @@ -187,9 +184,9 @@ Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, QgsCurve *ri } } - delete ring; return addRingReturnCode; } + Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QVector &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) { QgsPointSequence l; @@ -208,20 +205,22 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QgsPointSe Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) { + std::unique_ptr uniquePtrRing( ring ); if ( modifiedFeatureId ) { QgsFeatureIds *modifiedFeatureIds = new QgsFeatureIds; - Qgis::GeometryOperationResult result = staticAddRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, true ); + Qgis::GeometryOperationResult result = staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, modifiedFeatureIds, true ); *modifiedFeatureId = *modifiedFeatureIds->begin(); return result; } - return staticAddRing( mLayer, ring, targetFeatureIds, nullptr, true ); + return staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, nullptr, true ); } Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds ) { - return staticAddRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, false ); + std::unique_ptr uniquePtrRing( ring ); + return staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, modifiedFeatureIds, false ); } diff --git a/src/core/vector/qgsvectorlayereditutils.h b/src/core/vector/qgsvectorlayereditutils.h index 99ef62c88996..565498cf4e25 100644 --- a/src/core/vector/qgsvectorlayereditutils.h +++ b/src/core/vector/qgsvectorlayereditutils.h @@ -93,14 +93,14 @@ class CORE_EXPORT QgsVectorLayerEditUtils /** * Adds a ring to polygon/multipolygon features - * \param ring ring to add + * \param ring ring to add (ownership is transferred) * \param targetFeatureIds if specified, only these features will be the candidates for adding a ring. Otherwise * all intersecting features are tested and the ring is added to all valid features. * \param modifiedFeatureIds if specified, feature IDS for features that ring was added to will be stored in this parameter * \return OperationResult result code: success or reason of failure * \since QGIS 3.28 */ - Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds = nullptr ); + Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring SIP_TRANSFER, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds SIP_OUT = nullptr ); /** * Adds a ring to polygon/multipolygon features From cc07d1e7772e868779c76f947b6fccfadb6a198a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Tue, 18 Oct 2022 10:24:59 +0200 Subject: [PATCH 6/9] QgsVectorLayerEditUtils: Init tests --- tests/src/python/CMakeLists.txt | 1 + .../python/test_qgsvectorlayereditutils.py | 200 ++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 tests/src/python/test_qgsvectorlayereditutils.py diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt index 973aebc65120..1c17bc6c81ee 100644 --- a/tests/src/python/CMakeLists.txt +++ b/tests/src/python/CMakeLists.txt @@ -384,6 +384,7 @@ ADD_PYTHON_TEST(PyQgsVectorLayerFeatureCounter test_qgsvectorlayerfeaturecounter ADD_PYTHON_TEST(PyQgsVectorLayerCache test_qgsvectorlayercache.py) ADD_PYTHON_TEST(PyQgsVectorLayerEditBuffer test_qgsvectorlayereditbuffer.py) ADD_PYTHON_TEST(PyQgsVectorLayerEditBufferGroup test_qgsvectorlayereditbuffergroup.py) +ADD_PYTHON_TEST(PyQgsVectorLayerEditUtils test_qgsvectorlayereditutils.py) ADD_PYTHON_TEST(PyQgsVectorLayerNamedStyle test_qgsvectorlayer_namedstyle.py) ADD_PYTHON_TEST(PyQgsVectorLayerProfileGenerator test_qgsvectorlayerprofilegenerator.py) ADD_PYTHON_TEST(PyQgsVectorLayerRenderer test_qgsvectorlayerrenderer.py) diff --git a/tests/src/python/test_qgsvectorlayereditutils.py b/tests/src/python/test_qgsvectorlayereditutils.py new file mode 100644 index 000000000000..6fa6d575ea20 --- /dev/null +++ b/tests/src/python/test_qgsvectorlayereditutils.py @@ -0,0 +1,200 @@ +# -*- coding: utf-8 -*- +"""QGIS Unit tests for QgsVectorLayerEditUtils. + +From build dir, run: +ctest -R PyQgsVectorLayerEditUtils -V + +.. note:: This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. +""" +__author__ = 'Loïc Bartoletti' +__date__ = '18/10/2022' +__copyright__ = 'Copyright 2022, The QGIS Project' + +import qgis # NOQA + +from qgis.PyQt.QtCore import ( + QDate, + QDateTime, + QVariant, + Qt, + QDateTime, + QDate, + QTime, + QTimer, + QTemporaryDir, +) + +from qgis.core import (Qgis, + QgsFeature, + QgsGeometry, + QgsLineString, + QgsPolygon, + QgsPoint, + QgsPointXY, + QgsVectorLayer, + QgsVectorLayerTools, + QgsVectorLayerEditUtils) + + +from qgis.testing import start_app, unittest + +start_app() + + +def createEmptyPolygonLayer(): + layer = QgsVectorLayer("Polygon", + "polygon", "memory") + assert layer.isValid() + return layer + + +class TestQgsVectorLayerEditBuffer(unittest.TestCase): + + def testAddRing(self): + # test adding ring to a vector layer + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f = QgsFeature(layer.fields(), 1) + f.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + assert pr.addFeatures([f]) + assert layer.featureCount() == 1 + + vle = QgsVectorLayerEditUtils(layer) + result = vle.addRing([QgsPointXY(1, 1), QgsPointXY(1, 2), QgsPointXY(2, 2), QgsPointXY(2, 1), QgsPointXY(1, 1)]) + assert Qgis.GeometryOperationResult.Success == result[0] + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(1 1, 1 2, 2 2, 2 1, 1 1))" + + def testAddRingOutside(self): + # test trying to add ring outside the feature's extent + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f = QgsFeature(layer.fields(), 1) + f.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + assert pr.addFeatures([f]) + assert layer.featureCount() == 1 + + vle = QgsVectorLayerEditUtils(layer) + result = vle.addRing([QgsPointXY(-1, -1), QgsPointXY(-1, -2), QgsPointXY(-2, -2), QgsPointXY(-2, -1), QgsPointXY(-1, -1)]) + assert Qgis.GeometryOperationResult.AddRingNotInExistingFeature == result[0] + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" + + def testAddRingOverlappedFeatures(self): + # test adding ring on multi features + # the ring will be added only to the first one + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f1 = QgsFeature(layer.fields(), 1) + f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + f2 = QgsFeature(layer.fields(), 1) + f2.setGeometry(QgsGeometry.fromWkt('POLYGON((2 2, 6 2, 6 6, 2 6, 2 2))')) + assert pr.addFeatures([f1, f2]) + assert layer.featureCount() == 2 + + vle = QgsVectorLayerEditUtils(layer) + result = vle.addRing([QgsPointXY(3, 3), QgsPointXY(3, 4), QgsPointXY(4, 4), QgsPointXY(4, 3), QgsPointXY(3, 3)]) + assert Qgis.GeometryOperationResult.Success == result[0] + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(3 3, 3 4, 4 4, 4 3, 3 3))" + assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" + + def testAddMultiRingNotEditable(self): + # test adding ring on multi features + # layer not editable + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f1 = QgsFeature(layer.fields(), 1) + f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + f2 = QgsFeature(layer.fields(), 1) + f2.setGeometry(QgsGeometry.fromWkt('POLYGON((2 2, 6 2, 6 6, 2 6, 2 2))')) + assert pr.addFeatures([f1, f2]) + assert layer.featureCount() == 2 + layer.commitChanges() + + vle = QgsVectorLayerEditUtils(layer) + assert Qgis.GeometryOperationResult.LayerNotEditable == vle.addMultiRing(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" + assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" + + def testAddMultiRingNotClosedRing(self): + # test adding ring on multi features + # Not closed ring + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f1 = QgsFeature(layer.fields(), 1) + f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + f2 = QgsFeature(layer.fields(), 1) + f2.setGeometry(QgsGeometry.fromWkt('POLYGON((2 2, 6 2, 6 6, 2 6, 2 2))')) + assert pr.addFeatures([f1, f2]) + assert layer.featureCount() == 2 + + vle = QgsVectorLayerEditUtils(layer) + assert Qgis.GeometryOperationResult.AddRingNotClosed == vle.addMultiRing(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3)])) + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" + assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" + + def testAddMultiRingOutside(self): + # test adding ring on multi features + # Not In Existing Feature + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f1 = QgsFeature(layer.fields(), 1) + f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + f2 = QgsFeature(layer.fields(), 1) + f2.setGeometry(QgsGeometry.fromWkt('POLYGON((2 2, 6 2, 6 6, 2 6, 2 2))')) + assert pr.addFeatures([f1, f2]) + assert layer.featureCount() == 2 + + vle = QgsVectorLayerEditUtils(layer) + assert Qgis.GeometryOperationResult.AddRingNotInExistingFeature == vle.addMultiRing(QgsLineString([QgsPoint(8, 8), QgsPoint(8, 9), QgsPoint(9, 9), QgsPoint(9, 8), QgsPoint(8, 8)])) + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" + assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" + + def testAddMultiRing(self): + # test adding ring on multi features + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f1 = QgsFeature(layer.fields(), 1) + f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + f2 = QgsFeature(layer.fields(), 1) + f2.setGeometry(QgsGeometry.fromWkt('POLYGON((2 2, 6 2, 6 6, 2 6, 2 2))')) + assert pr.addFeatures([f1, f2]) + assert layer.featureCount() == 2 + + vle = QgsVectorLayerEditUtils(layer) + assert Qgis.GeometryOperationResult.Success == vle.addMultiRing(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(3 3, 3 4, 4 4, 4 3, 3 3))" + assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2),(3 3, 3 4, 4 4, 4 3, 3 3))" + + +if __name__ == '__main__': + unittest.main() From 732be4999a00b927e0fd8deee4212724564add5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Tue, 18 Oct 2022 12:09:32 +0200 Subject: [PATCH 7/9] QgsMapToolAddRing: add selectedFeatureIds to addMultiRing --- src/app/qgsmaptooladdring.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/qgsmaptooladdring.cpp b/src/app/qgsmaptooladdring.cpp index d971c5bcabd6..7871cafc54b4 100644 --- a/src/app/qgsmaptooladdring.cpp +++ b/src/app/qgsmaptooladdring.cpp @@ -70,7 +70,7 @@ void QgsMapToolAddRing::polygonCaptured( const QgsCurvePolygon *polygon ) vlayer->beginEditCommand( tr( "Ring added" ) ); QgsVectorLayerEditUtils utils( vlayer ); - const Qgis::GeometryOperationResult addRingReturnCode = utils.addMultiRing( polygon->exteriorRing()->clone() ); + const Qgis::GeometryOperationResult addRingReturnCode = utils.addMultiRing( polygon->exteriorRing()->clone(), vlayer->selectedFeatureIds() ); QString errorMessage; switch ( addRingReturnCode ) { From e3bf816f55b4b52aeca0e3bcade04aac0dc5f132 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Tue, 18 Oct 2022 12:28:22 +0200 Subject: [PATCH 8/9] rename addMultiRing to addRingV2 --- .../auto_generated/vector/qgsvectorlayereditutils.sip.in | 2 +- src/app/qgsmaptooladdring.cpp | 2 +- src/core/vector/qgsvectorlayereditutils.cpp | 2 +- src/core/vector/qgsvectorlayereditutils.h | 2 +- tests/src/python/test_qgsvectorlayereditutils.py | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in index 447750e48852..6fd37b7d27af 100644 --- a/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in +++ b/python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in @@ -87,7 +87,7 @@ Adds a ring to polygon/multipolygon features :return: OperationResult result code: success or reason of failure %End - Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring /Transfer/, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds /Out/ = 0 ); + Qgis::GeometryOperationResult addRingV2( QgsCurve *ring /Transfer/, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds /Out/ = 0 ); %Docstring Adds a ring to polygon/multipolygon features diff --git a/src/app/qgsmaptooladdring.cpp b/src/app/qgsmaptooladdring.cpp index 7871cafc54b4..b412d852e51b 100644 --- a/src/app/qgsmaptooladdring.cpp +++ b/src/app/qgsmaptooladdring.cpp @@ -70,7 +70,7 @@ void QgsMapToolAddRing::polygonCaptured( const QgsCurvePolygon *polygon ) vlayer->beginEditCommand( tr( "Ring added" ) ); QgsVectorLayerEditUtils utils( vlayer ); - const Qgis::GeometryOperationResult addRingReturnCode = utils.addMultiRing( polygon->exteriorRing()->clone(), vlayer->selectedFeatureIds() ); + const Qgis::GeometryOperationResult addRingReturnCode = utils.addRingV2( polygon->exteriorRing()->clone(), vlayer->selectedFeatureIds() ); QString errorMessage; switch ( addRingReturnCode ) { diff --git a/src/core/vector/qgsvectorlayereditutils.cpp b/src/core/vector/qgsvectorlayereditutils.cpp index afc8a176bacc..dc5d6a8d6d53 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -216,7 +216,7 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, return staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, nullptr, true ); } -Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds ) +Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRingV2( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds ) { std::unique_ptr uniquePtrRing( ring ); diff --git a/src/core/vector/qgsvectorlayereditutils.h b/src/core/vector/qgsvectorlayereditutils.h index 565498cf4e25..4e75ea5b48b5 100644 --- a/src/core/vector/qgsvectorlayereditutils.h +++ b/src/core/vector/qgsvectorlayereditutils.h @@ -100,7 +100,7 @@ class CORE_EXPORT QgsVectorLayerEditUtils * \return OperationResult result code: success or reason of failure * \since QGIS 3.28 */ - Qgis::GeometryOperationResult addMultiRing( QgsCurve *ring SIP_TRANSFER, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds SIP_OUT = nullptr ); + Qgis::GeometryOperationResult addRingV2( QgsCurve *ring SIP_TRANSFER, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds SIP_OUT = nullptr ); /** * Adds a ring to polygon/multipolygon features diff --git a/tests/src/python/test_qgsvectorlayereditutils.py b/tests/src/python/test_qgsvectorlayereditutils.py index 6fa6d575ea20..6bb3c44fbe4d 100644 --- a/tests/src/python/test_qgsvectorlayereditutils.py +++ b/tests/src/python/test_qgsvectorlayereditutils.py @@ -127,7 +127,7 @@ def testAddMultiRingNotEditable(self): layer.commitChanges() vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.LayerNotEditable == vle.addMultiRing(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + assert Qgis.GeometryOperationResult.LayerNotEditable == vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" @@ -148,7 +148,7 @@ def testAddMultiRingNotClosedRing(self): assert layer.featureCount() == 2 vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.AddRingNotClosed == vle.addMultiRing(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3)])) + assert Qgis.GeometryOperationResult.AddRingNotClosed == vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3)])) layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" @@ -169,7 +169,7 @@ def testAddMultiRingOutside(self): assert layer.featureCount() == 2 vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.AddRingNotInExistingFeature == vle.addMultiRing(QgsLineString([QgsPoint(8, 8), QgsPoint(8, 9), QgsPoint(9, 9), QgsPoint(9, 8), QgsPoint(8, 8)])) + assert Qgis.GeometryOperationResult.AddRingNotInExistingFeature == vle.addRingV2(QgsLineString([QgsPoint(8, 8), QgsPoint(8, 9), QgsPoint(9, 9), QgsPoint(9, 8), QgsPoint(8, 8)])) layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" @@ -189,7 +189,7 @@ def testAddMultiRing(self): assert layer.featureCount() == 2 vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.Success == vle.addMultiRing(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + assert Qgis.GeometryOperationResult.Success == vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(3 3, 3 4, 4 4, 4 3, 3 3))" From 2cede134edd84d69b86016eee97b2ca332af547d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Bartoletti?= Date: Tue, 18 Oct 2022 13:00:16 +0200 Subject: [PATCH 9/9] add selected features test --- .../python/test_qgsvectorlayereditutils.py | 49 +++++++++++++++---- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/tests/src/python/test_qgsvectorlayereditutils.py b/tests/src/python/test_qgsvectorlayereditutils.py index 6bb3c44fbe4d..acb0bcb0d4a2 100644 --- a/tests/src/python/test_qgsvectorlayereditutils.py +++ b/tests/src/python/test_qgsvectorlayereditutils.py @@ -51,7 +51,7 @@ def createEmptyPolygonLayer(): return layer -class TestQgsVectorLayerEditBuffer(unittest.TestCase): +class TestQgsVectorLayerEditUtils(unittest.TestCase): def testAddRing(self): # test adding ring to a vector layer @@ -111,7 +111,7 @@ def testAddRingOverlappedFeatures(self): assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(3 3, 3 4, 4 4, 4 3, 3 3))" assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" - def testAddMultiRingNotEditable(self): + def testAddRingV2NotEditable(self): # test adding ring on multi features # layer not editable layer = createEmptyPolygonLayer() @@ -127,13 +127,15 @@ def testAddMultiRingNotEditable(self): layer.commitChanges() vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.LayerNotEditable == vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + result = vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + assert Qgis.GeometryOperationResult.LayerNotEditable == result[0] + assert [] == result[1] layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" - def testAddMultiRingNotClosedRing(self): + def testAddRingV2NotClosedRing(self): # test adding ring on multi features # Not closed ring layer = createEmptyPolygonLayer() @@ -148,13 +150,15 @@ def testAddMultiRingNotClosedRing(self): assert layer.featureCount() == 2 vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.AddRingNotClosed == vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3)])) + result = vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3)])) + assert Qgis.GeometryOperationResult.AddRingNotClosed == result[0] + assert [] == result[1] layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" - def testAddMultiRingOutside(self): + def testAddRingV2Outside(self): # test adding ring on multi features # Not In Existing Feature layer = createEmptyPolygonLayer() @@ -169,13 +173,15 @@ def testAddMultiRingOutside(self): assert layer.featureCount() == 2 vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.AddRingNotInExistingFeature == vle.addRingV2(QgsLineString([QgsPoint(8, 8), QgsPoint(8, 9), QgsPoint(9, 9), QgsPoint(9, 8), QgsPoint(8, 8)])) + result = vle.addRingV2(QgsLineString([QgsPoint(8, 8), QgsPoint(8, 9), QgsPoint(9, 9), QgsPoint(9, 8), QgsPoint(8, 8)])) + assert Qgis.GeometryOperationResult.AddRingNotInExistingFeature == result[0] + assert [] == result[1] layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0))" assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" - def testAddMultiRing(self): + def testAddRingV2(self): # test adding ring on multi features layer = createEmptyPolygonLayer() self.assertTrue(layer.startEditing()) @@ -189,12 +195,37 @@ def testAddMultiRing(self): assert layer.featureCount() == 2 vle = QgsVectorLayerEditUtils(layer) - assert Qgis.GeometryOperationResult.Success == vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + result = vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)])) + assert Qgis.GeometryOperationResult.Success == result[0] + assert [2, 1] == result[1] layer.commitChanges() assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(3 3, 3 4, 4 4, 4 3, 3 3))" assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2),(3 3, 3 4, 4 4, 4 3, 3 3))" + def testAddRingV2SelectedFeatures(self): + # test adding ring on multi features + # test selected features + layer = createEmptyPolygonLayer() + self.assertTrue(layer.startEditing()) + + pr = layer.dataProvider() + f1 = QgsFeature(layer.fields(), 1) + f1.setGeometry(QgsGeometry.fromWkt('POLYGON((0 0, 5 0, 5 5, 0 5, 0 0))')) + f2 = QgsFeature(layer.fields(), 1) + f2.setGeometry(QgsGeometry.fromWkt('POLYGON((2 2, 6 2, 6 6, 2 6, 2 2))')) + assert pr.addFeatures([f1, f2]) + assert layer.featureCount() == 2 + + vle = QgsVectorLayerEditUtils(layer) + result = vle.addRingV2(QgsLineString([QgsPoint(3, 3), QgsPoint(3, 4), QgsPoint(4, 4), QgsPoint(4, 3), QgsPoint(3, 3)]), [1]) + assert Qgis.GeometryOperationResult.Success == result[0] + assert [1] == result[1] + layer.commitChanges() + + assert layer.getFeature(1).geometry().asWkt() == "Polygon ((0 0, 5 0, 5 5, 0 5, 0 0),(3 3, 3 4, 4 4, 4 3, 3 3))" + assert layer.getFeature(2).geometry().asWkt() == "Polygon ((2 2, 6 2, 6 6, 2 6, 2 2))" + if __name__ == '__main__': unittest.main()