Skip to content

Commit

Permalink
Revert "Revert "Utilise thread safe proj API within QgsCoordinateTran…
Browse files Browse the repository at this point in the history 
…sform""

This reverts commit 2480e26.

Brings back the thread safe proj implementation. Early in the
release cycle this time for maximal testing before inclusion
in stable releases.
  • Loading branch information
@nyalldawson
nyalldawson committed May 27, 2017
1 parent 8bd4238 commit f3407f8
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 54 deletions.
11 changes: 8 additions & 3 deletions src/core/qgscoordinatereferencesystem.cpp
View file
Expand Up @@ -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 // 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 // we use the original mProj4 with QgsCoordinateTransform, it will fail to initialize
// so better detect it now. // 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 ) if ( !theProj )
{ {
QgsDebugMsg( "proj.4 string rejected by pj_init_plus()" ); QgsDebugMsg( "proj.4 string rejected by pj_init_plus()" );
Expand All @@ -917,6 +918,7 @@ void QgsCoordinateReferenceSystem::setProj4String( const QString& theProj4String
{ {
pj_free( theProj ); pj_free( theProj );
} }
pj_ctx_free( pContext );
d->mWkt.clear(); d->mWkt.clear();
setMapUnits(); setMapUnits();


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


projCtx pContext = pj_ctx_alloc();

#if !defined(PJ_VERSION) || PJ_VERSION!=470 #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" ); 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 ) if ( sqlite3_prepare( database, sql.toAscii(), sql.size(), &select, &tail ) == SQLITE_OK )
Expand All @@ -1869,11 +1873,11 @@ int QgsCoordinateReferenceSystem::syncDb()
const char *params = reinterpret_cast< const char * >( sqlite3_column_text( select, 2 ) ); 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 ); 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 ) if ( !pj )
{ {
input = QString( "+init=%1:%2" ).arg( QString( auth_name ).toUpper(), auth_id ); 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 ) if ( pj )
Expand Down Expand Up @@ -1936,6 +1940,7 @@ int QgsCoordinateReferenceSystem::syncDb()
#endif #endif


OSRDestroySpatialReference( crs ); OSRDestroySpatialReference( crs );
pj_ctx_free( pContext );


if ( sqlite3_exec( database, "COMMIT", nullptr, nullptr, nullptr ) != SQLITE_OK ) if ( sqlite3_exec( database, "COMMIT", nullptr, nullptr, nullptr ) != SQLITE_OK )
{ {
Expand Down
125 changes: 82 additions & 43 deletions src/core/qgscoordinatetransform.cpp
View file
Expand Up @@ -37,12 +37,12 @@ extern "C"
// if defined shows all information about transform to stdout // if defined shows all information about transform to stdout
// #define COORDINATE_TRANSFORM_VERBOSE // #define COORDINATE_TRANSFORM_VERBOSE


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

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


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


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


bool useDefaultDatumTransform = ( mSourceDatumTransform == - 1 && mDestinationDatumTransform == -1 ); bool useDefaultDatumTransform = ( mSourceDatumTransform == - 1 && mDestinationDatumTransform == -1 );


// init the projections (destination and source) freeProj();


pj_free( mSourceProjection ); mSourceProjString = mSourceCRS.toProj4();
QString sourceProjString = mSourceCRS.toProj4();
if ( !useDefaultDatumTransform ) if ( !useDefaultDatumTransform )
{ {
sourceProjString = stripDatumTransform( sourceProjString ); mSourceProjString = stripDatumTransform( mSourceProjString );
} }
if ( mSourceDatumTransform != -1 ) if ( mSourceDatumTransform != -1 )
{ {
sourceProjString += ( ' ' + datumTransformString( mSourceDatumTransform ) ); mSourceProjString += ( ' ' + datumTransformString( mSourceDatumTransform ) );
} }


pj_free( mDestinationProjection ); mDestProjString = mDestCRS.toProj4();
QString destProjString = mDestCRS.toProj4();
if ( !useDefaultDatumTransform ) if ( !useDefaultDatumTransform )
{ {
destProjString = stripDatumTransform( destProjString ); mDestProjString = stripDatumTransform( mDestProjString );
} }
if ( mDestinationDatumTransform != -1 ) if ( mDestinationDatumTransform != -1 )
{ {
destProjString += ( ' ' + datumTransformString( mDestinationDatumTransform ) ); mDestProjString += ( ' ' + datumTransformString( mDestinationDatumTransform ) );
} }


if ( !useDefaultDatumTransform ) if ( !useDefaultDatumTransform )
{ {
addNullGridShifts( sourceProjString, destProjString ); addNullGridShifts( mSourceProjString, mDestProjString );
} }


mSourceProjection = pj_init_plus( sourceProjString.toUtf8() ); // create proj projections for current thread
mDestinationProjection = pj_init_plus( destProjString.toUtf8() ); QPair< projPJ, projPJ > res = threadLocalProjData();


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


mInitialisedFlag = true; mInitialisedFlag = true;
if ( !mDestinationProjection ) if ( !res.second )
{ {
mInitialisedFlag = false; mInitialisedFlag = false;
} }
if ( !mSourceProjection ) if ( !res.first )
{ {
mInitialisedFlag = false; mInitialisedFlag = false;
} }
Expand Down Expand Up @@ -661,8 +643,12 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
QString dir; QString dir;
// if the source/destination projection is lat/long, convert the points to radians // if the source/destination projection is lat/long, convert the points to radians
// prior to transforming // prior to transforming
if (( pj_is_latlong( mDestinationProjection ) && ( direction == ReverseTransform ) ) QPair< projPJ, projPJ > projData = threadLocalProjData();
|| ( pj_is_latlong( mSourceProjection ) && ( direction == ForwardTransform ) ) ) 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 ) for ( int i = 0; i < numPoints; ++i )
{ {
Expand All @@ -674,13 +660,13 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
int projResult; int projResult;
if ( direction == ReverseTransform ) if ( direction == ReverseTransform )
{ {
projResult = pj_transform( mDestinationProjection, mSourceProjection, numPoints, 0, x, y, z ); projResult = pj_transform( destProj, sourceProj, numPoints, 0, x, y, z );
} }
else else
{ {
Q_ASSERT( mSourceProjection ); Q_ASSERT( sourceProj );
Q_ASSERT( mDestinationProjection ); Q_ASSERT( destProj );
projResult = pj_transform( mSourceProjection, mDestinationProjection, numPoints, 0, x, y, z ); projResult = pj_transform( sourceProj, destProj, numPoints, 0, x, y, z );
} }


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


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


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


QString msg = tr( "%1 of\n" QString msg = tr( "%1 of\n"
"%2" "%2"
Expand All @@ -728,8 +714,8 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *


// if the result is lat/long, convert the results from radians back // if the result is lat/long, convert the results from radians back
// to degrees // to degrees
if (( pj_is_latlong( mDestinationProjection ) && ( direction == ForwardTransform ) ) if (( pj_is_latlong( destProj ) && ( direction == ForwardTransform ) )
|| ( pj_is_latlong( mSourceProjection ) && ( direction == ReverseTransform ) ) ) || ( pj_is_latlong( sourceProj ) && ( direction == ReverseTransform ) ) )
{ {
for ( int i = 0; i < numPoints; ++i ) for ( int i = 0; i < numPoints; ++i )
{ {
Expand Down Expand Up @@ -1055,3 +1041,56 @@ void QgsCoordinateTransform::addNullGridShifts( QString& srcProjString, QString&
destProjString += " +nadgrids=@null"; 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 );
}
33 changes: 25 additions & 8 deletions src/core/qgscoordinatetransform.h
View file
Expand Up @@ -19,6 +19,8 @@


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


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


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


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


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


/*! class QgsProjContextStore
* Proj4 data structure of the destination projection (map canvas coordinate system) {
*/ public:
projPJ mDestinationProjection;
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 mSourceDatumTransform;
int mDestinationDatumTransform; int mDestinationDatumTransform;
Expand All @@ -297,6 +311,9 @@ class CORE_EXPORT QgsCoordinateTransform : public QObject
static void searchDatumTransform( const QString& sql, QList< int >& transforms ); static void searchDatumTransform( const QString& sql, QList< int >& transforms );
/** In certain situations, null grid shifts have to be added to src / dst proj string*/ /** In certain situations, null grid shifts have to be added to src / dst proj string*/
void addNullGridShifts( QString& srcProjString, QString& destProjString ); void addNullGridShifts( QString& srcProjString, QString& destProjString );

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


//! Output stream operator //! Output stream operator
Expand Down

0 comments on commit f3407f8

Please sign in to comment.