Merge pull request #8231 from elpaso/bugfix-20147-in-place-difference

[in-place][needs-docs] add buffer for polygons and fix #20147 in place difference

Author: elpaso

python/core/auto_generated/qgsfeaturerequest.sip.in

@@ -508,8 +508,7 @@ Sets flags that affect how features will be fetched
 
     QgsFeatureRequest &setSubsetOfAttributes( const QgsAttributeList &attrs );
 %Docstring
-Set a subset of attributes that will be fetched. Empty list means that all attributes are used.
-To disable fetching attributes, reset the FetchAttributes flag (which is set by default)
+Set a subset of attributes that will be fetched.
 %End
 
     QgsAttributeList subsetOfAttributes() const;

python/plugins/processing/core/ProcessingConfig.py

@@ -293,7 +293,9 @@ def setValue(self, value):
         self.validator(value)
         self.value = value
 
-    def read(self, qsettings=QgsSettings()):
+    def read(self, qsettings=None):
+        if not qsettings:
+            qsettings = QgsSettings()
         value = qsettings.value(self.qname, None)
         if value is not None:
             if isinstance(self.value, bool):
@@ -307,7 +309,9 @@ def read(self, qsettings=QgsSettings()):
             else:
                 self.value = value
 
-    def save(self, qsettings=QgsSettings()):
+    def save(self, qsettings=None):
+        if not qsettings:
+            qsettings = QgsSettings()
         if self.valuetype == self.SELECTION:
             qsettings.setValue(self.qname, self.options.index(self.value))
         else:

python/plugins/processing/gui/AlgorithmExecutor.py

