QgsCoordinateTransformContext is implicitly shared

qgis · Dec 15, 2017 · 5131258 · 5131258
1 parent f7b315f
commit 5131258
Show file tree

Hide file tree

Showing 5 changed files with 176 additions and 25 deletions.
diff --git a/python/core/qgscoordinatetransformcontext.sip b/python/core/qgscoordinatetransformcontext.sip
@@ -10,6 +10,7 @@
 
 
 
+
 class QgsCoordinateTransformContext
 {
 %Docstring
@@ -26,6 +27,10 @@ class QgsCoordinateTransformContext
  addSourceDatumTransform() then this datum transform will be used. The same logic
  applies for destination CRS transforms set using addDestinationDatumTransform().
 
+.. note::
+
+   QgsCoordinateTransformContext objects are implicitly shared.
+
 .. versionadded:: 3.0
 %End
 
@@ -39,6 +44,13 @@ class QgsCoordinateTransformContext
  Constructor for QgsCoordinateTransformContext.
 %End
 
+    QgsCoordinateTransformContext( const QgsCoordinateTransformContext &rhs );
+%Docstring
+ Copy constructor
+%End
+
+
+
     void clear();
 %Docstring
  Clears all stored transform information from the context.

diff --git a/src/core/qgscoordinatetransformcontext.cpp b/src/core/qgscoordinatetransformcontext.cpp
@@ -16,71 +16,89 @@
  ***************************************************************************/
 
 #include "qgscoordinatetransformcontext.h"
+#include "qgscoordinatetransformcontext_p.h"
+QgsCoordinateTransformContext::QgsCoordinateTransformContext()
+  : d( new QgsCoordinateTransformContextPrivate() )
+{}
+
+QgsCoordinateTransformContext::QgsCoordinateTransformContext( const QgsCoordinateTransformContext &rhs ) //NOLINT
+  : d( rhs.d )
+{}
+
+QgsCoordinateTransformContext &QgsCoordinateTransformContext::operator=( const QgsCoordinateTransformContext &rhs ) //NOLINT
+{
+  d = rhs.d;
+  return *this;
+}
 
 void QgsCoordinateTransformContext::clear()
 {
-  mSourceDestDatumTransforms.clear();
-  mSourceDatumTransforms.clear();
-  mDestDatumTransforms.clear();
+  d.detach();
+  d->mSourceDestDatumTransforms.clear();
+  d->mSourceDatumTransforms.clear();
+  d->mDestDatumTransforms.clear();
 }
 
 QMap<QString, int> QgsCoordinateTransformContext::sourceDatumTransforms() const
 {
-  return mSourceDatumTransforms;
+  return d->mSourceDatumTransforms;
 }
 
 bool QgsCoordinateTransformContext::addSourceDatumTransform( const QgsCoordinateReferenceSystem &crs, int transform )
 {
   if ( !crs.isValid() )
     return false;
 
+  d.detach();
   if ( transform == -1 )
   {
-    mSourceDatumTransforms.remove( crs.authid() );
+    d->mSourceDatumTransforms.remove( crs.authid() );
     return true;
   }
 
-  mSourceDatumTransforms.insert( crs.authid(), transform );
+  d->mSourceDatumTransforms.insert( crs.authid(), transform );
   return true;
 }
 
 QMap<QString, int> QgsCoordinateTransformContext::destinationDatumTransforms() const
 {
-  return mDestDatumTransforms;
+  return d->mDestDatumTransforms;
 }
 
 bool QgsCoordinateTransformContext::addDestinationDatumTransform( const QgsCoordinateReferenceSystem &crs, int transform )
 {
   if ( !crs.isValid() )
     return false;
 
+  d.detach();
   if ( transform == -1 )
   {
-    mDestDatumTransforms.remove( crs.authid() );
+    d->mDestDatumTransforms.remove( crs.authid() );
     return true;
   }
 
-  mDestDatumTransforms.insert( crs.authid(), transform );
+  d->mDestDatumTransforms.insert( crs.authid(), transform );
   return true;
 }
 
 QMap<QPair<QString, QString>, QPair<int, int> > QgsCoordinateTransformContext::sourceDestinationDatumTransforms() const
 {
-  return mSourceDestDatumTransforms;
+  return d->mSourceDestDatumTransforms;
 }
 
 bool QgsCoordinateTransformContext::addSourceDestinationDatumTransform( const QgsCoordinateReferenceSystem &sourceCrs, const QgsCoordinateReferenceSystem &destinationCrs, int sourceTransform, int destinationTransform )
 {
   if ( !sourceCrs.isValid() || !destinationCrs.isValid() )
     return false;
 
+  d.detach();
   if ( sourceTransform == -1 || destinationTransform == -1 )
   {
-    mSourceDestDatumTransforms.remove( qMakePair( sourceCrs.authid(), destinationCrs.authid() ) );
+    d->mSourceDestDatumTransforms.remove( qMakePair( sourceCrs.authid(), destinationCrs.authid() ) );
     return true;
   }
 
-  mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) );
+  d->mSourceDestDatumTransforms.insert( qMakePair( sourceCrs.authid(), destinationCrs.authid() ), qMakePair( sourceTransform, destinationTransform ) );
   return true;
 }
 

