Utilise thread safe proj API within QgsCoordinateTransform

Avoids unpredictable behavior when transforms are being
conducted in background threads, such as map renders.

Refs #11441

This commit:
1. Uses thread_local storage for projCtx objects, to ensure
that every thread correctly has its own projCtx context.

2. Refactors QgsCoordinateTransformPrivate so that the
projPJ source and destination objects are instead stored
in a map (by projCtx). This allows transforms to be
transparently performed using the correct projPJ objects
for the particular thread in which the transform is being
conducted. This approach avoids expensive detachment
of QgsCoordinateTransformPrivate, and allows a single
QgsCoordinateTransformPrivate to be safely utilised
by QgsCoordinateTransform objects in different threads.

src/core/qgscoordinatetransform.cpp

@@ -474,8 +474,8 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
 
   // if the source/destination projection is lat/long, convert the points to radians
   // prior to transforming
-  if ( ( pj_is_latlong( d->mDestinationProjection ) && ( direction == ReverseTransform ) )
-       || ( pj_is_latlong( d->mSourceProjection ) && ( direction == ForwardTransform ) ) )
+  if ( ( pj_is_latlong( d->destProjection() ) && ( direction == ReverseTransform ) )
+       || ( pj_is_latlong( d->sourceProjection() ) && ( direction == ForwardTransform ) ) )
   {
     for ( int i = 0; i < numPoints; ++i )
     {
@@ -487,13 +487,13 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
   int projResult;
   if ( direction == ReverseTransform )
   {
-    projResult = pj_transform( d->mDestinationProjection, d->mSourceProjection, numPoints, 0, x, y, z );
+    projResult = pj_transform( d->destProjection(), d->sourceProjection(), numPoints, 0, x, y, z );
   }
   else
   {
-    Q_ASSERT( d->mSourceProjection );
-    Q_ASSERT( d->mDestinationProjection );
-    projResult = pj_transform( d->mSourceProjection, d->mDestinationProjection, numPoints, 0, x, y, z );
+    Q_ASSERT( d->sourceProjection() );
+    Q_ASSERT( d->destProjection() );
+    projResult = pj_transform( d->sourceProjection(), d->destProjection(), numPoints, 0, x, y, z );
   }
 
   if ( projResult != 0 )
@@ -515,8 +515,8 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
 
     QString dir = ( direction == ForwardTransform ) ? QObject::tr( "forward transform" ) : QObject::tr( "inverse transform" );
 
-    char *srcdef = pj_get_def( d->mSourceProjection, 0 );
-    char *dstdef = pj_get_def( d->mDestinationProjection, 0 );
+    char *srcdef = pj_get_def( d->sourceProjection(), 0 );
+    char *dstdef = pj_get_def( d->destProjection(), 0 );
 
     QString msg = QObject::tr( "%1 of\n"
                                "%2"
@@ -538,8 +538,8 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
 
   // if the result is lat/long, convert the results from radians back
   // to degrees
-  if ( ( pj_is_latlong( d->mDestinationProjection ) && ( direction == ForwardTransform ) )
-       || ( pj_is_latlong( d->mSourceProjection ) && ( direction == ReverseTransform ) ) )
+  if ( ( pj_is_latlong( d->destProjection() ) && ( direction == ForwardTransform ) )
+       || ( pj_is_latlong( d->sourceProjection() ) && ( direction == ReverseTransform ) ) )
   {
     for ( int i = 0; i < numPoints; ++i )
     {

src/core/qgscoordinatetransform.h

@@ -280,7 +280,7 @@ class CORE_EXPORT QgsCoordinateTransform
 
     static void searchDatumTransform( const QString &sql, QList< int > &transforms );
 
-    QExplicitlySharedDataPointer<QgsCoordinateTransformPrivate> d;
+    mutable QExplicitlySharedDataPointer<QgsCoordinateTransformPrivate> d;
 };
 
 //! Output stream operator

src/core/qgscoordinatetransform_p.cpp

@@ -29,11 +29,21 @@ extern "C"
 
 /// @cond PRIVATE
 
+thread_local QgsProjContextStore QgsCoordinateTransformPrivate::mProjContext;
+
+QgsProjContextStore::QgsProjContextStore()
+{
+  context = pj_ctx_alloc();
+}
+
+QgsProjContextStore::~QgsProjContextStore()
+{
+  pj_ctx_free( context );
+}
+
 QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate()
   : mIsValid( false )
   , mShortCircuit( false )
-  , mSourceProjection( nullptr )
-  , mDestinationProjection( nullptr )
   , mSourceDatumTransform( -1 )
   , mDestinationDatumTransform( -1 )
 {
@@ -45,8 +55,6 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
   , mShortCircuit( false )
   , mSourceCRS( source )
   , mDestCRS( destination )
-  , mSourceProjection( nullptr )
-  , mDestinationProjection( nullptr )
   , mSourceDatumTransform( -1 )
   , mDestinationDatumTransform( -1 )
 {
@@ -60,8 +68,6 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
   , mShortCircuit( other.mShortCircuit )
   , mSourceCRS( other.mSourceCRS )
   , mDestCRS( other.mDestCRS )
-  , mSourceProjection( nullptr )
-  , mDestinationProjection( nullptr )
   , mSourceDatumTransform( other.mSourceDatumTransform )
   , mDestinationDatumTransform( other.mDestinationDatumTransform )
 {
@@ -72,14 +78,7 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
 QgsCoordinateTransformPrivate::~QgsCoordinateTransformPrivate()
 {
   // free the proj objects
-  if ( mSourceProjection )
-  {
-    pj_free( mSourceProjection );
-  }
-  if ( mDestinationProjection )
-  {
-    pj_free( mDestinationProjection );
-  }
+  freeProj();
 }
 
 bool QgsCoordinateTransformPrivate::initialize()
@@ -109,43 +108,41 @@ bool QgsCoordinateTransformPrivate::initialize()
   bool useDefaultDatumTransform = ( mSourceDatumTransform == - 1 && mDestinationDatumTransform == -1 );
 
   // init the projections (destination and source)
+  freeProj();
 
-  pj_free( mSourceProjection );
-  QString sourceProjString = mSourceCRS.toProj4();
+  mSourceProjString = mSourceCRS.toProj4();
   if ( !useDefaultDatumTransform )
   {
-    sourceProjString = stripDatumTransform( sourceProjString );
+    mSourceProjString = stripDatumTransform( mSourceProjString );
   }
   if ( mSourceDatumTransform != -1 )
   {
-    sourceProjString += ( ' ' + datumTransformString( mSourceDatumTransform ) );
+    mSourceProjString += ( ' ' + datumTransformString( mSourceDatumTransform ) );
   }
 
-  pj_free( mDestinationProjection );
-  QString destProjString = mDestCRS.toProj4();
+  mDestProjString = mDestCRS.toProj4();
   if ( !useDefaultDatumTransform )
   {
-    destProjString = stripDatumTransform( destProjString );
+    mDestProjString = stripDatumTransform( mDestProjString );
   }
   if ( mDestinationDatumTransform != -1 )
   {
-    destProjString += ( ' ' +  datumTransformString( mDestinationDatumTransform ) );
+    mDestProjString += ( ' ' +  datumTransformString( mDestinationDatumTransform ) );
   }
 
   if ( !useDefaultDatumTransform )
   {
-    addNullGridShifts( sourceProjString, destProjString );
+    addNullGridShifts( mSourceProjString, mDestProjString );
   }
 
-  mSourceProjection = pj_init_plus( sourceProjString.toUtf8() );
-  mDestinationProjection = pj_init_plus( destProjString.toUtf8() );
+  initializeCurrentContext();
 
 #ifdef COORDINATE_TRANSFORM_VERBOSE
   QgsDebugMsg( "From proj : " + mSourceCRS.toProj4() );
   QgsDebugMsg( "To proj   : " + mDestCRS.toProj4() );
 #endif
 
-  if ( !mDestinationProjection || !mSourceProjection )
+  if ( !destProjection() || !sourceProjection() )
   {
     mIsValid = false;
   }
@@ -191,6 +188,44 @@ bool QgsCoordinateTransformPrivate::initialize()
   return mIsValid;
 }
 
+void QgsCoordinateTransformPrivate::initializeCurrentContext()
+{
+  mProjLock.lockForWrite();
+  mProjProjections.insert( reinterpret_cast< uintptr_t>( mProjContext.get() ), qMakePair( pj_init_plus_ctx( mProjContext.get(), mSourceProjString.toUtf8() ),
+                           pj_init_plus_ctx( mProjContext.get(), mDestProjString.toUtf8() ) ) );
+  mProjLock.unlock();
+}
+
+projPJ QgsCoordinateTransformPrivate::sourceProjection()
+{
+  mProjLock.lockForRead();
+  if ( mProjProjections.contains( reinterpret_cast< uintptr_t>( mProjContext.get() ) ) )
+  {
+    projPJ src = mProjProjections.value( reinterpret_cast< uintptr_t>( mProjContext.get() ) ).first;
+    mProjLock.unlock();
+    return src;
+  }
+  mProjLock.unlock();
+
+  initializeCurrentContext();
+  return sourceProjection();
+}
+
+projPJ QgsCoordinateTransformPrivate::destProjection()
+{
+  mProjLock.lockForRead();
+  if ( mProjProjections.contains( reinterpret_cast< uintptr_t>( mProjContext.get() ) ) )
+  {
+    projPJ dest = mProjProjections.value( reinterpret_cast< uintptr_t>( mProjContext.get() ) ).second;
+    mProjLock.unlock();
+    return dest;
+  }
+  mProjLock.unlock();
+
+  initializeCurrentContext();
+  return destProjection();
+}
+
 QString QgsCoordinateTransformPrivate::stripDatumTransform( const QString &proj4 ) const
 {
   QStringList parameterSplit = proj4.split( '+', QString::SkipEmptyParts );
@@ -309,4 +344,18 @@ void QgsCoordinateTransformPrivate::setFinder()
 #endif
 }
 
+void QgsCoordinateTransformPrivate::freeProj()
+{
+  mProjLock.lockForWrite();
+  QMap < uintptr_t, QPair< projPJ, projPJ > >::const_iterator it = mProjProjections.constBegin();
+  for ( ; it != mProjProjections.constEnd(); ++it )
+  {
+    pj_free( it.value().first );
+    pj_free( it.value().second );
+  }
+  mProjProjections.clear();
+  mProjLock.unlock();
+}
+
 ///@endcond
+

src/core/qgscoordinatetransform_p.h

@@ -32,6 +32,25 @@
 #include "qgscoordinatereferencesystem.h"
 
 typedef void *projPJ;
+typedef void *projCtx;
+
+/**
+ * \class QgsProjContextStore
+ * \ingroup core
+ * Used to create and store a proj projCtx object, correctly freeing the context upon destruction.
+ */
+class QgsProjContextStore
+{
+  public:
+
+    QgsProjContextStore();
+    ~QgsProjContextStore();
+
+    projCtx get() { return context; }
+
+  private:
+    projCtx context;
+};
 
 class QgsCoordinateTransformPrivate : public QSharedData
 {
@@ -48,6 +67,7 @@ class QgsCoordinateTransformPrivate : public QSharedData
     ~QgsCoordinateTransformPrivate();
 
     bool initialize();
+    void initializeCurrentContext();
 
     //! Flag to indicate whether the transform is valid (ie has a valid
     //! source and destination crs)
@@ -65,15 +85,23 @@ class QgsCoordinateTransformPrivate : public QSharedData
     //! QgsCoordinateReferenceSystem of the destination (map canvas) coordinate system
     QgsCoordinateReferenceSystem mDestCRS;
 
-    //! Proj4 data structure of the source projection (layer coordinate system)
-    projPJ mSourceProjection;
-
-    //! Proj4 data structure of the destination projection (map canvas coordinate system)
-    projPJ mDestinationProjection;
+    QString mSourceProjString;
+    QString mDestProjString;
+    projPJ sourceProjection();
+    projPJ destProjection();
 
     int mSourceDatumTransform;
     int mDestinationDatumTransform;
 
+    /**
+     * Thread local proj context storage. A new proj context will be created
+     * for every thread.
+     */
+    static thread_local QgsProjContextStore mProjContext;
+
+    QReadWriteLock mProjLock;
+    QMap < uintptr_t, QPair< projPJ, projPJ > > mProjProjections;
+
     static QString datumTransformString( int datumTransform );
 
   private:
@@ -85,6 +113,8 @@ class QgsCoordinateTransformPrivate : public QSharedData
     void addNullGridShifts( QString &srcProjString, QString &destProjString ) const;
 
     void setFinder();
+
+    void freeProj();
 };
 
 /// @endcond