Skip to content

Commit

Permalink
Move caching to QgsCoordinateTransform
Browse files Browse the repository at this point in the history
and remove no longer required QgsCoordinateTransformCache singleton
  • Loading branch information
nyalldawson committed Dec 15, 2017
1 parent 439ef20 commit cb693a7
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 222 deletions.
3 changes: 2 additions & 1 deletion doc/api_break.dox
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
---------------------------
Expand Down
2 changes: 1 addition & 1 deletion python/core/conversions.sip
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down
1 change: 0 additions & 1 deletion python/core/core_auto.sip
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 0 additions & 56 deletions python/core/qgscrscache.sip

This file was deleted.

2 changes: 0 additions & 2 deletions src/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ SET(QGIS_CORE_SRCS
qgscoordinatetransformcontext.cpp
qgscoordinateutils.cpp
qgscredentials.cpp
qgscrscache.cpp
qgsdartmeasurement.cpp
qgsdatadefinedsizelegend.cpp
qgsdataitem.cpp
Expand Down Expand Up @@ -856,7 +855,6 @@ SET(QGIS_CORE_HDRS
qgscoordinatetransform.h
qgscoordinatetransformcontext.h
qgscoordinateutils.h
qgscrscache.h
qgsdartmeasurement.h
qgsdatadefinedsizelegend.h
qgsdataitemprovider.h
Expand Down
77 changes: 67 additions & 10 deletions src/core/qgscoordinatetransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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 )
Expand All @@ -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 )
Expand All @@ -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 )
Expand All @@ -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 )
Expand All @@ -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 )
Expand All @@ -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
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -794,12 +857,6 @@ void QgsCoordinateTransform::setDestinationDatumTransform( int dt )
d->mDestinationDatumTransform = dt;
}

void QgsCoordinateTransform::initialize()
{
d.detach();
d->initialize();
}

void QgsCoordinateTransform::invalidateCache()
{
sCacheLock.lockForWrite();
Expand Down
15 changes: 9 additions & 6 deletions src/core/qgscoordinatetransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
*/
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 0 additions & 2 deletions src/core/qgscoordinatetransform_p.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
{
setFinder();
calculateTransforms();
initialize();
}

QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinateReferenceSystem &source, const QgsCoordinateReferenceSystem &destination, int sourceDatumTransform, int destDatumTransform )
Expand All @@ -69,7 +68,6 @@ QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinat
, mDestinationDatumTransform( destDatumTransform )
{
setFinder();
initialize();
}

QgsCoordinateTransformPrivate::QgsCoordinateTransformPrivate( const QgsCoordinateTransformPrivate &other )
Expand Down
77 changes: 0 additions & 77 deletions src/core/qgscrscache.cpp

This file was deleted.

Loading

0 comments on commit cb693a7

Please sign in to comment.