diff --git a/src/core/qgscoordinatetransformcontext.h b/src/core/qgscoordinatetransformcontext.h
@@ -20,7 +20,13 @@
 
 #include "qgis_core.h"
 #include "qgis.h"
-#include "qgscoordinatereferencesystem.h"
+#include "qgscoordinatetransformcontext_p.h"
+
+/***************************************************************************
+ * This class is considered CRITICAL and any change MUST be accompanied with
+ * full unit tests in testqgsfeature.cpp.
+ * See details in QEP #17
+ ****************************************************************************/
 
 /**
  * \class QgsCoordinateTransformContext
@@ -38,6 +44,8 @@
  * addSourceDatumTransform() then this datum transform will be used. The same logic
  * applies for destination CRS transforms set using addDestinationDatumTransform().
  *
+ * \note QgsCoordinateTransformContext objects are implicitly shared.
+ *
  * \since QGIS 3.0
 */
 
@@ -48,7 +56,18 @@ class CORE_EXPORT QgsCoordinateTransformContext
     /**
      * Constructor for QgsCoordinateTransformContext.
      */
-    QgsCoordinateTransformContext() = default;
+    QgsCoordinateTransformContext();
+
+    /**
+     * Copy constructor
+     */
+    QgsCoordinateTransformContext( const QgsCoordinateTransformContext &rhs );
+
+    /**
+     * Assignment operator
+     */
+    QgsCoordinateTransformContext &operator=( const QgsCoordinateTransformContext &rhs ) SIP_SKIP;
+
 
     /**
      * Clears all stored transform information from the context.
@@ -156,17 +175,7 @@ class CORE_EXPORT QgsCoordinateTransformContext
 
   private:
 
-    /**
-     * Mapping for datum transforms to use for source/destination CRS pairs.
-     * Matching records from this map will take precedence over other transform maps.
-     */
-    QMap< QPair< QString, QString >, QPair< int, int > > mSourceDestDatumTransforms;
-
-    //! Mapping for datum transforms to use for source CRS
-    QMap< QString, int > mSourceDatumTransforms;
-
-    //! Mapping for datum transforms to use for destination CRS
-    QMap< QString, int > mDestDatumTransforms;
+    QExplicitlySharedDataPointer<QgsCoordinateTransformContextPrivate> d;
 
 };
 

