Skip to content

Commit 2d2c229

Browse files
committedJun 11, 2017
Port checkInputCRS to c++, and allow algorithms to flag when they
require all input layers to be in the same CRS The default behaviour is to assume that algorithms are well behaved and can handle multi-CRS inputs, but algs have the option to flag that they do not allow this and require the input CRS check. Those algs should document that they require all inputs to have matching CRS - processing 3.0 behaviour is to assume that algs can handle this.
1 parent 386c424 commit 2d2c229

File tree

8 files changed

+160
-26
lines changed

8 files changed

+160
-26
lines changed
 

‎python/core/processing/qgsprocessingalgorithm.sip

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ class QgsProcessingAlgorithm
2929
FlagHideFromModeler,
3030
FlagSupportsBatch,
3131
FlagCanCancel,
32+
FlagRequiresMatchingCrs,
3233
FlagDeprecated,
3334
};
3435
typedef QFlags<QgsProcessingAlgorithm::Flag> Flags;
@@ -244,6 +245,15 @@ class QgsProcessingAlgorithm
244245
:rtype: QgsExpressionContext
245246
%End
246247

248+
virtual bool validateInputCrs( const QVariantMap &parameters,
249+
QgsProcessingContext &context ) const;
250+
%Docstring
251+
Checks whether the coordinate reference systems for the specified set of ``parameters``
252+
are valid for the algorithm. For instance, the base implementation performs
253+
checks to ensure that all input CRS are equal
254+
Returns true if ``parameters`` have passed the CRS check.
255+
:rtype: bool
256+
%End
247257

248258
protected:
249259

‎python/plugins/processing/core/GeoAlgorithm.py

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -264,26 +264,6 @@ def setOutputCRS(self):
264264
except:
265265
pass
266266

267-
def checkInputCRS(self, context=None):
268-
"""It checks that all input layers use the same CRS. If so,
269-
returns True. False otherwise.
270-
"""
271-
if context is None:
272-
context = dataobjects.createContext()
273-
crsList = []
274-
for param in self.parameterDefinitions():
275-
if isinstance(param, (ParameterRaster, ParameterVector, ParameterMultipleInput)):
276-
if param.value:
277-
if isinstance(param, ParameterMultipleInput):
278-
layers = param.value.split(';')
279-
else:
280-
layers = [param.value]
281-
for item in layers:
282-
crs = QgsProcessingUtils.mapLayerFromString(item, context).crs()
283-
if crs not in crsList:
284-
crsList.append(crs)
285-
return len(crsList) < 2
286-
287267
def addOutput(self, output):
288268
# TODO: check that name does not exist
289269
if isinstance(output, Output):

‎python/plugins/processing/core/Processing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def runAlgorithm(algOrName, onFinish, *args, **kwargs):
212212
Processing.tr("Processing"))
213213
return
214214

