Move caching to QgsCoordinateTransform

and remove no longer required QgsCoordinateTransformCache singleton
qgis · Dec 15, 2017 · cb693a7 · cb693a7
1 parent 439ef20
commit cb693a7
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 222 deletions.
diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -276,6 +276,7 @@ Use QgsComposerAttributeTableV2 instead.
 - QgsCRSCache was removed. QgsCoordinateReferenceSystem now internally uses a cache for CRS creation,
 so there is no longer a need for the separate cache class. Code which previously called QgsCRSCache::updateCRSCache()
 should now call QgsCoordinateReferenceSystem::invalidateCache() and QgsCoordinateTransformCache::instance()->invalidateCrs( authid ).
+- QgsCoordinateTransformCache was removed. QgsCoordinateTransform now transparently caches transforms.
 - QgsDataDefined was removed. Use the QgsProperty framework instead.
 - QgsDataDefinedButton was removed. Use QgsPropertyOverrideButton instead.
 - QgsDataDefinedSymbolDialog was removed. Code using this dialog should be reworked to use QgsPropertyOverrideButton
@@ -933,7 +934,7 @@ plugins calling these methods will need to be updated.
 - setDestCRSID has been removed, use setDestinationCrs() instead
 - 'theNode', 'theDoc' parameters in readXML and writeXML have been renamed to 'node' and 'document' respectively
 - readXML() and writeXML() have been removed.
