From 39166282128449bad29dc2bdc9c21c250805fea0 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 7 Nov 2017 10:10:06 +1000 Subject: [PATCH] Fix handling of transform-not-required in QgsCoordinateTransformContext --- python/core/qgscoordinatetransformcontext.sip | 44 ++++++++++- src/core/qgscoordinatetransformcontext.cpp | 42 +++++----- src/core/qgscoordinatetransformcontext.h | 44 ++++++++++- .../test_qgscoordinatetransformcontext.py | 78 ++++++++++++------- 4 files changed, 150 insertions(+), 58 deletions(-) diff --git a/python/core/qgscoordinatetransformcontext.sip b/python/core/qgscoordinatetransformcontext.sip index ce589d2db5a..d0ef9eac7bd 100644 --- a/python/core/qgscoordinatetransformcontext.sip +++ b/python/core/qgscoordinatetransformcontext.sip @@ -66,6 +66,9 @@ class QgsCoordinateTransformContext Returns the stored mapping for source CRS to associated datum transform to use. The map keys will be QgsCoordinateReferenceSystems.authid()s. + A datum transform of -1 indicates that no datum transform is required for the + source CRS. + \warning This method should not be used to calculate the corresponding datum transforms to use for a coordinate transform. Instead, always use calculateDatumTransforms() to determine this. @@ -80,7 +83,8 @@ class QgsCoordinateTransformContext Adds a new ``transform`` to use when projecting coordinates from the specified source ``crs``. - If ``transform`` is -1, then any existing source transform for the ``crs`` will be removed. + A datum ``transform`` of -1 indicates that no datum transform is required for the + source CRS. Returns true if the new transform was added successfully. @@ -89,14 +93,25 @@ class QgsCoordinateTransformContext .. seealso:: sourceDatumTransforms() .. seealso:: addDestinationDatumTransform() +.. seealso:: removeSourceDatumTransform() :rtype: bool %End + void removeSourceDatumTransform( const QgsCoordinateReferenceSystem &crs ); +%Docstring + Removes the source datum transform for the specified ``crs``. +.. seealso:: addSourceDatumTransform() +.. seealso:: removeDestinationDatumTransform() +%End + QMap< QString, int > destinationDatumTransforms() const; %Docstring Returns the stored mapping for destination CRS to associated datum transform to use. The map keys will be QgsCoordinateReferenceSystems.authid()s. + A datum transform of -1 indicates that no datum transform is required for the + destination CRS. + \warning This method should not be used to calculate the corresponding datum transforms to use for a coordinate transform. Instead, always use calculateDatumTransforms() to determine this. @@ -111,7 +126,8 @@ class QgsCoordinateTransformContext Adds a new ``transform`` to use when projecting coordinates to the specified destination ``crs``. - If ``transform`` is -1, then any existing destination transform for the ``crs`` will be removed. + A datum ``transform`` of -1 indicates that no datum transform is required for the + destination CRS. Returns true if the new transform was added successfully. @@ -120,14 +136,25 @@ class QgsCoordinateTransformContext .. seealso:: destinationDatumTransforms() .. seealso:: addSourceDatumTransform() +.. seealso:: removeDestinationDatumTransform() :rtype: bool %End + void removeDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs ); +%Docstring + Removes the destination datum transform for the specified ``crs``. +.. seealso:: addDestinationDatumTransform() +.. seealso:: removeSourceDatumTransform() +%End + QMap< QPair< QString, QString>, QPair< int, int > > sourceDestinationDatumTransforms() const; %Docstring Returns the stored mapping for source to destination CRS pairs to associated datum transforms to use. The map keys will be QgsCoordinateReferenceSystems.authid()s. + If either the source transform or destination transform is -1, then no datum transform is + required for transformations for that source or destination. + \warning This method should not be used to calculate the corresponding datum transforms to use for a coordinate transform. Instead, always use calculateDatumTransforms() to determine this. @@ -144,8 +171,8 @@ class QgsCoordinateTransformContext Adds a new ``sourceTransform`` and ``destinationTransform`` to use when projecting coordinates from the the specified ``sourceCrs`` to the specified ``destinationCrs``. - If either ``sourceTransform`` or ``destinationTransform`` is -1, then any existing source to destination - transform for the crs pair will be removed. + If either ``sourceTransform`` or ``destinationTransform`` is -1, then no datum transform is + required for transformations for that source or destination. Returns true if the new transform pair was added successfully. @@ -155,9 +182,18 @@ class QgsCoordinateTransformContext transforms set by addSourceDatumTransform() or addDestinationDatumTransform(). .. seealso:: sourceDestinationDatumTransforms() +.. seealso:: removeSourceDestinationDatumTransform() :rtype: bool %End + void removeSourceDestinationDatumTransform( const QgsCoordinateReferenceSystem &sourceCrs, + const QgsCoordinateReferenceSystem &destinationCrs ); +%Docstring + Removes the source to destination datum transform pair for the specified ``sourceCrs`` and + ``destinationCrs``. +.. seealso:: addSourceDestinationDatumTransform() +%End + QPair< int, int > calculateDatumTransforms( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const; %Docstring diff --git a/src/core/qgscoordinatetransformcontext.cpp b/src/core/qgscoordinatetransformcontext.cpp index 697ba58a1f0..15f3aec64ca 100644 --- a/src/core/qgscoordinatetransformcontext.cpp +++ b/src/core/qgscoordinatetransformcontext.cpp @@ -58,18 +58,16 @@ bool QgsCoordinateTransformContext::addSourceDatumTransform( const QgsCoordinate d.detach(); d->mLock.lockForWrite(); - if ( transform == -1 ) - { - d->mSourceDatumTransforms.remove( crs.authid() ); - } - else - { - d->mSourceDatumTransforms.insert( crs.authid(), transform ); - } + d->mSourceDatumTransforms.insert( crs.authid(), transform ); d->mLock.unlock(); return true; } +void QgsCoordinateTransformContext::removeSourceDatumTransform( const QgsCoordinateReferenceSystem &crs ) +{ + d->mSourceDatumTransforms.remove( crs.authid() ); +} + QMap QgsCoordinateTransformContext::destinationDatumTransforms() const { d->mLock.lockForRead(); @@ -87,18 +85,16 @@ bool QgsCoordinateTransformContext::addDestinationDatumTransform( const QgsCoord d.detach(); d->mLock.lockForWrite(); - if ( transform == -1 ) - { - d->mDestDatumTransforms.remove( crs.authid() ); - } - else - { - d->mDestDatumTransforms.insert( crs.authid(), transform ); - } + d->mDestDatumTransforms.insert( crs.authid(), transform ); d->mLock.unlock(); return true; } +void QgsCoordinateTransformContext::removeDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs ) +{ + d->mDestDatumTransforms.remove( crs.authid() ); +} + QMap, QPair > QgsCoordinateTransformContext::sourceDestinationDatumTransforms() const { d->mLock.lockForRead(); @@ -115,18 +111,16 @@ bool QgsCoordinateTransformContext::addSourceDestinationDatumTransform( const Qg d.detach(); d->mLock.lockForWrite(); - if ( sourceTransform == -1 && destinationTransform == -1 ) - { - d->mSourceDestDatumTransforms.remove( qMakePair( sourceCrs.authid(), destinationCrs.authid() ) ); - } - else - { - d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) ); - } + d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) ); d->mLock.unlock(); return true; } +void QgsCoordinateTransformContext::removeSourceDestinationDatumTransform( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs ) +{ + d->mSourceDestDatumTransforms.remove( qMakePair( sourceCrs.authid(), destinationCrs.authid() ) ); +} + QPair QgsCoordinateTransformContext::calculateDatumTransforms( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const { QString srcKey = source.authid(); diff --git a/src/core/qgscoordinatetransformcontext.h b/src/core/qgscoordinatetransformcontext.h index 375210347cd..4d056aac435 100644 --- a/src/core/qgscoordinatetransformcontext.h +++ b/src/core/qgscoordinatetransformcontext.h @@ -82,6 +82,9 @@ class CORE_EXPORT QgsCoordinateTransformContext * Returns the stored mapping for source CRS to associated datum transform to use. * The map keys will be QgsCoordinateReferenceSystems::authid()s. * + * A datum transform of -1 indicates that no datum transform is required for the + * source CRS. + * * \warning This method should not be used to calculate the corresponding datum transforms * to use for a coordinate transform. Instead, always use calculateDatumTransforms() * to determine this. @@ -95,7 +98,8 @@ class CORE_EXPORT QgsCoordinateTransformContext * Adds a new \a transform to use when projecting coordinates from the specified source * \a crs. * - * If \a transform is -1, then any existing source transform for the \a crs will be removed. + * A datum \a transform of -1 indicates that no datum transform is required for the + * source CRS. * * Returns true if the new transform was added successfully. * @@ -104,13 +108,24 @@ class CORE_EXPORT QgsCoordinateTransformContext * * \see sourceDatumTransforms() * \see addDestinationDatumTransform() + * \see removeSourceDatumTransform() */ bool addSourceDatumTransform( const QgsCoordinateReferenceSystem &crs, int transform ); + /** + * Removes the source datum transform for the specified \a crs. + * \see addSourceDatumTransform() + * \see removeDestinationDatumTransform() + */ + void removeSourceDatumTransform( const QgsCoordinateReferenceSystem &crs ); + /** * Returns the stored mapping for destination CRS to associated datum transform to use. * The map keys will be QgsCoordinateReferenceSystems::authid()s. * + * A datum transform of -1 indicates that no datum transform is required for the + * destination CRS. + * * \warning This method should not be used to calculate the corresponding datum transforms * to use for a coordinate transform. Instead, always use calculateDatumTransforms() * to determine this. @@ -124,7 +139,8 @@ class CORE_EXPORT QgsCoordinateTransformContext * Adds a new \a transform to use when projecting coordinates to the specified destination * \a crs. * - * If \a transform is -1, then any existing destination transform for the \a crs will be removed. + * A datum \a transform of -1 indicates that no datum transform is required for the + * destination CRS. * * Returns true if the new transform was added successfully. * @@ -133,13 +149,24 @@ class CORE_EXPORT QgsCoordinateTransformContext * * \see destinationDatumTransforms() * \see addSourceDatumTransform() + * \see removeDestinationDatumTransform() */ bool addDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs, int transform ); + /** + * Removes the destination datum transform for the specified \a crs. + * \see addDestinationDatumTransform() + * \see removeSourceDatumTransform() + */ + void removeDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs ); + /** * Returns the stored mapping for source to destination CRS pairs to associated datum transforms to use. * The map keys will be QgsCoordinateReferenceSystems::authid()s. * + * If either the source transform or destination transform is -1, then no datum transform is + * required for transformations for that source or destination. + * * \warning This method should not be used to calculate the corresponding datum transforms * to use for a coordinate transform. Instead, always use calculateDatumTransforms() * to determine this. @@ -152,8 +179,8 @@ class CORE_EXPORT QgsCoordinateTransformContext * Adds a new \a sourceTransform and \a destinationTransform to use when projecting coordinates * from the the specified \a sourceCrs to the specified \a destinationCrs. * - * If either \a sourceTransform or \a destinationTransform is -1, then any existing source to destination - * transform for the crs pair will be removed. + * If either \a sourceTransform or \a destinationTransform is -1, then no datum transform is + * required for transformations for that source or destination. * * Returns true if the new transform pair was added successfully. * @@ -161,12 +188,21 @@ class CORE_EXPORT QgsCoordinateTransformContext * transforms set by addSourceDatumTransform() or addDestinationDatumTransform(). * * \see sourceDestinationDatumTransforms() + * \see removeSourceDestinationDatumTransform() */ bool addSourceDestinationDatumTransform( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, int sourceTransform, int destinationTransform ); + /** + * Removes the source to destination datum transform pair for the specified \a sourceCrs and + * \a destinationCrs. + * \see addSourceDestinationDatumTransform() + */ + void removeSourceDestinationDatumTransform( const QgsCoordinateReferenceSystem &sourceCrs, + const QgsCoordinateReferenceSystem &destinationCrs ); + /** * Returns the pair of source and destination datum transforms to use * for a transform from the specified \a source CRS to \a destination CRS. diff --git a/tests/src/python/test_qgscoordinatetransformcontext.py b/tests/src/python/test_qgscoordinatetransformcontext.py index 280efb0730c..95bb5958453 100644 --- a/tests/src/python/test_qgscoordinatetransformcontext.py +++ b/tests/src/python/test_qgscoordinatetransformcontext.py @@ -43,13 +43,17 @@ class TestQgsCoordinateTransformContext(unittest.TestCase): self.assertFalse(context.addSourceDatumTransform(QgsCoordinateReferenceSystem(), 4)) self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3}) - # removing non-existing + # indicate no transform required self.assertTrue(context.addSourceDatumTransform(QgsCoordinateReferenceSystem(28357), -1)) - self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3}) + self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3, 'EPSG:28357': -1}) + + # removing non-existing + context.removeSourceDatumTransform(QgsCoordinateReferenceSystem(28354)) + self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3, 'EPSG:28357': -1}) # remove existing - self.assertTrue(context.addSourceDatumTransform(QgsCoordinateReferenceSystem(28356), -1)) - self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1}) + context.removeSourceDatumTransform(QgsCoordinateReferenceSystem(28356)) + self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28357': -1}) context.clear() self.assertEqual(context.sourceDatumTransforms(), {}) @@ -70,13 +74,17 @@ class TestQgsCoordinateTransformContext(unittest.TestCase): self.assertFalse(context.addDestinationDatumTransform(QgsCoordinateReferenceSystem(), 4)) self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3}) - # removing non-existing + # indicate no transform required self.assertTrue(context.addDestinationDatumTransform(QgsCoordinateReferenceSystem(28357), -1)) - self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3}) + self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3, 'EPSG:28357': -1}) + + # removing non-existing + context.removeSourceDatumTransform(QgsCoordinateReferenceSystem(28354)) + self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28356': 3, 'EPSG:28357': -1}) # remove existing - self.assertTrue(context.addDestinationDatumTransform(QgsCoordinateReferenceSystem(28356), -1)) - self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1}) + context.removeDestinationDatumTransform(QgsCoordinateReferenceSystem(28356)) + self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1, 'EPSG:28357': -1}) context.clear() self.assertEqual(context.destinationDatumTransforms(), {}) @@ -114,33 +122,51 @@ class TestQgsCoordinateTransformContext(unittest.TestCase): ('EPSG:28356', 'EPSG:4283'): (3, 4), ('EPSG:28356', 'EPSG:28357'): (9, 11)}) - # removing non-existing + # indicate no transform required self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(28357), QgsCoordinateReferenceSystem(28356), -1, -1)) self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (1, 2), ('EPSG:28356', 'EPSG:4283'): (3, 4), - ('EPSG:28356', 'EPSG:28357'): (9, 11)}) + ('EPSG:28356', 'EPSG:28357'): (9, 11), + ('EPSG:28357', 'EPSG:28356'): (-1, -1)}) self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3111), - QgsCoordinateReferenceSystem(28356), -1, -1)) + QgsCoordinateReferenceSystem(28356), 17, -1)) self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (1, 2), ('EPSG:28356', 'EPSG:4283'): (3, 4), - ('EPSG:28356', 'EPSG:28357'): (9, 11)}) + ('EPSG:28356', 'EPSG:28357'): (9, 11), + ('EPSG:28357', 'EPSG:28356'): (-1, -1), + ('EPSG:3111', 'EPSG:28356'): (17, -1)}) + self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3113), + QgsCoordinateReferenceSystem(28356), -1, 18)) + self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (1, 2), + ('EPSG:28356', 'EPSG:4283'): (3, 4), + ('EPSG:28356', 'EPSG:28357'): (9, 11), + ('EPSG:28357', 'EPSG:28356'): (-1, -1), + ('EPSG:3111', 'EPSG:28356'): (17, -1), + ('EPSG:3113', 'EPSG:28356'): (-1, 18)}) + # remove non-existing + context.removeSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3113), QgsCoordinateReferenceSystem(3111)) + self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (1, 2), + ('EPSG:28356', 'EPSG:4283'): (3, 4), + ('EPSG:28356', 'EPSG:28357'): (9, 11), + ('EPSG:28357', 'EPSG:28356'): (-1, -1), + ('EPSG:3111', 'EPSG:28356'): (17, -1), + ('EPSG:3113', 'EPSG:28356'): (-1, 18)}) # remove existing - self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3111), - QgsCoordinateReferenceSystem(4283), -1, 3)) - self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (-1, 3), - ('EPSG:28356', 'EPSG:4283'): (3, 4), - ('EPSG:28356', 'EPSG:28357'): (9, 11)}) - self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(28356), - QgsCoordinateReferenceSystem(28357), 1, -1)) - self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (-1, 3), - ('EPSG:28356', 'EPSG:4283'): (3, 4), - ('EPSG:28356', 'EPSG:28357'): (1, -1)}) - self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(28356), - QgsCoordinateReferenceSystem(28357), -1, -1)) - self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (-1, 3), - ('EPSG:28356', 'EPSG:4283'): (3, 4)}) + context.removeSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3111), + QgsCoordinateReferenceSystem(4283)) + self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:28356', 'EPSG:4283'): (3, 4), + ('EPSG:28356', 'EPSG:28357'): (9, 11), + ('EPSG:28357', 'EPSG:28356'): (-1, -1), + ('EPSG:3111', 'EPSG:28356'): (17, -1), + ('EPSG:3113', 'EPSG:28356'): (-1, 18)}) + context.removeSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3111), + QgsCoordinateReferenceSystem(28356)) + self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:28356', 'EPSG:4283'): (3, 4), + ('EPSG:28356', 'EPSG:28357'): (9, 11), + ('EPSG:28357', 'EPSG:28356'): (-1, -1), + ('EPSG:3113', 'EPSG:28356'): (-1, 18)}) context.clear() self.assertEqual(context.sourceDestinationDatumTransforms(), {})