QgsVectorLayer: add addMultiRing and use it in QgsMapToolAddRing

python/core/auto_generated/vector/qgsvectorlayereditutils.sip.in

@@ -85,6 +85,20 @@ Adds a ring to polygon/multipolygon features
 :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 /Transfer/, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds /Out/ = 0 );
+%Docstring
+Adds a ring to polygon/multipolygon features
+
+: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.
+
+: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
 
     Qgis::GeometryOperationResult addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureId *modifiedFeatureId = 0 ) /PyName=addCurvedRing/;

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->addRing( polygon->exteriorRing()->clone() );
+  QgsVectorLayerEditUtils utils( vlayer );
+  const Qgis::GeometryOperationResult addRingReturnCode = utils.addMultiRing( polygon->exteriorRing()->clone() );
   QString errorMessage;
   switch ( addRingReturnCode )
   {

src/core/vector/qgsvectorlayereditutils.cpp

@@ -118,28 +118,29 @@ Qgis::VectorEditResult QgsVectorLayerEditUtils::deleteVertex( QgsFeatureId featu
   return !geometry.isNull() ? Qgis::VectorEditResult::Success : Qgis::VectorEditResult::EmptyGeometry;
 }
 
-Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( const QVector<QgsPointXY> &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId )
+
+static
+Qgis::GeometryOperationResult staticAddRing( QgsVectorLayer *layer, std::unique_ptr< QgsCurve > &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds, bool firstOne = true )
 {
-  QgsPointSequence l;
-  for ( QVector<QgsPointXY>::const_iterator it = ring.constBegin(); it != ring.constEnd(); ++it )
+
+  if ( !layer || !layer->isSpatial() )
   {
-    l <<  QgsPoint( *it );
+    return Qgis::GeometryOperationResult::AddRingNotInExistingFeature;
   }
-  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 );
-}
+  if ( !ring )
+  {
+    return Qgis::GeometryOperationResult::InvalidInputGeometryType;
+  }
 
-Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId )
-{
-  if ( !mLayer->isSpatial() )
+  if ( !ring->isClosed() )
   {
-    delete ring;
-    return Qgis::GeometryOperationResult::AddRingNotInExistingFeature;
+    return Qgis::GeometryOperationResult::AddRingNotClosed;
+  }
+
+  if ( !layer->isValid() || !layer->editBuffer() || !layer->dataProvider() )
+  {
+    return Qgis::GeometryOperationResult::LayerNotEditable;
   }
 
   Qgis::GeometryOperationResult addRingReturnCode = Qgis::GeometryOperationResult::AddRingNotInExistingFeature; //default: return code for 'ring not inserted'
@@ -149,13 +150,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,19 +171,60 @@ 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<QgsPointXY> &ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureId *modifiedFeatureId )
+{
+  QgsPointSequence l;
+  for ( QVector<QgsPointXY>::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 )
+{
+  std::unique_ptr<QgsCurve> uniquePtrRing( ring );
+  if ( modifiedFeatureId )
+  {
+    QgsFeatureIds *modifiedFeatureIds = new QgsFeatureIds;
+    Qgis::GeometryOperationResult result = staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, modifiedFeatureIds, true );
+    *modifiedFeatureId = *modifiedFeatureIds->begin();
+    return result;
+  }
+  return staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, nullptr, true );
+}
+
+Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addMultiRing( QgsCurve *ring, const QgsFeatureIds &targetFeatureIds, QgsFeatureIds *modifiedFeatureIds )
+{
+
+  std::unique_ptr<QgsCurve> uniquePtrRing( ring );
+  return staticAddRing( mLayer, uniquePtrRing, targetFeatureIds, modifiedFeatureIds, false );
+}
+
+
+
 Qgis::GeometryOperationResult QgsVectorLayerEditUtils::addPart( const QVector<QgsPointXY> &points, QgsFeatureId featureId )
 {
   QgsPointSequence l;

src/core/vector/qgsvectorlayereditutils.h

@@ -91,6 +91,17 @@ 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 (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 SIP_TRANSFER, const QgsFeatureIds &targetFeatureIds = QgsFeatureIds(), QgsFeatureIds *modifiedFeatureIds SIP_OUT = nullptr );
+
     /**
      * Adds a ring to polygon/multipolygon features
      * \param ring ring to add

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)

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()