-
+- initialize() was removed.
 
 QgsCoordinateTransformCache        {#qgis_api_break_3_0_QgsCoordinateTransformCache}
 ---------------------------

diff --git a/python/core/conversions.sip b/python/core/conversions.sip
@@ -1756,7 +1756,7 @@ template<int, TYPE2*>
   if ( ( d = PyDict_New() ) == NULL )
     return NULL;
 
-  for ( const auto it = sipCpp->constBegin(); it != sipCpp->constEnd(); ++ it )
+  for ( auto it = sipCpp->constBegin(); it != sipCpp->constEnd(); ++ it )
   {
     PyObject *keyobj;
     if ( ( keyobj = PyTuple_New( 2 ) ) == NULL )

diff --git a/python/core/core_auto.sip b/python/core/core_auto.sip
@@ -24,7 +24,6 @@
 %Include qgscoordinateformatter.sip
 %Include qgscoordinatetransform.sip
 %Include qgscoordinatetransformcontext.sip
-%Include qgscrscache.sip
 %Include qgsdartmeasurement.sip
 %Include qgsdatadefinedsizelegend.sip
 %Include qgsdataitemprovider.sip

diff --git a/python/core/qgscrscache.sip b/python/core/qgscrscache.sip
diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt
@@ -153,7 +153,6 @@ SET(QGIS_CORE_SRCS
   qgscoordinatetransformcontext.cpp
   qgscoordinateutils.cpp
   qgscredentials.cpp
-  qgscrscache.cpp
   qgsdartmeasurement.cpp
   qgsdatadefinedsizelegend.cpp
   qgsdataitem.cpp
@@ -856,7 +855,6 @@ SET(QGIS_CORE_HDRS
   qgscoordinatetransform.h
   qgscoordinatetransformcontext.h
   qgscoordinateutils.h
-  qgscrscache.h
   qgsdartmeasurement.h
   qgsdatadefinedsizelegend.h
   qgsdataitemprovider.h

diff --git a/src/core/qgscoordinatetransform.cpp b/src/core/qgscoordinatetransform.cpp
@@ -42,7 +42,7 @@ extern "C"
 // #define COORDINATE_TRANSFORM_VERBOSE
 
 QReadWriteLock QgsCoordinateTransform::sCacheLock;
-QMultiHash< QPair< QString, QString >, QgsCoordinateTransform > QgsCoordinateTransform::sTransforms;
+QMultiHash< QPair< QString, QString >, QgsCoordinateTransform > QgsCoordinateTransform::sTransforms; //same auth_id pairs might have different datum transformations
 
 QgsCoordinateTransform::QgsCoordinateTransform()
 {
@@ -52,6 +52,11 @@ QgsCoordinateTransform::QgsCoordinateTransform()
 QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination )
 {
   d = new QgsCoordinateTransformPrivate( source, destination, QgsCoordinateTransformContext() );
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 
 QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination, const QgsCoordinateTransformContext &context )
@@ -60,6 +65,11 @@ QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSyst
 #ifdef QGISDEBUG
   d->mHasContext = true;
 #endif
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 
 QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination, const QgsProject *project )
@@ -68,6 +78,11 @@ QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSyst
 #ifdef QGISDEBUG
   d->mHasContext = true;
 #endif
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 
 QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination, int sourceDatumTransform, int destinationDatumTransform )
@@ -76,6 +91,11 @@ QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSyst
 #ifdef QGISDEBUG
   d->mHasContext = true; // not strictly true, but we don't need to worry if datums have been explicitly set
 #endif
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 
 QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateTransform &o )
@@ -96,14 +116,22 @@ void QgsCoordinateTransform::setSourceCrs( const QgsCoordinateReferenceSystem &c
   d.detach();
   d->mSourceCRS = crs;
   d->calculateTransforms();
-  d->initialize();
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 void QgsCoordinateTransform::setDestinationCrs( const QgsCoordinateReferenceSystem &crs )
 {
   d.detach();
   d->mDestCRS = crs;
   d->calculateTransforms();
-  d->initialize();
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 
 void QgsCoordinateTransform::setContext( const QgsCoordinateTransformContext &context )
@@ -114,7 +142,11 @@ void QgsCoordinateTransform::setContext( const QgsCoordinateTransformContext &co
   d->mHasContext = true;
 #endif
   d->calculateTransforms();
-  d->initialize();
+  if ( !setFromCache( d->mSourceCRS, d->mDestCRS, d->mSourceDatumTransform, d->mDestinationDatumTransform ) )
+  {
+    d->initialize();
+    addToCache();
+  }
 }
 
 QgsCoordinateReferenceSystem QgsCoordinateTransform::sourceCrs() const
@@ -727,6 +759,37 @@ void QgsCoordinateTransform::searchDatumTransform( const QString &sql, QList< in
   }
 }
 
+bool QgsCoordinateTransform::setFromCache( const QgsCoordinateReferenceSystem &src, const QgsCoordinateReferenceSystem &dest, int srcDatumTransform, int destDatumTransform )
+{
+  if ( !src.isValid() || !dest.isValid() )
+    return false;
+
+  sCacheLock.lockForRead();
+  const QList< QgsCoordinateTransform > values = sTransforms.values( qMakePair( src.authid(), dest.authid() ) );
+  for ( auto valIt = values.constBegin(); valIt != values.constEnd(); ++valIt )
+  {
+    if ( ( *valIt ).sourceDatumTransform() == srcDatumTransform &&
+         ( *valIt ).destinationDatumTransform() == destDatumTransform )
+    {
+      *this = *valIt;
+      sCacheLock.unlock();
+      return true;
+    }
+  }
+  sCacheLock.unlock();
+  return false;
+}
+
+void QgsCoordinateTransform::addToCache()
+{
+  if ( !d->mSourceCRS.isValid() || !d->mDestCRS.isValid() )
+    return;
+
+  sCacheLock.lockForWrite();
+  sTransforms.insertMulti( qMakePair( d->mSourceCRS.authid(), d->mDestCRS.authid() ), *this );
+  sCacheLock.unlock();
+}
+
 QString QgsCoordinateTransform::datumTransformString( int datumTransform )
 {
   return QgsCoordinateTransformPrivate::datumTransformString( datumTransform );
@@ -794,12 +857,6 @@ void QgsCoordinateTransform::setDestinationDatumTransform( int dt )
   d->mDestinationDatumTransform = dt;
 }
 
-void QgsCoordinateTransform::initialize()
-{
-  d.detach();
-  d->initialize();
-}
-
 void QgsCoordinateTransform::invalidateCache()
 {
   sCacheLock.lockForWrite();

diff --git a/src/core/qgscoordinatetransform.h b/src/core/qgscoordinatetransform.h
@@ -64,7 +64,7 @@ class CORE_EXPORT QgsCoordinateTransform
      * Constructs a QgsCoordinateTransform using QgsCoordinateReferenceSystem objects.
      * \param source source CRS, typically of the layer's coordinate system
      * \param destination CRS, typically of the map canvas coordinate system
-     * \warning Use of this constructor is strongly discouraged, as it will not
+     * \deprecated Use of this constructor is strongly discouraged, as it will not
      * correctly handle the user's datum transform setup. Instead the constructor
      * variant which accepts a QgsCoordinateTransformContext or QgsProject
      * argument should be used instead. It is highly likely that this constructor
@@ -79,7 +79,7 @@ class CORE_EXPORT QgsCoordinateTransform
      *
      * The \a context argument specifies the context under which the transform
      * will be applied, and is used for calculating necessary datum transforms
-     * to utilise.
+     * to utilize.
      *
      * \since QGIS 3.0
      */
@@ -92,7 +92,7 @@ class CORE_EXPORT QgsCoordinateTransform
      * to \a destination coordinate reference system, when used with the
      * given \a project.
      *
-     * No reference to \a project is stored or utilised outside of the constructor,
+     * No reference to \a project is stored or utilized outside of the constructor,
      * and it is used to retrieve the project's transform context only.
      *
      * \since QGIS 3.0
@@ -373,9 +373,6 @@ class CORE_EXPORT QgsCoordinateTransform
      */
     void setDestinationDatumTransform( int datum );
 
-    //!initialize is used to actually create the Transformer instance
-    void initialize();
-
     /**
      * Clears the internal cache used to initialize QgsCoordinateTransform objects.
      * This should be called whenever the srs database has
@@ -390,6 +387,12 @@ class CORE_EXPORT QgsCoordinateTransform
 
     mutable QExplicitlySharedDataPointer<QgsCoordinateTransformPrivate> d;
 
+    bool setFromCache( const QgsCoordinateReferenceSystem &src,
+                       const QgsCoordinateReferenceSystem &dest,
+                       int srcDatumTransform,
+                       int destDatumTransform );
+    void addToCache();
+
     // cache
     static QReadWriteLock sCacheLock;
     static QMultiHash< QPair< QString, QString >, QgsCoordinateTransform > sTransforms; //same auth_id pairs might have different datum transformations

diff --git a/src/core/qgscoordinatetransform_p.cpp b/src/core/qgscoordinatetransform_p.cpp
@@ -59,7 +59,6 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
 {
   setFinder();
   calculateTransforms();
-  initialize();
 }
 
 QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination, int sourceDatumTransform, int destDatumTransform )
@@ -69,7 +68,6 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
   , mDestinationDatumTransform( destDatumTransform )
 {
   setFinder();
-  initialize();
 }
 
 QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinateTransformPrivate &other )

diff --git a/src/core/qgscrscache.cpp b/src/core/qgscrscache.cpp