215-
if not alg.checkInputCRS(context):
215+
if not alg.validateInputCrs(parameters, context):
216216
print('Warning: Not all input layers use the same CRS.\n' +
217217
'This can cause unexpected results.')
218218
QgsMessageLog.logMessage(

‎python/plugins/processing/gui/AlgorithmDialog.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,7 @@ def accept(self):
168168

169169
QgsMessageLog.logMessage(str(parameters), 'Processing', QgsMessageLog.CRITICAL)
170170

171-
# TODO
172-
if False and checkCRS and not self.alg.checkInputCRS():
171+
if checkCRS and not self.alg.validateInputCrs(parameters, context):
173172
reply = QMessageBox.question(self, self.tr("Unmatching CRS's"),
174173
self.tr('Layers do not all use the same CRS. This can '
175174
'cause unexpected results.\nDo you want to '

‎python/plugins/processing/script/ScriptAlgorithm.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,11 @@ def defineCharacteristicsFromScript(self):
125125
def canExecute(self):
126126
return not self.error, self.error
127127

128-
def checkInputCRS(self):
128+
def validateInputCrs(self, parameters, context):
129129
if self.noCRSWarning:
130130
return True
131131
else:
132-
return GeoAlgorithm.checkInputCRS(self)
132+
return QgsProcessingAlgorithm.validateInputCrs(self, parameters, context)
133133

134134
def createDescriptiveName(self, s):
135135
return s.replace('_', ' ')

‎src/core/processing/qgsprocessingalgorithm.cpp

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "qgsprocessingoutputs.h"
2323
#include "qgsrectangle.h"
2424
#include "qgsprocessingcontext.h"
25+
#include "qgsprocessingutils.h"
2526

2627
QgsProcessingAlgorithm::~QgsProcessingAlgorithm()
2728
{
@@ -117,6 +118,73 @@ QgsExpressionContext QgsProcessingAlgorithm::createExpressionContext( const QVar
117118
return c;
118119
}
119120

121+
bool QgsProcessingAlgorithm::validateInputCrs( const QVariantMap &parameters, QgsProcessingContext &context ) const
122+
{
123+
if ( !( flags() & FlagRequiresMatchingCrs ) )
124+
{
125+
// I'm a well behaved algorithm - I take work AWAY from users!
126+
return true;
127+
}
128+
129+
bool foundCrs = false;
130+
QgsCoordinateReferenceSystem crs;
131+
Q_FOREACH ( const QgsProcessingParameterDefinition *def, mParameters )
132+
{
133+
if ( def->type() == QStringLiteral( "layer" ) || def->type() == QStringLiteral( "raster" ) )
134+
{
135+
QgsMapLayer *layer = QgsProcessingParameters::parameterAsLayer( def, parameters, context );
136+
if ( layer )
137+
{
138+
if ( foundCrs && layer->crs().isValid() && crs != layer->crs() )
139+
{
140+
return false;
141+
}
142+
else if ( !foundCrs && layer->crs().isValid() )
143+
{
144+
foundCrs = true;
145+
crs = layer->crs();
146+
}
147+
}
148+
}
149+
else if ( def->type() == QStringLiteral( "source" ) )
150+
{
151+
QgsFeatureSource *source = QgsProcessingParameters::parameterAsSource( def, parameters, context );
152+
if ( source )
153+
{
154+
if ( foundCrs && source->sourceCrs().isValid() && crs != source->sourceCrs() )
155+
{
156+
return false;
157+
}
158+
else if ( !foundCrs && source->sourceCrs().isValid() )
159+
{
160+
foundCrs = true;
161+
crs = source->sourceCrs();
162+
}
163+
}
164+
}
165+
else if ( def->type() == QStringLiteral( "multilayer" ) )
166+
{
167+
QList< QgsMapLayer *> layers = QgsProcessingParameters::parameterAsLayerList( def, parameters, context );
168+
Q_FOREACH ( QgsMapLayer *layer, layers )
169+
{
170+
if ( !layer )
171+
continue;
172+
173+
if ( foundCrs && layer->crs().isValid() && crs != layer->crs() )
174+
{
175+
return false;
176+
}
177+
else if ( !foundCrs && layer->crs().isValid() )
178+
{
179+
foundCrs = true;
180+
crs = layer->crs();
181+
}
182+
}
183+
}
184+
}
185+
return true;
186+
}
187+
120188
bool QgsProcessingAlgorithm::addParameter( QgsProcessingParameterDefinition *definition )
121189
{
122190
if ( !definition )

‎src/core/processing/qgsprocessingalgorithm.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
4949
FlagHideFromModeler = 1 << 2, //!< Algorithm should be hidden from the modeler
5050
FlagSupportsBatch = 1 << 3, //!< Algorithm supports batch mode
5151
FlagCanCancel = 1 << 4, //!< Algorithm can be canceled
52+
FlagRequiresMatchingCrs = 1 << 5, //!< Algorithm requires that all input layers have matching coordinate reference systems
5253
FlagDeprecated = FlagHideFromToolbox | FlagHideFromModeler, //!< Algorithm is deprecated
5354
};
5455
Q_DECLARE_FLAGS( Flags, Flag )
@@ -243,6 +244,14 @@ class CORE_EXPORT QgsProcessingAlgorithm
243244
QgsExpressionContext createExpressionContext( const QVariantMap &parameters,
244245
QgsProcessingContext &context ) const;
245246

247+
/**
248+
* Checks whether the coordinate reference systems for the specified set of \a parameters
249+
* are valid for the algorithm. For instance, the base implementation performs
250+
* checks to ensure that all input CRS are equal
251+
* Returns true if \a parameters have passed the CRS check.
252+
*/
253+
virtual bool validateInputCrs( const QVariantMap &parameters,
254+
QgsProcessingContext &context ) const;
246255

247256
protected:
248257

‎tests/src/core/testqgsprocessing.cpp

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,19 @@ class DummyAlgorithm : public QgsProcessingAlgorithm
3636
{
3737
public:
3838

39-
DummyAlgorithm( const QString &name ) : mName( name ) {}
39+
DummyAlgorithm( const QString &name ) : mName( name ) { mFlags = QgsProcessingAlgorithm::flags(); }
4040

4141
QString name() const override { return mName; }
4242
QString displayName() const override { return mName; }
4343
virtual QVariantMap processAlgorithm( const QVariantMap &,
4444
QgsProcessingContext &, QgsProcessingFeedback * ) const override { return QVariantMap(); }
45+
46+
virtual Flags flags() const override { return mFlags; }
47+
4548
QString mName;
4649

50+
Flags mFlags;
51+
4752
void checkParameterVals()
4853
{
4954
addParameter( new QgsProcessingParameterString( "p1" ) );
@@ -126,6 +131,62 @@ class DummyAlgorithm : public QgsProcessingAlgorithm
126131
QVERIFY( hasHtmlOutputs() );
127132
}
128133

134+
void runValidateInputCrsChecks()
135+
{
136+
addParameter( new QgsProcessingParameterMapLayer( "p1" ) );
137+
addParameter( new QgsProcessingParameterMapLayer( "p2" ) );
138+
QVariantMap parameters;
139+
140+
QgsVectorLayer *layer3111 = new QgsVectorLayer( "Point?crs=epsg:3111", "v1", "memory" );
141+
QgsProject p;
142+
p.addMapLayer( layer3111 );
143+
144+
QString testDataDir = QStringLiteral( TEST_DATA_DIR ) + '/'; //defined in CmakeLists.txt
145+
QString raster1 = testDataDir + "tenbytenraster.asc";
146+
QFileInfo fi1( raster1 );
147+
QgsRasterLayer *r1 = new QgsRasterLayer( fi1.filePath(), "R1" );
148+
QVERIFY( r1->isValid() );
149+
p.addMapLayer( r1 );
150+
151+
QgsVectorLayer *layer4326 = new QgsVectorLayer( "Point?crs=epsg:4326", "v1", "memory" );
152+
p.addMapLayer( layer4326 );
153+
154+
QgsProcessingContext context;
155+
context.setProject( &p );
156+
157+
// flag not set
158+
mFlags = 0;
159+
parameters.insert( "p1", QVariant::fromValue( layer3111 ) );
160+
QVERIFY( validateInputCrs( parameters, context ) );
161+
mFlags = FlagRequiresMatchingCrs;
162+
QVERIFY( validateInputCrs( parameters, context ) );
163+
164+
// two layers, different crs
165+
parameters.insert( "p2", QVariant::fromValue( layer4326 ) );
166+
// flag not set
167+
mFlags = 0;
168+
QVERIFY( validateInputCrs( parameters, context ) );
169+
mFlags = FlagRequiresMatchingCrs;
170+
QVERIFY( !validateInputCrs( parameters, context ) );
171+
172+
// raster layer
173+
parameters.remove( "p2" );
174+
addParameter( new QgsProcessingParameterRasterLayer( "p3" ) );
175+
parameters.insert( "p3", QVariant::fromValue( r1 ) );
176+
QVERIFY( !validateInputCrs( parameters, context ) );
177+
178+
// feature source
179+
parameters.remove( "p3" );
180+
addParameter( new QgsProcessingParameterFeatureSource( "p4" ) );
181+
parameters.insert( "p4", layer4326->id() );
182+
QVERIFY( !validateInputCrs( parameters, context ) );
183+
184+
parameters.remove( "p4" );
185+
addParameter( new QgsProcessingParameterMultipleLayers( "p5" ) );
186+
parameters.insert( "p5", QVariantList() << layer4326->id() << r1->id() );
187+
QVERIFY( !validateInputCrs( parameters, context ) );
188+
}
189+
129190
};
130191

131192
//dummy provider for testing
@@ -228,6 +289,7 @@ class TestQgsProcessing: public QObject
228289
void processingFeatureSource();
229290
void processingFeatureSink();
230291
void algorithmScope();
292+
void validateInputCrs();
231293

232294
private:
233295

@@ -2640,5 +2702,11 @@ void TestQgsProcessing::algorithmScope()
26402702
QCOMPARE( exp2.evaluate( &context ).toInt(), 5 );
26412703
}
26422704

2705+
void TestQgsProcessing::validateInputCrs()
2706+
{
2707+
DummyAlgorithm alg( "test" );
2708+
alg.runValidateInputCrsChecks();
2709+
}
2710+
26432711
QGSTEST_MAIN( TestQgsProcessing )
26442712
#include "testqgsprocessing.moc"

0 commit comments

Comments
 (0)
Please sign in to comment.