diff --git a/src/core/qgscoordinatetransformcontext_p.h b/src/core/qgscoordinatetransformcontext_p.h
@@ -0,0 +1,78 @@
+/***************************************************************************
+                         qgscoordinatetransformcontext_p.h
+                         -------------------------------
+    begin                : November 2017
+    copyright            : (C) 2017 by Nyall Dawson
+    email                : nyall dot dawson at gmail dot com
+ ***************************************************************************/
+
+/***************************************************************************
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ ***************************************************************************/
+
+#ifndef QGSCOORDINATETRANSFORMCONTEXT_PRIVATE_H
+#define QGSCOORDINATETRANSFORMCONTEXT_PRIVATE_H
+
+/// @cond PRIVATE
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the QGIS API.  It exists purely as an
+// implementation detail.  This header file may change from version to
+// version without notice, or even be removed.
+//
+
+/***************************************************************************
+ * This class is considered CRITICAL and any change MUST be accompanied with
+ * full unit tests in testqgsfeature.cpp.
+ * See details in QEP #17
+ ****************************************************************************/
+
+#include "qgscoordinatereferencesystem.h"
+
+
+class QgsCoordinateTransformContextPrivate : public QSharedData
+{
+
+  public:
+
+    QgsCoordinateTransformContextPrivate() = default;
+
+    QgsCoordinateTransformContextPrivate( const QgsCoordinateTransformContextPrivate &other )
+      : QSharedData( other )
+      , mSourceDestDatumTransforms( other.mSourceDestDatumTransforms )
+      , mSourceDatumTransforms( other.mSourceDatumTransforms )
+      , mDestDatumTransforms( other.mDestDatumTransforms )
+    {
+    }
+
+    /**
+     * Mapping for datum transforms to use for source/destination CRS pairs.
+     * Matching records from this map will take precedence over other transform maps.
+     */
+    QMap< QPair< QString, QString >, QPair< int, int > > mSourceDestDatumTransforms;
+
+    //! Mapping for datum transforms to use for source CRS
+    QMap< QString, int > mSourceDatumTransforms;
+
+    //! Mapping for datum transforms to use for destination CRS
+    QMap< QString, int > mDestDatumTransforms;
+
+};
+
+
+/// @endcond
+
+
+#endif // QGSCOORDINATETRANSFORMCONTEXT_PRIVATE_H
+
+
+
+
diff --git a/tests/src/core/testqgscoordinatetransform.cpp b/tests/src/core/testqgscoordinatetransform.cpp
@@ -17,6 +17,7 @@
 #include "qgscoordinatetransform.h"
 #include "qgsapplication.h"
 #include "qgsrectangle.h"
+#include "qgscoordinatetransformcontext.h"
 #include <QObject>
 #include "qgstest.h"
 #include "qgsexception.h"
@@ -33,6 +34,7 @@ class TestQgsCoordinateTransform: public QObject
     void assignment();
     void isValid();
     void isShortCircuited();
+    void contextShared();
 
   private:
 
@@ -178,6 +180,38 @@ void TestQgsCoordinateTransform::isShortCircuited()
   QVERIFY( tr5.isShortCircuited() );
 }
 
+void TestQgsCoordinateTransform::contextShared()
+{
+  //test implicit sharing of QgsCoordinateTransformContext
+  QgsCoordinateTransformContext original;
+  original.addDestinationDatumTransform( QgsCoordinateReferenceSystem( 3111 ), 1 );
+
+  QgsCoordinateTransformContext copy( original );
+  QMap< QString, int > expected;
+  expected.insert( "EPSG:3111", 1 );
+  QCOMPARE( original.destinationDatumTransforms(), expected );
+  QCOMPARE( copy.destinationDatumTransforms(), expected );
+
+  // trigger detach
+  copy.addDestinationDatumTransform( QgsCoordinateReferenceSystem( 3111 ), 2 );
+  QCOMPARE( original.destinationDatumTransforms(), expected );
+
+  expected.insert( "EPSG:3111", 2 );
+  QCOMPARE( copy.destinationDatumTransforms(), expected );
+
+  // copy via assignment
+  QgsCoordinateTransformContext copy2;
+  copy2 = original;
+  expected.insert( "EPSG:3111", 1 );
+  QCOMPARE( original.destinationDatumTransforms(), expected );
+  QCOMPARE( copy2.destinationDatumTransforms(), expected );
+
+  copy2.addDestinationDatumTransform( QgsCoordinateReferenceSystem( 3111 ), 3 );
+  QCOMPARE( original.destinationDatumTransforms(), expected );
+  expected.insert( "EPSG:3111", 3 );
+  QCOMPARE( copy2.destinationDatumTransforms(), expected );
+}
+
 
 void TestQgsCoordinateTransform::transformBoundingBox()
 {