From 71e2a6be7e5d59d6ee7d137c28754ae527a8465b Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 18 Oct 2018 17:43:00 +0200 Subject: [PATCH] In-place passthrough for invalid geometries if processing option is to skip invalid Fixes #20147 - difference deletes invalid geometries --- .../processing/core/ProcessingConfig.py | 8 ++- .../processing/gui/AlgorithmExecutor.py | 45 ++++++++++--- tests/src/python/test_qgsprocessinginplace.py | 67 +++++++++++++++++++ 3 files changed, 110 insertions(+), 10 deletions(-) diff --git a/python/plugins/processing/core/ProcessingConfig.py b/python/plugins/processing/core/ProcessingConfig.py index c63858039fe..a6ae48d9e2c 100644 --- a/python/plugins/processing/core/ProcessingConfig.py +++ b/python/plugins/processing/core/ProcessingConfig.py @@ -293,7 +293,9 @@ class Setting: 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 @@ class Setting: 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 index 563cbe51ac6..95053100c58 100644 --- 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 index ad44a4f492a..bee76230b55 100644 --- a/tests/src/python/test_qgsprocessinginplace.py +++ b/tests/src/python/test_qgsprocessinginplace.py @@ -17,6 +17,8 @@ from qgis.core import ( 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 @@ class TestQgsProcessingInPlace(unittest.TestCase): 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()