Fix handling of transform-not-required in QgsCoordinateTransformContext

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
+ If either ``sourceTransform`` or ``destinationTransform`` is -1, then no datum transform is
- transform for the crs pair will be removed.
+ 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

src/core/qgscoordinatetransformcontext.cpp

@@ -58,18 +58,16 @@ bool QgsCoordinateTransformContext::addSourceDatumTransform( const QgsCoordinate
 
   d.detach();
   d->mLock.lockForWrite();
-  if ( transform == -1 )
+  d->mSourceDatumTransforms.insert( crs.authid(), transform );
-  {
-    d->mSourceDatumTransforms.remove( crs.authid() );
-  }
-  else
-  {
-    d->mSourceDatumTransforms.insert( crs.authid(), transform );
-  }
   d->mLock.unlock();
   return true;
 }
 
+void QgsCoordinateTransformContext::removeSourceDatumTransform( const QgsCoordinateReferenceSystem &crs )
+{
+  d->mSourceDatumTransforms.remove( crs.authid() );
+}
+
 QMap<QString, int> 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.insert( crs.authid(), transform );
-  {
-    d->mDestDatumTransforms.remove( crs.authid() );
-  }
-  else
-  {
-    d->mDestDatumTransforms.insert( crs.authid(), transform );
-  }
   d->mLock.unlock();
   return true;
 }
 
+void QgsCoordinateTransformContext::removeDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs )
+{
+  d->mDestDatumTransforms.remove( crs.authid() );
+}
+
 QMap<QPair<QString, QString>, QPair<int, int> > 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.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) );
-  {
-    d->mSourceDestDatumTransforms.remove( qMakePair( sourceCrs.authid(), destinationCrs.authid() ) );
-  }
-  else
-  {
-    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<int, int> QgsCoordinateTransformContext::calculateDatumTransforms( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination ) const
 {
   QString srcKey = source.authid();

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
+     * If either \a sourceTransform or \a destinationTransform is -1, then no datum transform is
-     * transform for the crs pair will be removed.
+     * 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.

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))
+        context.removeSourceDatumTransform(QgsCoordinateReferenceSystem(28356))
-        self.assertEqual(context.sourceDatumTransforms(), {'EPSG:3111': 1})
+        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))
+        context.removeDestinationDatumTransform(QgsCoordinateReferenceSystem(28356))
-        self.assertEqual(context.destinationDatumTransforms(), {'EPSG:3111': 1})
+        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),
+        context.removeSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3111),
-                                                                   QgsCoordinateReferenceSystem(4283), -1, 3))
+                                                      QgsCoordinateReferenceSystem(4283))
-        self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (-1, 3),
+        self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:28356', 'EPSG:4283'): (3, 4),
-                                                                      ('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(28356),
+                                                                      ('EPSG:3111', 'EPSG:28356'): (17, -1),
-                                                                   QgsCoordinateReferenceSystem(28357), 1, -1))
+                                                                      ('EPSG:3113', 'EPSG:28356'): (-1, 18)})
-        self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (-1, 3),
+        context.removeSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(3111),
-                                                                      ('EPSG:28356', 'EPSG:4283'): (3, 4),
+                                                      QgsCoordinateReferenceSystem(28356))
-                                                                      ('EPSG:28356', 'EPSG:28357'): (1, -1)})
+        self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:28356', 'EPSG:4283'): (3, 4),
-        self.assertTrue(context.addSourceDestinationDatumTransform(QgsCoordinateReferenceSystem(28356),
+                                                                      ('EPSG:28356', 'EPSG:28357'): (9, 11),
-                                                                   QgsCoordinateReferenceSystem(28357), -1, -1))
+                                                                      ('EPSG:28357', 'EPSG:28356'): (-1, -1),
-        self.assertEqual(context.sourceDestinationDatumTransforms(), {('EPSG:3111', 'EPSG:4283'): (-1, 3),
+                                                                      ('EPSG:3113', 'EPSG:28356'): (-1, 18)})
-                                                                      ('EPSG:28356', 'EPSG:4283'): (3, 4)})
 
         context.clear()
         self.assertEqual(context.sourceDestinationDatumTransforms(), {})