Skip to content

Commit

Permalink
Merge pull request #8231 from elpaso/bugfix-20147-in-place-difference
Browse files Browse the repository at this point in the history 
[in-place][needs-docs] add buffer for polygons and fix #20147 in place difference
  • Loading branch information
@elpaso
elpaso committed Oct 19, 2018
2 parents f00e43d + 21e685b commit 71e85cc
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 17 deletions.
3 changes: 1 addition & 2 deletions python/core/auto_generated/qgsfeaturerequest.sip.in
View file
Expand Up @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions python/plugins/processing/core/ProcessingConfig.py
View file
Expand Up @@ -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):
Expand All @@ -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:
Expand Down
43 changes: 36 additions & 7 deletions python/plugins/processing/gui/AlgorithmExecutor.py
View file
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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))
Expand Down Expand Up @@ -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)

This comment has been minimized.

Sign in to view

Copy link
@nirvn

nirvn Oct 20, 2018

Contributor

@elpaso , this change has regressed in place editing for convex hull alg (possibly more, just spotted that one). When selecting a few features, enabling in-place editing and selecting the convex hull alg, the following error is thrown:

TypeError: QgsProcessingFeatureBasedAlgorithm.parameterAsVectorLayer(): argument 3 has unexpected type 'NoneType' 
Traceback (most recent call last):
  File "/home/webmaster/apps/share/qgis/python/plugins/processing/gui/ProcessingToolbox.py", line 236, in executeAlgorithm
    ok, results = execute_in_place(alg, parameters, feedback=feedback)
  File "/home/webmaster/apps/share/qgis/python/plugins/processing/gui/AlgorithmExecutor.py", line 276, in execute_in_place
    layer = alg.parameterAsVectorLayer({'INPUT': parameters['INPUT'].source}, 'INPUT', context)
TypeError: QgsProcessingFeatureBasedAlgorithm.parameterAsVectorLayer(): argument 3 has unexpected type 'NoneType'

This comment has been minimized.

Sign in to view

Copy link
@elpaso

elpaso Oct 20, 2018

Author Contributor

@nirvn ouch, fixed with dcd3318
elif isinstance(parameters['INPUT'], QgsVectorLayer):
layer = parameters['INPUT']
if layer:
layer.triggerRepaint()
return ok, results


Expand Down
18 changes: 18 additions & 0 deletions src/analysis/processing/qgsalgorithmbuffer.cpp
View file
Expand Up @@ -16,6 +16,8 @@
***************************************************************************/

#include "qgsalgorithmbuffer.h"
#include "qgswkbtypes.h"
#include "qgsvectorlayer.h"

///@cond PRIVATE

Expand Down Expand Up @@ -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
4 changes: 4 additions & 0 deletions src/analysis/processing/qgsalgorithmbuffer.h
View file
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/analysis/processing/qgsoverlayutils.cpp
View file
Expand Up @@ -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 );
Expand Down
3 changes: 1 addition & 2 deletions src/core/qgsfeaturerequest.h
View file
Expand Up @@ -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 );

Expand Down
77 changes: 73 additions & 4 deletions tests/src/python/test_qgsprocessinginplace.py
View file
Expand Up @@ -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
Expand Down Expand Up @@ -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()}
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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()

0 comments on commit 71e85cc

Please sign in to comment.