From 61e6ab0ea823148f3a11603bd657b3e35a9184eb Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 17 Feb 2020 11:00:12 +1000 Subject: [PATCH] Add API to QgsCoordinateTransformContext to prevent use of fallback ballpark transforms for a source/dest CRS pair --- .../qgscoordinatetransformcontext.sip.in | 21 +++++- src/core/qgscoordinatetransformcontext.cpp | 68 ++++++++++++++----- src/core/qgscoordinatetransformcontext.h | 21 +++++- src/core/qgscoordinatetransformcontext_p.h | 13 +++- .../test_qgscoordinatetransformcontext.py | 43 ++++++++++-- 5 files changed, 141 insertions(+), 25 deletions(-) diff --git a/python/core/auto_generated/qgscoordinatetransformcontext.sip.in b/python/core/auto_generated/qgscoordinatetransformcontext.sip.in index 6d956410a2b2..2886c84c5634 100644 --- a/python/core/auto_generated/qgscoordinatetransformcontext.sip.in +++ b/python/core/auto_generated/qgscoordinatetransformcontext.sip.in @@ -125,7 +125,7 @@ Returns ``True`` if the new transform pair was added successfully. Has no effect on builds based on Proj 6.0 or later, use addCoordinateOperation() instead. %End - bool addCoordinateOperation( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, const QString &coordinateOperationProjString ); + bool addCoordinateOperation( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, const QString &coordinateOperationProjString, bool allowFallback = true ); %Docstring Adds a new ``coordinateOperationProjString`` to use when projecting coordinates from the specified ``sourceCrs`` to the specified ``destinationCrs``. @@ -134,6 +134,14 @@ from the specified ``sourceCrs`` to the specified ``destinationCrs``. string. If ``coordinateOperationProjString`` is empty, then the default Proj operation will be used when transforming between the coordinate reference systems. +If ``allowFallback`` is ``True`` (since QGIS 3.12), then "ballpark" fallback transformations +will be used in the case that the specified coordinate operation fails (such as when +coordinates from outside a required grid shift file are transformed). See +QgsCoordinateTransform.fallbackOperationOccurred() for further details. Note that if an +existing ``sourceCrs`` and ``destinationCrs`` pair are added with a different ``allowFallback`` +value, that value will replace the existing one (i.e. each combination of ``sourceCrs`` and +``destinationCrs`` must be unique). + .. warning:: coordinateOperationProjString MUST be a proj string which has been normalized for @@ -229,6 +237,17 @@ be used. .. versionadded:: 3.8 +%End + + bool allowFallbackTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const; +%Docstring +Returns ``True`` if approximate "ballpark" transforms may be used when transforming +between a \source and ``destination`` CRS pair, in the case that the preferred +coordinate operation fails (such as when +coordinates from outside a required grid shift file are transformed). See +QgsCoordinateTransform.fallbackOperationOccurred() for further details. + +.. versionadded:: 3.12 %End bool mustReverseCoordinateOperation( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const; diff --git a/src/core/qgscoordinatetransformcontext.cpp b/src/core/qgscoordinatetransformcontext.cpp index 1c8e5bd82ee5..22e3cd72539b 100644 --- a/src/core/qgscoordinatetransformcontext.cpp +++ b/src/core/qgscoordinatetransformcontext.cpp @@ -80,7 +80,11 @@ QMap, QString> QgsCoordinateTransformContext::coordinate auto res = d->mSourceDestDatumTransforms; res.detach(); d->mLock.unlock(); - return res; + QMap, QString> results; + for ( auto it = res.constBegin(); it != res.constEnd(); ++it ) + results.insert( it.key(), it.value().operation ); + + return results; #else return QMap, QString>(); #endif @@ -103,18 +107,22 @@ bool QgsCoordinateTransformContext::addSourceDestinationDatumTransform( const Qg #endif } -bool QgsCoordinateTransformContext::addCoordinateOperation( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, const QString &coordinateOperationProjString ) +bool QgsCoordinateTransformContext::addCoordinateOperation( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, const QString &coordinateOperationProjString, bool allowFallback ) { if ( !sourceCrs.isValid() || !destinationCrs.isValid() ) return false; #if PROJ_VERSION_MAJOR>=6 d.detach(); d->mLock.lockForWrite(); - d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), coordinateOperationProjString ); + QgsCoordinateTransformContextPrivate::OperationDetails details; + details.operation = coordinateOperationProjString; + details.allowFallback = allowFallback; + d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), details ); d->mLock.unlock(); return true; #else Q_UNUSED( coordinateOperationProjString ) + Q_UNUSED( allowFallback ) return false; #endif } @@ -174,14 +182,14 @@ QString QgsCoordinateTransformContext::calculateCoordinateOperation( const QgsCo const QString destKey = destination.authid(); d->mLock.lockForRead(); - QString res = d->mSourceDestDatumTransforms.value( qMakePair( srcKey, destKey ), QString() ); - if ( res.isEmpty() ) + QgsCoordinateTransformContextPrivate::OperationDetails res = d->mSourceDestDatumTransforms.value( qMakePair( srcKey, destKey ), QgsCoordinateTransformContextPrivate::OperationDetails() ); + if ( res.operation.isEmpty() ) { // try to reverse - res = d->mSourceDestDatumTransforms.value( qMakePair( destKey, srcKey ), QString() ); + res = d->mSourceDestDatumTransforms.value( qMakePair( destKey, srcKey ), QgsCoordinateTransformContextPrivate::OperationDetails() ); } d->mLock.unlock(); - return res; + return res.operation; #else Q_UNUSED( source ) Q_UNUSED( destination ) @@ -189,6 +197,23 @@ QString QgsCoordinateTransformContext::calculateCoordinateOperation( const QgsCo #endif } +bool QgsCoordinateTransformContext::allowFallbackTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const +{ +#if PROJ_VERSION_MAJOR>=6 + const QString srcKey = source.authid(); + const QString destKey = destination.authid(); + + d->mLock.lockForRead(); + QgsCoordinateTransformContextPrivate::OperationDetails res = d->mSourceDestDatumTransforms.value( qMakePair( srcKey, destKey ), QgsCoordinateTransformContextPrivate::OperationDetails() ); + d->mLock.unlock(); + return res.allowFallback; +#else + Q_UNUSED( source ) + Q_UNUSED( destination ) + return false; +#endif +} + bool QgsCoordinateTransformContext::mustReverseCoordinateOperation( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const { #if PROJ_VERSION_MAJOR>=6 @@ -196,15 +221,15 @@ bool QgsCoordinateTransformContext::mustReverseCoordinateOperation( const QgsCoo const QString destKey = destination.authid(); d->mLock.lockForRead(); - QString res = d->mSourceDestDatumTransforms.value( qMakePair( srcKey, destKey ), QString() ); - if ( !res.isEmpty() ) + QgsCoordinateTransformContextPrivate::OperationDetails res = d->mSourceDestDatumTransforms.value( qMakePair( srcKey, destKey ), QgsCoordinateTransformContextPrivate::OperationDetails() ); + if ( !res.operation.isEmpty() ) { d->mLock.unlock(); return false; } // see if the reverse operation is present - res = d->mSourceDestDatumTransforms.value( qMakePair( destKey, srcKey ), QString() ); - if ( !res.isEmpty() ) + res = d->mSourceDestDatumTransforms.value( qMakePair( destKey, srcKey ), QgsCoordinateTransformContextPrivate::OperationDetails() ); + if ( !res.operation.isEmpty() ) { d->mLock.unlock(); return true; @@ -248,6 +273,7 @@ bool QgsCoordinateTransformContext::readXml( const QDomElement &element, const Q #if PROJ_VERSION_MAJOR>=6 const QString coordinateOp = transformElem.attribute( QStringLiteral( "coordinateOp" ) ); + const bool allowFallback = transformElem.attribute( QStringLiteral( "allowFallback" ), QStringLiteral( "1" ) ).toInt(); // try to instantiate operation, and check for missing grids if ( !QgsProjUtils::coordinateOperationIsAvailable( coordinateOp ) ) @@ -257,7 +283,10 @@ bool QgsCoordinateTransformContext::readXml( const QDomElement &element, const Q result = false; } - d->mSourceDestDatumTransforms.insert( qMakePair( key1, key2 ), coordinateOp ); + QgsCoordinateTransformContextPrivate::OperationDetails deets; + deets.operation = coordinateOp; + deets.allowFallback = allowFallback; + d->mSourceDestDatumTransforms.insert( qMakePair( key1, key2 ), deets ); #else QString value1 = transformElem.attribute( QStringLiteral( "sourceTransform" ) ); QString value2 = transformElem.attribute( QStringLiteral( "destTransform" ) ); @@ -307,7 +336,8 @@ void QgsCoordinateTransformContext::writeXml( QDomElement &element, const QgsRea transformElem.setAttribute( QStringLiteral( "source" ), it.key().first ); transformElem.setAttribute( QStringLiteral( "dest" ), it.key().second ); #if PROJ_VERSION_MAJOR>=6 - transformElem.setAttribute( QStringLiteral( "coordinateOp" ), it.value() ); + transformElem.setAttribute( QStringLiteral( "coordinateOp" ), it.value().operation ); + transformElem.setAttribute( QStringLiteral( "allowFallback" ), it.value().allowFallback ? QStringLiteral( "1" ) : QStringLiteral( "0" ) ); #else Q_NOWARN_DEPRECATED_PUSH transformElem.setAttribute( QStringLiteral( "sourceTransform" ), it.value().sourceTransformId < 0 ? QString() : QgsDatumTransform::datumTransformToProj( it.value().sourceTransformId ) ); @@ -334,7 +364,7 @@ void QgsCoordinateTransformContext::readSettings() //collect src and dest entries that belong together #if PROJ_VERSION_MAJOR>=6 - QMap< QPair< QString, QString >, QString > transforms; + QMap< QPair< QString, QString >, QgsCoordinateTransformContextPrivate::OperationDetails > transforms; #else QMap< QPair< QString, QString >, QPair< int, int > > transforms; #endif @@ -356,7 +386,11 @@ void QgsCoordinateTransformContext::readSettings() } const QString proj = settings.value( *pkeyIt ).toString(); - transforms[ qMakePair( srcAuthId, destAuthId )] = proj; + const bool allowFallback = settings.value( QStringLiteral( "%1//%2_allowFallback" ).arg( srcAuthId, destAuthId ) ).toBool(); + QgsCoordinateTransformContextPrivate::OperationDetails deets; + deets.operation = proj; + deets.allowFallback = allowFallback; + transforms[ qMakePair( srcAuthId, destAuthId )] = deets; } #else if ( pkeyIt->contains( QLatin1String( "srcTransform" ) ) || pkeyIt->contains( QLatin1String( "destTransform" ) ) ) @@ -423,8 +457,10 @@ void QgsCoordinateTransformContext::writeSettings() const QString destAuthId = transformIt.key().second; #if PROJ_VERSION_MAJOR>=6 - const QString proj = transformIt.value(); + const QString proj = transformIt.value().operation; + const bool allowFallback = transformIt.value().allowFallback; settings.setValue( srcAuthId + "//" + destAuthId + "_coordinateOp", proj ); + settings.setValue( srcAuthId + "//" + destAuthId + "_allowFallback", allowFallback ); #else int sourceDatumTransform = transformIt.value().sourceTransformId; QString sourceDatumProj; diff --git a/src/core/qgscoordinatetransformcontext.h b/src/core/qgscoordinatetransformcontext.h index 364f913ebc80..f60c5f4d9572 100644 --- a/src/core/qgscoordinatetransformcontext.h +++ b/src/core/qgscoordinatetransformcontext.h @@ -140,6 +140,14 @@ class CORE_EXPORT QgsCoordinateTransformContext * string. If \a coordinateOperationProjString is empty, then the default Proj operation * will be used when transforming between the coordinate reference systems. * + * If \a allowFallback is TRUE (since QGIS 3.12), then "ballpark" fallback transformations + * will be used in the case that the specified coordinate operation fails (such as when + * coordinates from outside a required grid shift file are transformed). See + * QgsCoordinateTransform::fallbackOperationOccurred() for further details. Note that if an + * existing \a sourceCrs and \a destinationCrs pair are added with a different \a allowFallback + * value, that value will replace the existing one (i.e. each combination of \a sourceCrs and + * \a destinationCrs must be unique). + * * \warning coordinateOperationProjString MUST be a proj string which has been normalized for * visualization, and must be constructed so that coordinates are always input and output * with x/y coordinate ordering. (Proj strings output by utilities such as projinfo will NOT @@ -155,7 +163,7 @@ class CORE_EXPORT QgsCoordinateTransformContext * * \since QGIS 3.8 */ - bool addCoordinateOperation( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, const QString &coordinateOperationProjString ); + bool addCoordinateOperation( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, const QString &coordinateOperationProjString, bool allowFallback = true ); /** * Removes the source to destination datum transform pair for the specified \a sourceCrs and @@ -214,6 +222,17 @@ class CORE_EXPORT QgsCoordinateTransformContext */ QString calculateCoordinateOperation( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const; + /** + * Returns TRUE if approximate "ballpark" transforms may be used when transforming + * between a \source and \a destination CRS pair, in the case that the preferred + * coordinate operation fails (such as when + * coordinates from outside a required grid shift file are transformed). See + * QgsCoordinateTransform::fallbackOperationOccurred() for further details. + * + * \since QGIS 3.12 + */ + bool allowFallbackTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const; + /** * Returns TRUE if the coordinate operation returned by calculateCoordinateOperation() for the \a source to \a destination pair * must be inverted. diff --git a/src/core/qgscoordinatetransformcontext_p.h b/src/core/qgscoordinatetransformcontext_p.h index 60df8aca074b..5274fa55eba4 100644 --- a/src/core/qgscoordinatetransformcontext_p.h +++ b/src/core/qgscoordinatetransformcontext_p.h @@ -59,7 +59,18 @@ class QgsCoordinateTransformContextPrivate : public QSharedData * Mapping for coordinate operation Proj string to use for source/destination CRS pairs. */ #if PROJ_VERSION_MAJOR>=6 - QMap< QPair< QString, QString >, QString > mSourceDestDatumTransforms; + class OperationDetails + { + public: + QString operation; + bool allowFallback = true; + + bool operator==( const OperationDetails &other ) const + { + return operation == other.operation && allowFallback == other.allowFallback; + } + }; + QMap< QPair< QString, QString >, OperationDetails > mSourceDestDatumTransforms; #else QMap< QPair< QString, QString >, QgsDatumTransform::TransformPair > mSourceDestDatumTransforms; #endif diff --git a/tests/src/python/test_qgscoordinatetransformcontext.py b/tests/src/python/test_qgscoordinatetransformcontext.py index 5e6322974e40..7072b82580bb 100644 --- a/tests/src/python/test_qgscoordinatetransformcontext.py +++ b/tests/src/python/test_qgscoordinatetransformcontext.py @@ -161,6 +161,8 @@ def testSourceDestinationDatumTransformsProj6(self): self.assertEqual(context.coordinateOperations(), {('EPSG:3111', 'EPSG:4283'): proj_string}) self.assertTrue(context.mustReverseCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111'))) + self.assertTrue( + context.allowFallbackTransform(QgsCoordinateReferenceSystem('EPSG:3111'), QgsCoordinateReferenceSystem('EPSG:4283'))) self.assertTrue( context.hasTransform(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111'))) @@ -174,13 +176,18 @@ def testSourceDestinationDatumTransformsProj6(self): QgsCoordinateReferenceSystem('EPSG:3111')), proj_string) self.assertTrue(context.mustReverseCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111'))) + self.assertTrue( + context.allowFallbackTransform(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111'))) + proj_string_2 = 'dummy' self.assertTrue(context.addCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:4283'), - QgsCoordinateReferenceSystem('EPSG:3111'), proj_string_2)) + QgsCoordinateReferenceSystem('EPSG:3111'), proj_string_2, False)) self.assertEqual(context.calculateCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111')), proj_string_2) self.assertFalse(context.mustReverseCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111'))) + self.assertFalse( + context.allowFallbackTransform(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111'))) context.removeCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:4283'), QgsCoordinateReferenceSystem('EPSG:3111')) @@ -346,9 +353,9 @@ def testWriteReadXmlProj6(self): proj_2 = '+proj=pipeline +step +proj=axisswap +order=2,1 +step +proj=unitconvert +xy_in=deg +xy_out=rad +step +proj=push +v_3 +step +proj=cart +ellps=intl +step +proj=helmert +x=-150 +y=-250 +z=-1 +step +inv +proj=cart +ellps=WGS84 +step +proj=pop +v_3 +step +proj=unitconvert +xy_in=rad +xy_out=deg +step +proj=axisswap +order=2,1' self.assertTrue(context.addCoordinateOperation(QgsCoordinateReferenceSystem(4204), - QgsCoordinateReferenceSystem(4326), proj_1)) + QgsCoordinateReferenceSystem(4326), proj_1, True)) self.assertTrue(context.addCoordinateOperation(QgsCoordinateReferenceSystem(4205), - QgsCoordinateReferenceSystem(4326), proj_2)) + QgsCoordinateReferenceSystem(4326), proj_2, False)) self.assertEqual(context.coordinateOperations(), {('EPSG:4204', 'EPSG:4326'): proj_1, @@ -367,6 +374,10 @@ def testWriteReadXmlProj6(self): self.assertEqual(context2.coordinateOperations(), {('EPSG:4204', 'EPSG:4326'): proj_1, ('EPSG:4205', 'EPSG:4326'): proj_2}) + self.assertTrue(context2.allowFallbackTransform(QgsCoordinateReferenceSystem(4204), + QgsCoordinateReferenceSystem(4326))) + self.assertFalse(context2.allowFallbackTransform(QgsCoordinateReferenceSystem(4205), + QgsCoordinateReferenceSystem(4326))) @unittest.skipIf(QgsProjUtils.projVersionMajor() >= 6, 'Skipped on proj6 builds') def testMissingTransforms(self): @@ -443,11 +454,18 @@ def testProjectProj6(self): context_changed_spy = QSignalSpy(project.transformContextChanged) context = project.transformContext() context.addCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:3111'), - QgsCoordinateReferenceSystem('EPSG:4283'), 'proj') + QgsCoordinateReferenceSystem('EPSG:4283'), 'proj', True) project.setTransformContext(context) self.assertEqual(len(context_changed_spy), 1) self.assertEqual(project.transformContext().coordinateOperations(), {('EPSG:3111', 'EPSG:4283'): 'proj'}) + context2 = project.transformContext() + context2.addCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:3111'), + QgsCoordinateReferenceSystem('EPSG:4283'), 'proj', False) + project.setTransformContext(context2) + self.assertEqual(len(context_changed_spy), 2) + self.assertEqual(project.transformContext(), context2) + self.assertNotEqual(project.transformContext(), context) @unittest.skipIf(QgsProjUtils.projVersionMajor() >= 6, 'Skipped on proj6 builds') def testReadWriteSettings(self): @@ -503,13 +521,17 @@ def testReadWriteSettingsProj6(self): self.assertEqual(context.coordinateOperations(), {}) self.assertTrue(context.addCoordinateOperation(QgsCoordinateReferenceSystem(4204), - QgsCoordinateReferenceSystem(4326), proj_1)) + QgsCoordinateReferenceSystem(4326), proj_1, True)) self.assertTrue(context.addCoordinateOperation(QgsCoordinateReferenceSystem(4205), - QgsCoordinateReferenceSystem(4326), proj_2)) + QgsCoordinateReferenceSystem(4326), proj_2, False)) self.assertEqual(context.coordinateOperations(), {('EPSG:4204', 'EPSG:4326'): proj_1, ('EPSG:4205', 'EPSG:4326'): proj_2}) + self.assertTrue(context.allowFallbackTransform(QgsCoordinateReferenceSystem(4204), + QgsCoordinateReferenceSystem(4326))) + self.assertFalse(context.allowFallbackTransform(QgsCoordinateReferenceSystem(4205), + QgsCoordinateReferenceSystem(4326))) # save to settings context.writeSettings() @@ -524,6 +546,11 @@ def testReadWriteSettingsProj6(self): {('EPSG:4204', 'EPSG:4326'): proj_1, ('EPSG:4205', 'EPSG:4326'): proj_2}) + self.assertTrue(context2.allowFallbackTransform(QgsCoordinateReferenceSystem(4204), + QgsCoordinateReferenceSystem(4326))) + self.assertFalse(context2.allowFallbackTransform(QgsCoordinateReferenceSystem(4205), + QgsCoordinateReferenceSystem(4326))) + @unittest.skipIf(QgsProjUtils.projVersionMajor() >= 6, 'Skipped on proj6 builds') def testEqualOperator(self): context1 = QgsCoordinateTransformContext() @@ -552,6 +579,10 @@ def testEqualOperatorProj6(self): QgsCoordinateReferenceSystem('EPSG:4283'), 'p1') self.assertTrue(context1 == context2) + context2.addCoordinateOperation(QgsCoordinateReferenceSystem('EPSG:3111'), + QgsCoordinateReferenceSystem('EPSG:4283'), 'p1', False) + self.assertFalse(context1 == context2) + if __name__ == '__main__': unittest.main()