In-place passthrough for invalid geometries if processing option is t…

…o skip invalid Fixes #20147 - difference deletes invalid geometries
qgis · Oct 18, 2018 · 71e2a6b · 71e2a6b
1 parent 51c3c2a
commit 71e2a6b
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 10 deletions.
diff --git a/python/plugins/processing/core/ProcessingConfig.py b/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:

diff --git a/python/plugins/processing/gui/AlgorithmExecutor.py b/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:
@@ -178,7 +183,7 @@ def execute_in_place_run(alg, parameters, context=None, feedback=None, raise_exc
                 else:
                     active_layer.deleteFeature(f.id())
                     # Get the new ids
-                    old_ids = set([f.id() for f in active_layer.getFeatures(req)])
+                    old_ids = set([f.id() for f in active_layer.getFeatures()])
                     if not active_layer.addFeatures(new_features):
                         raise QgsProcessingException(tr("Error adding processed features back into the layer."))
                     new_ids = set([f.id() for f in active_layer.getFeatures(req)])
@@ -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
 
 

diff --git a/tests/src/python/test_qgsprocessinginplace.py b/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
@@ -659,6 +661,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()