@@ -128,8 +128,14 @@ def execute_in_place_run(alg, parameters, context=None, feedback=None, raise_exc
     if not active_layer.selectedFeatureIds():
         active_layer.selectAll()
 
+    # Make sure we are working on selected features only
+    parameters['INPUT'] = QgsProcessingFeatureSourceDefinition(active_layer.id(), True)
     parameters['OUTPUT'] = 'memory:'
 
+    req = QgsFeatureRequest(QgsExpression(r"$id < 0"))
+    req.setFlags(QgsFeatureRequest.NoGeometry)
+    req.setSubsetOfAttributes([])
+
     # Start the execution
     # If anything goes wrong and raise_exceptions is True an exception
     # is raised, else the execution is aborted and the error reported in
@@ -139,10 +145,6 @@ def execute_in_place_run(alg, parameters, context=None, feedback=None, raise_exc
 
         active_layer.beginEditCommand(alg.displayName())
 
-        req = QgsFeatureRequest(QgsExpression(r"$id < 0"))
-        req.setFlags(QgsFeatureRequest.NoGeometry)
-        req.setSubsetOfAttributes([])
-
         # Checks whether the algorithm has a processFeature method
         if hasattr(alg, 'processFeature'):  # in-place feature editing
             # Make a clone or it will crash the second time the dialog
@@ -154,7 +156,9 @@ def execute_in_place_run(alg, parameters, context=None, feedback=None, raise_exc
             if not alg.supportInPlaceEdit(active_layer):
                 raise QgsProcessingException(tr("Selected algorithm and parameter configuration are not compatible with in-place modifications."))
             field_idxs = range(len(active_layer.fields()))
-            feature_iterator = active_layer.getFeatures(QgsFeatureRequest(active_layer.selectedFeatureIds()))
+            iterator_req = QgsFeatureRequest(active_layer.selectedFeatureIds())
+            iterator_req.setInvalidGeometryCheck(context.invalidGeometryCheck())
+            feature_iterator = active_layer.getFeatures(iterator_req)
             step = 100 / len(active_layer.selectedFeatureIds()) if active_layer.selectedFeatureIds() else 1
             for current, f in enumerate(feature_iterator):
                 feedback.setProgress(current * step)
@@ -166,6 +170,7 @@ def execute_in_place_run(alg, parameters, context=None, feedback=None, raise_exc
                 input_feature = QgsFeature(f)
                 new_features = alg.processFeature(input_feature, context, feedback)
                 new_features = QgsVectorLayerUtils.makeFeaturesCompatible(new_features, active_layer)
+
                 if len(new_features) == 0:
                     active_layer.deleteFeature(f.id())
                 elif len(new_features) == 1:
@@ -187,13 +192,32 @@ def execute_in_place_run(alg, parameters, context=None, feedback=None, raise_exc
             results, ok = {}, True
 
         else:  # Traditional 'run' with delete and add features cycle
+
+            # There is no way to know if some features have been skipped
+            # due to invalid geometries
+            if context.invalidGeometryCheck() == QgsFeatureRequest.GeometrySkipInvalid:
+                selected_ids = active_layer.selectedFeatureIds()
+            else:
+                selected_ids = []
+
             results, ok = alg.run(parameters, context, feedback)
 
             if ok:
                 result_layer = QgsProcessingUtils.mapLayerFromString(results['OUTPUT'], context)
                 # TODO: check if features have changed before delete/add cycle
-                active_layer.deleteFeatures(active_layer.selectedFeatureIds())
+
                 new_features = []
+
+                # Check if there are any skipped features
+                if context.invalidGeometryCheck() == QgsFeatureRequest.GeometrySkipInvalid:
+                    missing_ids = list(set(selected_ids) - set(result_layer.allFeatureIds()))
+                    if missing_ids:
+                        for f in active_layer.getFeatures(QgsFeatureRequest(missing_ids)):
+                            if not f.geometry().isGeosValid():
+                                new_features.append(f)
+
+                active_layer.deleteFeatures(active_layer.selectedFeatureIds())
+
                 for f in result_layer.getFeatures():
                     new_features.extend(QgsVectorLayerUtils.
                                         makeFeaturesCompatible([f], active_layer))
@@ -248,7 +272,12 @@ def execute_in_place(alg, parameters, context=None, feedback=None):
         parameters['INPUT'] = iface.activeLayer()
     ok, results = execute_in_place_run(alg, parameters, context=context, feedback=feedback)
     if ok:
-        parameters['INPUT'].triggerRepaint()
+        if isinstance(parameters['INPUT'], QgsProcessingFeatureSourceDefinition):
+            layer = alg.parameterAsVectorLayer({'INPUT': parameters['INPUT'].source}, 'INPUT', context)
+        elif isinstance(parameters['INPUT'], QgsVectorLayer):
+            layer = parameters['INPUT']
+        if layer:
+            layer.triggerRepaint()
     return ok, results
 
 

src/analysis/processing/qgsalgorithmbuffer.cpp

@@ -16,6 +16,8 @@
  ***************************************************************************/
 
 #include "qgsalgorithmbuffer.h"
+#include "qgswkbtypes.h"
+#include "qgsvectorlayer.h"
 
 ///@cond PRIVATE
 
@@ -165,4 +167,20 @@ QVariantMap QgsBufferAlgorithm::processAlgorithm( const QVariantMap &parameters,
   return outputs;
 }
 
+QgsProcessingAlgorithm::Flags QgsBufferAlgorithm::flags() const
+{
+  Flags f = QgsProcessingAlgorithm::flags();
+  f |= QgsProcessingAlgorithm::FlagSupportsInPlaceEdits;
+  return f;
+}
+
+bool QgsBufferAlgorithm::supportInPlaceEdit( const QgsMapLayer *layer ) const
+{
+  const QgsVectorLayer *vlayer = qobject_cast< const QgsVectorLayer * >( layer );
+  if ( !vlayer )
+    return false;
+  //Only Polygons
+  return vlayer->wkbType() == QgsWkbTypes::Type::Polygon || vlayer->wkbType() == QgsWkbTypes::Type::MultiPolygon;
+}
+
 ///@endcond

src/analysis/processing/qgsalgorithmbuffer.h

@@ -44,12 +44,16 @@ class QgsBufferAlgorithm : public QgsProcessingAlgorithm
     QString groupId() const override;
     QString shortHelpString() const override;
     QgsBufferAlgorithm *createInstance() const override SIP_FACTORY;
+    bool supportInPlaceEdit( const QgsMapLayer *layer ) const override;
+    QgsProcessingAlgorithm::Flags flags() const override;
+
 
   protected:
 
     QVariantMap processAlgorithm( const QVariantMap &parameters,
                                   QgsProcessingContext &context, QgsProcessingFeedback *feedback ) override;
 
+
 };
 
 ///@endcond PRIVATE

src/analysis/processing/qgsoverlayutils.cpp

@@ -92,6 +92,7 @@ void QgsOverlayUtils::difference( const QgsFeatureSource &sourceA, const QgsFeat
 
   QgsFeature featA;
   QgsFeatureRequest requestA;
+  requestA.setInvalidGeometryCheck( context.invalidGeometryCheck() );
   if ( outputAttrs == OutputBA )
     requestA.setDestinationCrs( sourceB.sourceCrs(), context.transformContext() );
   QgsFeatureIterator fitA = sourceA.getFeatures( requestA );

src/core/qgsfeaturerequest.h

@@ -493,8 +493,7 @@ class CORE_EXPORT QgsFeatureRequest
     const Flags &flags() const { return mFlags; }
 
     /**
-     * Set a subset of attributes that will be fetched. Empty list means that all attributes are used.
-     * To disable fetching attributes, reset the FetchAttributes flag (which is set by default)
+     * Set a subset of attributes that will be fetched.
      */
     QgsFeatureRequest &setSubsetOfAttributes( const QgsAttributeList &attrs );
 

tests/src/python/test_qgsprocessinginplace.py

@@ -17,6 +17,8 @@
     QgsFeature, QgsGeometry, QgsSettings, QgsApplication, QgsMemoryProviderUtils, QgsWkbTypes, QgsField, QgsFields, QgsProcessingFeatureSourceDefinition, QgsProcessingContext, QgsProcessingFeedback, QgsCoordinateReferenceSystem, QgsProject, QgsProcessingException
 )
 from processing.core.Processing import Processing
+from processing.core.ProcessingConfig import ProcessingConfig
+from processing.tools import dataobjects
 from processing.gui.AlgorithmExecutor import execute_in_place_run
 from qgis.testing import start_app, unittest
 from qgis.PyQt.QtTest import QSignalSpy
@@ -132,7 +134,8 @@ def test_support_in_place_edit(self):
         Z_ONLY = {t: t.find('Z') > 0 for t in _all_true().keys()}
         M_ONLY = {t: t.rfind('M') > 0 for t in _all_true().keys()}
         NOT_M = {t: t.rfind('M') < 1 and t != 'NoGeometry' for t in _all_true().keys()}
-        POLYGON_ONLY = {t: t in ('Polygon', 'MultiPolygon') for t in _all_true().keys()}
+        POLYGON_ONLY = {t: t.find('Polygon') for t in _all_true().keys()}
+        POLYGON_ONLY_NOT_M_NOT_Z = {t: t in ('Polygon', 'MultiPolygon') for t in _all_true().keys()}
         MULTI_ONLY = {t: t.find('Multi') == 0 for t in _all_true().keys()}
         SINGLE_ONLY = {t: t.find('Multi') == -1 for t in _all_true().keys()}
         LINESTRING_AND_POLYGON_ONLY = {t: (t.find('LineString') >= 0 or t.find('Polygon') >= 0) for t in _all_true().keys()}
@@ -149,9 +152,9 @@ def test_support_in_place_edit(self):
         self._support_inplace_edit_tester('native:explodelines', LINESTRING_ONLY)
         self._support_inplace_edit_tester('native:extendlines', LINESTRING_ONLY)
         self._support_inplace_edit_tester('native:fixgeometries', NOT_M)
-        self._support_inplace_edit_tester('native:minimumenclosingcircle', POLYGON_ONLY)
-        self._support_inplace_edit_tester('native:multiringconstantbuffer', POLYGON_ONLY)
-        self._support_inplace_edit_tester('native:orientedminimumboundingbox', POLYGON_ONLY)
+        self._support_inplace_edit_tester('native:minimumenclosingcircle', POLYGON_ONLY_NOT_M_NOT_Z)
+        self._support_inplace_edit_tester('native:multiringconstantbuffer', POLYGON_ONLY_NOT_M_NOT_Z)
+        self._support_inplace_edit_tester('native:orientedminimumboundingbox', POLYGON_ONLY_NOT_M_NOT_Z)
         self._support_inplace_edit_tester('qgis:orthogonalize', LINESTRING_AND_POLYGON_ONLY)
         self._support_inplace_edit_tester('native:removeduplicatevertices', GEOMETRY_ONLY)
         self._support_inplace_edit_tester('native:rotatefeatures', GEOMETRY_ONLY)
@@ -172,6 +175,7 @@ def test_support_in_place_edit(self):
         self._support_inplace_edit_tester('native:difference', GEOMETRY_ONLY)
         self._support_inplace_edit_tester('native:dropgeometries', ALL)
         self._support_inplace_edit_tester('native:splitwithlines', LINESTRING_AND_POLYGON_ONLY)
+        self._support_inplace_edit_tester('native:buffer', POLYGON_ONLY_NOT_M_NOT_Z)
 
     def _make_compatible_tester(self, feature_wkt, layer_wkb_name, attrs=[1]):
         layer = self._make_layer(layer_wkb_name)
@@ -659,6 +663,71 @@ def test_fix_geometries(self):
         self.assertEqual(wkt1, 'PolygonZ ((0 0 1, 1 1 2.25, 2 0 4, 0 0 1))')
         self.assertEqual(wkt2, 'PolygonZ ((1 1 2.25, 0 2 3, 2 2 1, 1 1 2.25))')
 
+    def _test_difference_on_invalid_geometries(self, geom_option):
+        polygon_layer = self._make_layer('Polygon')
+        self.assertTrue(polygon_layer.startEditing())
+        f = QgsFeature(polygon_layer.fields())
+        f.setAttributes([1])
+        # Flake!
+        f.setGeometry(QgsGeometry.fromWkt('Polygon ((0 0, 2 2, 0 2, 2 0, 0 0))'))
+        self.assertTrue(f.isValid())
+        self.assertTrue(polygon_layer.addFeatures([f]))
+        polygon_layer.commitChanges()
+        polygon_layer.rollBack()
+        self.assertEqual(polygon_layer.featureCount(), 1)
+
+        overlay_layer = self._make_layer('Polygon')
+        self.assertTrue(overlay_layer.startEditing())
+        f = QgsFeature(overlay_layer.fields())
+        f.setAttributes([1])
+        f.setGeometry(QgsGeometry.fromWkt('Polygon ((0 0, 2 0, 2 2, 0 2, 0 0))'))
+        self.assertTrue(f.isValid())
+        self.assertTrue(overlay_layer.addFeatures([f]))
+        overlay_layer.commitChanges()
+        overlay_layer.rollBack()
+        self.assertEqual(overlay_layer.featureCount(), 1)
+
+        QgsProject.instance().addMapLayers([polygon_layer, overlay_layer])
+
+        old_features = [f for f in polygon_layer.getFeatures()]
+
+        # 'Ignore features with invalid geometries' = 1
+        ProcessingConfig.setSettingValue(ProcessingConfig.FILTER_INVALID_GEOMETRIES, geom_option)
+
+        feedback = ConsoleFeedBack()
+        context = dataobjects.createContext(feedback)
+        context.setProject(QgsProject.instance())
+
+        alg = self.registry.createAlgorithmById('native:difference')
+        self.assertIsNotNone(alg)
+
+        parameters = {
+            'OVERLAY': overlay_layer,
+            'INPUT': polygon_layer,
+            'OUTPUT': ':memory',
+        }
+
+        old_features = [f for f in polygon_layer.getFeatures()]
+
+        self.assertTrue(polygon_layer.startEditing())
+        polygon_layer.selectAll()
+        ok, _ = execute_in_place_run(
+            alg, parameters, context=context, feedback=feedback, raise_exceptions=True)
+
+        new_features = [f for f in polygon_layer.getFeatures()]
+
+        return old_features, new_features
+
+    def test_difference_on_invalid_geometries(self):
+        """Test #20147 difference deletes invalid geometries"""
+
+        old_features, new_features = self._test_difference_on_invalid_geometries(1)
+        self.assertEqual(len(new_features), 1)
+        old_features, new_features = self._test_difference_on_invalid_geometries(0)
+        self.assertEqual(len(new_features), 1)
+        old_features, new_features = self._test_difference_on_invalid_geometries(2)
+        self.assertEqual(len(new_features), 1)
+
 
 if __name__ == '__main__':
     unittest.main()