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] 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 | 111 +++++++++++------- src/core/vector/qgsvectorlayereditutils.h | 10 ++ 7 files changed, 183 insertions(+), 44 deletions(-) diff --git a/python/core/auto_generated/vector/qgsvectorlayer.sip.in b/python/core/auto_generated/vector/qgsvectorlayer.sip.in index be89cf3e240..030d469e7bc 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 17bd71e297e..af0c2469baf 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 bbb39558973..86796c5223f 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 593bc382ece..9a40d8abd23 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 b60315b6236..2120ddc2fab 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 a532eeba75d..9d1cd0b2692 100644 --- a/src/core/vector/qgsvectorlayereditutils.cpp +++ b/src/core/vector/qgsvectorlayereditutils.cpp @@ -118,6 +118,61 @@ Qgis::VectorEditResult QgsVectorLayerEditUtils::deleteVertex( QgsFeatureId featu return !geometry.isNull() ? Qgis::VectorEditResult::Success : Qgis::VectorEditResult::EmptyGeometry; } + +static +Qgis::GeometryOperationResult _addRing( QgsVectorLayer *layer, QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true ) +{ + + if ( !layer || !layer->isSpatial() ) + { + delete ring; + return Qgis::GeometryOperationResult::AddRingNotInExistingFeature; + } + + Qgis::GeometryOperationResult addRingReturnCode = Qgis::GeometryOperationResult::AddRingNotInExistingFeature; //default: return code for 'ring not inserted' + QgsFeature f; + + QgsFeatureIterator fit; + if ( !targetFeatureIds.isEmpty() ) + { + //check only specified features + fit = layer->getFeatures( QgsFeatureRequest().setFilterFids( targetFeatureIds ) ); + } + else + { + //check all intersecting features + QgsRectangle bBox = ring->boundingBox(); + fit = layer->getFeatures( QgsFeatureRequest().setFilterRect( bBox ).setFlags( QgsFeatureRequest::ExactIntersect ) ); + } + + //find first valid feature we can add the ring to + while ( fit.nextFeature( f ) ) + { + if ( !f.hasGeometry() ) + continue; + + //add ring takes ownership of ring, and deletes it if there's an error + QgsGeometry g = f.geometry(); + + addRingReturnCode = g.addRing( static_cast< QgsCurve * >( ring->clone() ) ); + if ( addRingReturnCode == Qgis::GeometryOperationResult::Success ) + { + layer->changeGeometry( f.id(), g ); + if ( modifiedFeatureIds ) + { + modifiedFeatureIds->insert( f.id() ); + if ( firstOne ) + { + break; + } + } + + } + } + + delete ring; + return addRingReturnCode; +} Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QVector &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) { QgsPointSequence l; @@ -136,53 +191,23 @@ Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QgsPointSe Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId ) { - if ( !mLayer->isSpatial() ) + if ( modifiedFeatureId ) { - delete ring; - return Qgis::GeometryOperationResult::AddRingNotInExistingFeature; + QgsFeatureIds *modifiedFeatureIds = new QgsFeatureIds; + Qgis::GeometryOperationResult result = _addRing( mLayer, ring, targetFeatureIds, modifiedFeatureIds, true ); + *modifiedFeatureId = *modifiedFeatureIds->begin(); + return result; } - - Qgis::GeometryOperationResult addRingReturnCode = Qgis::GeometryOperationResult::AddRingNotInExistingFeature; //default: return code for 'ring not inserted' - QgsFeature f; - - QgsFeatureIterator fit; - if ( !targetFeatureIds.isEmpty() ) - { - //check only specified features - fit = mLayer->getFeatures( QgsFeatureRequest().setFilterFids( targetFeatureIds ) ); - } - else - { - //check all intersecting features - QgsRectangle bBox = ring->boundingBox(); - fit = mLayer->getFeatures( QgsFeatureRequest().setFilterRect( bBox ).setFlags( QgsFeatureRequest::ExactIntersect ) ); - } - - //find first valid feature we can add the ring to - while ( fit.nextFeature( f ) ) - { - if ( !f.hasGeometry() ) - continue; - - //add ring takes ownership of ring, and deletes it if there's an error - QgsGeometry g = f.geometry(); - - addRingReturnCode = g.addRing( static_cast< QgsCurve * >( ring->clone() ) ); - if ( addRingReturnCode == Qgis::GeometryOperationResult::Success ) - { - mLayer->changeGeometry( f.id(), g ); - if ( modifiedFeatureId ) - *modifiedFeatureId = f.id(); - - //setModified( true, true ); - break; - } - } - - delete ring; - return addRingReturnCode; + 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 ) { QgsPointSequence l; diff --git a/src/core/vector/qgsvectorlayereditutils.h b/src/core/vector/qgsvectorlayereditutils.h index 2cfa26dd52a..797698a6692 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