Skip to content
Permalink
Browse files
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 QThreadStorage for projCtx objects, to ensure that every
thread correctly has its own projCtx context

2. Refactors QgsCoordinateTransform 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 allows a single QgsCoordinateTransform
to be safely utilised in different threads.
  • Loading branch information
nyalldawson committed May 22, 2017
1 parent 47f7bbe commit af3370d03e7e7502e8a738d99cd99e38df23768d
Showing with 115 additions and 54 deletions.
  1. +8 −3 src/core/qgscoordinatereferencesystem.cpp
  2. +82 −43 src/core/qgscoordinatetransform.cpp
  3. +25 −8 src/core/qgscoordinatetransform.h
@@ -907,7 +907,8 @@ void QgsCoordinateReferenceSystem::setProj4String( const QString& theProj4String
// e.g if they lack a +ellps parameter, it will automatically add +ellps=WGS84, but as
// we use the original mProj4 with QgsCoordinateTransform, it will fail to initialize
// so better detect it now.
projPJ theProj = pj_init_plus( theProj4String.trimmed().toLatin1().constData() );
projCtx pContext = pj_ctx_alloc();
projPJ theProj = pj_init_plus_ctx( pContext, theProj4String.trimmed().toLatin1().constData() );
if ( !theProj )
{
QgsDebugMsg( "proj.4 string rejected by pj_init_plus()" );
@@ -917,6 +918,7 @@ void QgsCoordinateReferenceSystem::setProj4String( const QString& theProj4String
{
pj_free( theProj );
}
pj_ctx_free( pContext );
d->mWkt.clear();
setMapUnits();

@@ -1858,6 +1860,8 @@ int QgsCoordinateReferenceSystem::syncDb()
sqlite3_errmsg( database ) );
}

projCtx pContext = pj_ctx_alloc();

#if !defined(PJ_VERSION) || PJ_VERSION!=470
sql = QString( "select auth_name,auth_id,parameters from tbl_srs WHERE auth_name<>'EPSG' AND NOT deprecated AND NOT noupdate" );
if ( sqlite3_prepare( database, sql.toAscii(), sql.size(), &select, &tail ) == SQLITE_OK )
@@ -1869,11 +1873,11 @@ int QgsCoordinateReferenceSystem::syncDb()
const char *params = reinterpret_cast< const char * >( sqlite3_column_text( select, 2 ) );

QString input = QString( "+init=%1:%2" ).arg( QString( auth_name ).toLower(), auth_id );
projPJ pj = pj_init_plus( input.toAscii() );
projPJ pj = pj_init_plus_ctx( pContext, input.toAscii() );
if ( !pj )
{
input = QString( "+init=%1:%2" ).arg( QString( auth_name ).toUpper(), auth_id );
pj = pj_init_plus( input.toAscii() );
pj = pj_init_plus_ctx( pContext, input.toAscii() );
}

if ( pj )
@@ -1936,6 +1940,7 @@ int QgsCoordinateReferenceSystem::syncDb()
#endif

OSRDestroySpatialReference( crs );
pj_ctx_free( pContext );

if ( sqlite3_exec( database, "COMMIT", nullptr, nullptr, nullptr ) != SQLITE_OK )
{
@@ -37,12 +37,12 @@ extern "C"
// if defined shows all information about transform to stdout
// #define COORDINATE_TRANSFORM_VERBOSE

QThreadStorage< QgsCoordinateTransform::QgsProjContextStore* > QgsCoordinateTransform::mProjContext;

QgsCoordinateTransform::QgsCoordinateTransform()
: QObject()
, mShortCircuit( false )
, mInitialisedFlag( false )
, mSourceProjection( nullptr )
, mDestinationProjection( nullptr )
, mSourceDatumTransform( -1 )
, mDestinationDatumTransform( -1 )
{
@@ -53,8 +53,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSyst
: QObject()
, mShortCircuit( false )
, mInitialisedFlag( false )
, mSourceProjection( nullptr )
, mDestinationProjection( nullptr )
, mSourceDatumTransform( -1 )
, mDestinationDatumTransform( -1 )
{
@@ -69,8 +67,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrsId, long theDes
, mInitialisedFlag( false )
, mSourceCRS( QgsCRSCache::instance()->crsBySrsId( theSourceSrsId ) )
, mDestCRS( QgsCRSCache::instance()->crsBySrsId( theDestSrsId ) )
, mSourceProjection( nullptr )
, mDestinationProjection( nullptr )
, mSourceDatumTransform( -1 )
, mDestinationDatumTransform( -1 )
{
@@ -80,8 +76,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrsId, long theDes
QgsCoordinateTransform::QgsCoordinateTransform( const QString& theSourceCRS, const QString& theDestCRS )
: QObject()
, mInitialisedFlag( false )
, mSourceProjection( nullptr )
, mDestinationProjection( nullptr )
, mSourceDatumTransform( -1 )
, mDestinationDatumTransform( -1 )
{
@@ -100,8 +94,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrid,
QgsCoordinateReferenceSystem::CrsType theSourceCRSType )
: QObject()
, mInitialisedFlag( false )
, mSourceProjection( nullptr )
, mDestinationProjection( nullptr )
, mSourceDatumTransform( -1 )
, mDestinationDatumTransform( -1 )
{
@@ -118,15 +110,7 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrid,

QgsCoordinateTransform::~QgsCoordinateTransform()
{
// free the proj objects
if ( mSourceProjection )
{
pj_free( mSourceProjection );
}
if ( mDestinationProjection )
{
pj_free( mDestinationProjection );
}
freeProj();
}

QgsCoordinateTransform* QgsCoordinateTransform::clone() const
@@ -180,49 +164,47 @@ void QgsCoordinateTransform::initialise()

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() );
// create proj projections for current thread
QPair< projPJ, projPJ > res = threadLocalProjData();

#ifdef COORDINATE_TRANSFORM_VERBOSE
QgsDebugMsg( "From proj : " + mSourceCRS.toProj4() );
QgsDebugMsg( "To proj : " + mDestCRS.toProj4() );
#endif

mInitialisedFlag = true;
if ( !mDestinationProjection )
if ( !res.second )
{
mInitialisedFlag = false;
}
if ( !mSourceProjection )
if ( !res.first )
{
mInitialisedFlag = false;
}
@@ -661,8 +643,12 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
QString dir;
// if the source/destination projection is lat/long, convert the points to radians
// prior to transforming
if (( pj_is_latlong( mDestinationProjection ) && ( direction == ReverseTransform ) )
|| ( pj_is_latlong( mSourceProjection ) && ( direction == ForwardTransform ) ) )
QPair< projPJ, projPJ > projData = threadLocalProjData();
projPJ sourceProj = projData.first;
projPJ destProj = projData.second;

if (( pj_is_latlong( destProj ) && ( direction == ReverseTransform ) )
|| ( pj_is_latlong( sourceProj ) && ( direction == ForwardTransform ) ) )
{
for ( int i = 0; i < numPoints; ++i )
{
@@ -674,13 +660,13 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
int projResult;
if ( direction == ReverseTransform )
{
projResult = pj_transform( mDestinationProjection, mSourceProjection, numPoints, 0, x, y, z );
projResult = pj_transform( destProj, sourceProj, numPoints, 0, x, y, z );
}
else
{
Q_ASSERT( mSourceProjection );
Q_ASSERT( mDestinationProjection );
projResult = pj_transform( mSourceProjection, mDestinationProjection, numPoints, 0, x, y, z );
Q_ASSERT( sourceProj );
Q_ASSERT( destProj );
projResult = pj_transform( sourceProj, destProj, numPoints, 0, x, y, z );
}

if ( projResult != 0 )
@@ -702,8 +688,8 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *

dir = ( direction == ForwardTransform ) ? tr( "forward transform" ) : tr( "inverse transform" );

char *srcdef = pj_get_def( mSourceProjection, 0 );
char *dstdef = pj_get_def( mDestinationProjection, 0 );
char *srcdef = pj_get_def( sourceProj, 0 );
char *dstdef = pj_get_def( destProj, 0 );

QString msg = tr( "%1 of\n"
"%2"
@@ -728,8 +714,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( mDestinationProjection ) && ( direction == ForwardTransform ) )
|| ( pj_is_latlong( mSourceProjection ) && ( direction == ReverseTransform ) ) )
if (( pj_is_latlong( destProj ) && ( direction == ForwardTransform ) )
|| ( pj_is_latlong( sourceProj ) && ( direction == ReverseTransform ) ) )
{
for ( int i = 0; i < numPoints; ++i )
{
@@ -1055,3 +1041,56 @@ void QgsCoordinateTransform::addNullGridShifts( QString& srcProjString, QString&
destProjString += " +nadgrids=@null";
}
}

QPair<projPJ, projPJ> QgsCoordinateTransform::threadLocalProjData() const
{
mProjLock.lockForRead();
projCtx pContext = nullptr;
if ( mProjContext.hasLocalData() )
pContext = mProjContext.localData()->get();
else
{
mProjContext.setLocalData( new QgsProjContextStore() );
pContext = mProjContext.localData()->get();
}

QMap< uintptr_t, QPair< projPJ, projPJ > >::const_iterator it = mProjProjections.constFind( reinterpret_cast< uintptr_t >( pContext ) );
if ( it != mProjProjections.constEnd() )
{
QPair< projPJ, projPJ > res = it.value();
mProjLock.unlock();
return res;
}

// proj projections don't exist yet, so we need to create
mProjLock.unlock();
mProjLock.lockForWrite();
QPair< projPJ, projPJ > res = qMakePair( pj_init_plus_ctx( pContext, mSourceProjString.toUtf8() ),
pj_init_plus_ctx( pContext, mDestProjString.toUtf8() ) );
mProjProjections.insert( reinterpret_cast< uintptr_t >( pContext ), res );
mProjLock.unlock();
return res;
}

void QgsCoordinateTransform::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();
}

QgsCoordinateTransform::QgsProjContextStore::QgsProjContextStore()
{
context = pj_ctx_alloc();
}

QgsCoordinateTransform::QgsProjContextStore::~QgsProjContextStore()
{
pj_ctx_free( context );
}
@@ -19,6 +19,8 @@

//qt includes
#include <QObject>
#include <QThreadStorage>
#include <QReadWriteLock>

//qgis includes
#include "qgspoint.h"
@@ -34,6 +36,7 @@ class QPolygonF;
#include <vector>

typedef void* projPJ;
typedef void* projCtx;
class QString;

/** \ingroup core
@@ -274,15 +277,26 @@ class CORE_EXPORT QgsCoordinateTransform : public QObject
*/
QgsCoordinateReferenceSystem mDestCRS;

/*!
* Proj4 data structure of the source projection (layer coordinate system)
*/
projPJ mSourceProjection;
QString mSourceProjString;
QString mDestProjString;

/*!
* Proj4 data structure of the destination projection (map canvas coordinate system)
*/
projPJ mDestinationProjection;
class QgsProjContextStore
{
public:

QgsProjContextStore();
~QgsProjContextStore();

projCtx get() { return context; }

private:

projCtx context;
};

static QThreadStorage< QgsProjContextStore* > mProjContext;
mutable QReadWriteLock mProjLock;
mutable QMap< uintptr_t, QPair< projPJ, projPJ > > mProjProjections;

int mSourceDatumTransform;
int mDestinationDatumTransform;
@@ -297,6 +311,9 @@ class CORE_EXPORT QgsCoordinateTransform : public QObject
static void searchDatumTransform( const QString& sql, QList< int >& transforms );
/** In certain situations, null grid shifts have to be added to src / dst proj string*/
void addNullGridShifts( QString& srcProjString, QString& destProjString );

QPair< projPJ, projPJ > threadLocalProjData() const;
void freeProj();
};

//! Output stream operator

0 comments on commit af3370d

Please sign in to comment.