Skip to content

Commit af3370d

Browse files
nyalldawsonnyalldawson
committedMay 22, 2017
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.
·
final-2_18_28final-2_18_9
1 parent 47f7bbe commit af3370d

File tree

3 files changed

+115
-54
lines changed

3 files changed

+115
-54
lines changed
 

‎src/core/qgscoordinatereferencesystem.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,8 @@ void QgsCoordinateReferenceSystem::setProj4String( const QString& theProj4String
907907
// e.g if they lack a +ellps parameter, it will automatically add +ellps=WGS84, but as
908908
// we use the original mProj4 with QgsCoordinateTransform, it will fail to initialize
909909
// so better detect it now.
910-
projPJ theProj = pj_init_plus( theProj4String.trimmed().toLatin1().constData() );
910+
projCtx pContext = pj_ctx_alloc();
911+
projPJ theProj = pj_init_plus_ctx( pContext, theProj4String.trimmed().toLatin1().constData() );
911912
if ( !theProj )
912913
{
913914
QgsDebugMsg( "proj.4 string rejected by pj_init_plus()" );
@@ -917,6 +918,7 @@ void QgsCoordinateReferenceSystem::setProj4String( const QString& theProj4String
917918
{
918919
pj_free( theProj );
919920
}
921+
pj_ctx_free( pContext );
920922
d->mWkt.clear();
921923
setMapUnits();
922924

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

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

18711875
QString input = QString( "+init=%1:%2" ).arg( QString( auth_name ).toLower(), auth_id );
1872-
projPJ pj = pj_init_plus( input.toAscii() );
1876+
projPJ pj = pj_init_plus_ctx( pContext, input.toAscii() );
18731877
if ( !pj )
18741878
{
18751879
input = QString( "+init=%1:%2" ).arg( QString( auth_name ).toUpper(), auth_id );
1876-
pj = pj_init_plus( input.toAscii() );
1880+
pj = pj_init_plus_ctx( pContext, input.toAscii() );
18771881
}
18781882

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

19381942
OSRDestroySpatialReference( crs );
1943+
pj_ctx_free( pContext );
19391944

19401945
if ( sqlite3_exec( database, "COMMIT", nullptr, nullptr, nullptr ) != SQLITE_OK )
19411946
{

‎src/core/qgscoordinatetransform.cpp

Lines changed: 82 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ extern "C"
3737
// if defined shows all information about transform to stdout
3838
// #define COORDINATE_TRANSFORM_VERBOSE
3939

40+
QThreadStorage< QgsCoordinateTransform::QgsProjContextStore* > QgsCoordinateTransform::mProjContext;
41+
4042
QgsCoordinateTransform::QgsCoordinateTransform()
4143
: QObject()
4244
, mShortCircuit( false )
4345
, mInitialisedFlag( false )
44-
, mSourceProjection( nullptr )
45-
, mDestinationProjection( nullptr )
4646
, mSourceDatumTransform( -1 )
4747
, mDestinationDatumTransform( -1 )
4848
{
@@ -53,8 +53,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( const QgsCoordinateReferenceSyst
5353
: QObject()
5454
, mShortCircuit( false )
5555
, mInitialisedFlag( false )
56-
, mSourceProjection( nullptr )
57-
, mDestinationProjection( nullptr )
5856
, mSourceDatumTransform( -1 )
5957
, mDestinationDatumTransform( -1 )
6058
{
@@ -69,8 +67,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrsId, long theDes
6967
, mInitialisedFlag( false )
7068
, mSourceCRS( QgsCRSCache::instance()->crsBySrsId( theSourceSrsId ) )
7169
, mDestCRS( QgsCRSCache::instance()->crsBySrsId( theDestSrsId ) )
72-
, mSourceProjection( nullptr )
73-
, mDestinationProjection( nullptr )
7470
, mSourceDatumTransform( -1 )
7571
, mDestinationDatumTransform( -1 )
7672
{
@@ -80,8 +76,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrsId, long theDes
8076
QgsCoordinateTransform::QgsCoordinateTransform( const QString& theSourceCRS, const QString& theDestCRS )
8177
: QObject()
8278
, mInitialisedFlag( false )
83-
, mSourceProjection( nullptr )
84-
, mDestinationProjection( nullptr )
8579
, mSourceDatumTransform( -1 )
8680
, mDestinationDatumTransform( -1 )
8781
{
@@ -100,8 +94,6 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrid,
10094
QgsCoordinateReferenceSystem::CrsType theSourceCRSType )
10195
: QObject()
10296
, mInitialisedFlag( false )
103-
, mSourceProjection( nullptr )
104-
, mDestinationProjection( nullptr )
10597
, mSourceDatumTransform( -1 )
10698
, mDestinationDatumTransform( -1 )
10799
{
@@ -118,15 +110,7 @@ QgsCoordinateTransform::QgsCoordinateTransform( long theSourceSrid,
118110

119111
QgsCoordinateTransform::~QgsCoordinateTransform()
120112
{
121-
// free the proj objects
122-
if ( mSourceProjection )
123-
{
124-
pj_free( mSourceProjection );
125-
}
126-
if ( mDestinationProjection )
127-
{
128-
pj_free( mDestinationProjection );
129-
}
113+
freeProj();
130114
}
131115

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

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

183-
// init the projections (destination and source)
167+
freeProj();
184168

185-
pj_free( mSourceProjection );
186-
QString sourceProjString = mSourceCRS.toProj4();
169+
mSourceProjString = mSourceCRS.toProj4();
187170
if ( !useDefaultDatumTransform )
188171
{
189-
sourceProjString = stripDatumTransform( sourceProjString );
172+
mSourceProjString = stripDatumTransform( mSourceProjString );
190173
}
191174
if ( mSourceDatumTransform != -1 )
192175
{
193-
sourceProjString += ( ' ' + datumTransformString( mSourceDatumTransform ) );
176+
mSourceProjString += ( ' ' + datumTransformString( mSourceDatumTransform ) );
194177
}
195178

196-
pj_free( mDestinationProjection );
197-
QString destProjString = mDestCRS.toProj4();
179+
mDestProjString = mDestCRS.toProj4();
198180
if ( !useDefaultDatumTransform )
199181
{
200-
destProjString = stripDatumTransform( destProjString );
182+
mDestProjString = stripDatumTransform( mDestProjString );
201183
}
202184
if ( mDestinationDatumTransform != -1 )
203185
{
204-
destProjString += ( ' ' + datumTransformString( mDestinationDatumTransform ) );
186+
mDestProjString += ( ' ' + datumTransformString( mDestinationDatumTransform ) );
205187
}
206188

207189
if ( !useDefaultDatumTransform )
208190
{
209-
addNullGridShifts( sourceProjString, destProjString );
191+
addNullGridShifts( mSourceProjString, mDestProjString );
210192
}
211193

212-
mSourceProjection = pj_init_plus( sourceProjString.toUtf8() );
213-
mDestinationProjection = pj_init_plus( destProjString.toUtf8() );
194+
// create proj projections for current thread
195+
QPair< projPJ, projPJ > res = threadLocalProjData();
214196

215197
#ifdef COORDINATE_TRANSFORM_VERBOSE
216198
QgsDebugMsg( "From proj : " + mSourceCRS.toProj4() );
217199
QgsDebugMsg( "To proj : " + mDestCRS.toProj4() );
218200
#endif
219201

220202
mInitialisedFlag = true;
221-
if ( !mDestinationProjection )
203+
if ( !res.second )
222204
{
223205
mInitialisedFlag = false;
224206
}
225-
if ( !mSourceProjection )
207+
if ( !res.first )
226208
{
227209
mInitialisedFlag = false;
228210
}
@@ -661,8 +643,12 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
661643
QString dir;
662644
// if the source/destination projection is lat/long, convert the points to radians
663645
// prior to transforming
664-
if (( pj_is_latlong( mDestinationProjection ) && ( direction == ReverseTransform ) )
665-
|| ( pj_is_latlong( mSourceProjection ) && ( direction == ForwardTransform ) ) )
646+
QPair< projPJ, projPJ > projData = threadLocalProjData();
647+
projPJ sourceProj = projData.first;
648+
projPJ destProj = projData.second;
649+
650+
if (( pj_is_latlong( destProj ) && ( direction == ReverseTransform ) )
651+
|| ( pj_is_latlong( sourceProj ) && ( direction == ForwardTransform ) ) )
666652
{
667653
for ( int i = 0; i < numPoints; ++i )
668654
{
@@ -674,13 +660,13 @@ void QgsCoordinateTransform::transformCoords( int numPoints, double *x, double *
674660
int projResult;
675661
if ( direction == ReverseTransform )
676662
{
677-
projResult = pj_transform( mDestinationProjection, mSourceProjection, numPoints, 0, x, y, z );
663+
projResult = pj_transform( destProj, sourceProj, numPoints, 0, x, y, z );
678664
}
679665
else
680666
{
681-
Q_ASSERT( mSourceProjection );
682-
Q_ASSERT( mDestinationProjection );
683-
projResult = pj_transform( mSourceProjection, mDestinationProjection, numPoints, 0, x, y, z );
667+
Q_ASSERT( sourceProj );
668+
Q_ASSERT( destProj );
669+
projResult = pj_transform( sourceProj, destProj, numPoints, 0, x, y, z );
684670
}
685671

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

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

705-
char *srcdef = pj_get_def( mSourceProjection, 0 );
706-
char *dstdef = pj_get_def( mDestinationProjection, 0 );
691+
char *srcdef = pj_get_def( sourceProj, 0 );
692+
char *dstdef = pj_get_def( destProj, 0 );
707693

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

729715
// if the result is lat/long, convert the results from radians back
730716
// to degrees
731-
if (( pj_is_latlong( mDestinationProjection ) && ( direction == ForwardTransform ) )
732-
|| ( pj_is_latlong( mSourceProjection ) && ( direction == ReverseTransform ) ) )
717+
if (( pj_is_latlong( destProj ) && ( direction == ForwardTransform ) )
718+
|| ( pj_is_latlong( sourceProj ) && ( direction == ReverseTransform ) ) )
733719
{
734720
for ( int i = 0; i < numPoints; ++i )
735721
{
@@ -1055,3 +1041,56 @@ void QgsCoordinateTransform::addNullGridShifts( QString& srcProjString, QString&
10551041
destProjString += " +nadgrids=@null";
10561042
}
10571043
}
1044+
1045+
QPair<projPJ, projPJ> QgsCoordinateTransform::threadLocalProjData() const
1046+
{
1047+
mProjLock.lockForRead();
1048+
projCtx pContext = nullptr;
1049+
if ( mProjContext.hasLocalData() )
1050+
pContext = mProjContext.localData()->get();
1051+
else
1052+
{
1053+
mProjContext.setLocalData( new QgsProjContextStore() );
1054+
pContext = mProjContext.localData()->get();
1055+
}
1056+
1057+
QMap< uintptr_t, QPair< projPJ, projPJ > >::const_iterator it = mProjProjections.constFind( reinterpret_cast< uintptr_t >( pContext ) );
1058+
if ( it != mProjProjections.constEnd() )
1059+
{
1060+
QPair< projPJ, projPJ > res = it.value();
1061+
mProjLock.unlock();
1062+
return res;
1063+
}
1064+
1065+
// proj projections don't exist yet, so we need to create
1066+
mProjLock.unlock();
1067+
mProjLock.lockForWrite();
1068+
QPair< projPJ, projPJ > res = qMakePair( pj_init_plus_ctx( pContext, mSourceProjString.toUtf8() ),
1069+
pj_init_plus_ctx( pContext, mDestProjString.toUtf8() ) );
1070+
mProjProjections.insert( reinterpret_cast< uintptr_t >( pContext ), res );
1071+
mProjLock.unlock();
1072+
return res;
1073+
}
1074+
1075+
void QgsCoordinateTransform::freeProj()
1076+
{
1077+
mProjLock.lockForWrite();
1078+
QMap< uintptr_t, QPair< projPJ, projPJ > >::const_iterator it = mProjProjections.constBegin();
1079+
for ( ; it != mProjProjections.constEnd(); ++it )
1080+
{
1081+
pj_free( it.value().first );
1082+
pj_free( it.value().second );
1083+
}
1084+
mProjProjections.clear();
1085+
mProjLock.unlock();
1086+
}
1087+
1088+
QgsCoordinateTransform::QgsProjContextStore::QgsProjContextStore()
1089+
{
1090+
context = pj_ctx_alloc();
1091+
}
1092+
1093+
QgsCoordinateTransform::QgsProjContextStore::~QgsProjContextStore()
1094+
{
1095+
pj_ctx_free( context );
1096+
}

‎src/core/qgscoordinatetransform.h

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
//qt includes
2121
#include <QObject>
22+
#include <QThreadStorage>
23+
#include <QReadWriteLock>
2224

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

3638
typedef void* projPJ;
39+
typedef void* projCtx;
3740
class QString;
3841

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

277-
/*!
278-
* Proj4 data structure of the source projection (layer coordinate system)
279-
*/
280-
projPJ mSourceProjection;
280+
QString mSourceProjString;
281+
QString mDestProjString;
281282

282-
/*!
283-
* Proj4 data structure of the destination projection (map canvas coordinate system)
284-
*/
285-
projPJ mDestinationProjection;
283+
class QgsProjContextStore
284+
{
285+
public:
286+
287+
QgsProjContextStore();
288+
~QgsProjContextStore();
289+
290+
projCtx get() { return context; }
291+
292+
private:
293+
294+
projCtx context;
295+
};
296+
297+
static QThreadStorage< QgsProjContextStore* > mProjContext;
298+
mutable QReadWriteLock mProjLock;
299+
mutable QMap< uintptr_t, QPair< projPJ, projPJ > > mProjProjections;
286300

287301
int mSourceDatumTransform;
288302
int mDestinationDatumTransform;
@@ -297,6 +311,9 @@ class CORE_EXPORT QgsCoordinateTransform : public QObject
297311
static void searchDatumTransform( const QString& sql, QList< int >& transforms );
298312
/** In certain situations, null grid shifts have to be added to src / dst proj string*/
299313
void addNullGridShifts( QString& srcProjString, QString& destProjString );
314+
315+
QPair< projPJ, projPJ > threadLocalProjData() const;
316+
void freeProj();
300317
};
301318

302319
//! Output stream operator

0 commit comments

Comments
 (0)
Please sign in to comment.