diff --git a/doc/api_break.dox b/doc/api_break.dox index 724adc1a09ee..f8d35060f9a8 100644 --- a/doc/api_break.dox +++ b/doc/api_break.dox @@ -1339,6 +1339,9 @@ QgsMapRendererCache {#qgis_api_break_3_0_QgsMapRendererCache} ------------------- - All protected members have been made private. This class is not designed to be subclassed. +- setCacheImage no longer uses layer IDs for cache keys. Cache keys can now be any arbitrary string. +A new parameter for setCacheImage is used to list all layers on which the cache image is dependent. This +allows for cache images which have either no layer dependencies or multiple layer dependencies. QgsMapRendererJob {#qgis_api_break_3_0_QgsMapRendererJob} diff --git a/python/core/qgsmaprenderercache.sip b/python/core/qgsmaprenderercache.sip index 7059be416d5f..1b67911eebec 100644 --- a/python/core/qgsmaprenderercache.sip +++ b/python/core/qgsmaprenderercache.sip @@ -12,10 +12,10 @@ class QgsMapRendererCache : QObject bool init( const QgsRectangle& extent, double scale ); - void setCacheImage( const QString& layerId, const QImage& img ); + void setCacheImage( const QString& cacheKey, const QImage& image, const QStringList& dependentLayerIds = QStringList() ); - QImage cacheImage( const QString& layerId ); + QImage cacheImage( const QString& cacheKey ) const; - void clearCacheImage( const QString& layerId ); + void clearCacheImage( const QString& cacheKey ); }; diff --git a/src/core/qgsmaprenderercache.cpp b/src/core/qgsmaprenderercache.cpp index cd5427de6137..423fb8bfde71 100644 --- a/src/core/qgsmaprenderercache.cpp +++ b/src/core/qgsmaprenderercache.cpp @@ -35,16 +35,47 @@ void QgsMapRendererCache::clearInternal() mScale = 0; // make sure we are disconnected from all layers - QMap::const_iterator it = mCachedImages.constBegin(); - for ( ; it != mCachedImages.constEnd(); ++it ) + Q_FOREACH ( const QString& id, mConnectedLayerIds ) { - QgsMapLayer* layer = QgsProject::instance()->mapLayer( it.key() ); + QgsMapLayer* layer = QgsProject::instance()->mapLayer( id ); if ( layer ) { disconnect( layer, &QgsMapLayer::repaintRequested, this, &QgsMapRendererCache::layerRequestedRepaint ); } } mCachedImages.clear(); + mConnectedLayerIds.clear(); +} + +void QgsMapRendererCache::dropUnusedConnections() +{ + QSet< QString > stillDepends = dependentLayerIds(); + QSet< QString > disconnects = mConnectedLayerIds.subtract( stillDepends ); + Q_FOREACH ( const QString& id, disconnects ) + { + QgsMapLayer* layer = QgsProject::instance()->mapLayer( id ); + if ( layer ) + { + disconnect( layer, &QgsMapLayer::repaintRequested, this, &QgsMapRendererCache::layerRequestedRepaint ); + } + } + + mConnectedLayerIds = stillDepends; +} + +QSet< QString > QgsMapRendererCache::dependentLayerIds() const +{ + QSet< QString > result; + QMap::const_iterator it = mCachedImages.constBegin(); + for ( ; it != mCachedImages.constEnd(); ++it ) + { + Q_FOREACH ( const QPointer< QgsMapLayer >& l, it.value().dependentLayers ) + { + if ( l.data() ) + result << l->id(); + } + } + return result; } bool QgsMapRendererCache::init( const QgsRectangle& extent, double scale ) @@ -65,41 +96,64 @@ bool QgsMapRendererCache::init( const QgsRectangle& extent, double scale ) return false; } -void QgsMapRendererCache::setCacheImage( const QString& layerId, const QImage& img ) +void QgsMapRendererCache::setCacheImage( const QString& cacheKey, const QImage& image, const QStringList& dependentLayerIds ) { QMutexLocker lock( &mMutex ); - mCachedImages[layerId] = img; + + CacheParameters params; + params.cachedImage = image; // connect to the layer to listen to layer's repaintRequested() signals - QgsMapLayer* layer = QgsProject::instance()->mapLayer( layerId ); - if ( layer ) + Q_FOREACH ( const QString& id, dependentLayerIds ) { - connect( layer, &QgsMapLayer::repaintRequested, this, &QgsMapRendererCache::layerRequestedRepaint ); + QgsMapLayer* layer = QgsProject::instance()->mapLayer( id ); + if ( layer ) + { + params.dependentLayers << layer; + if ( !mConnectedLayerIds.contains( id ) ) + { + connect( layer, &QgsMapLayer::repaintRequested, this, &QgsMapRendererCache::layerRequestedRepaint ); + mConnectedLayerIds << id; + } + } } + + mCachedImages[cacheKey] = params; } -QImage QgsMapRendererCache::cacheImage( const QString& layerId ) +QImage QgsMapRendererCache::cacheImage( const QString& cacheKey ) const { QMutexLocker lock( &mMutex ); - return mCachedImages.value( layerId ); + return mCachedImages.value( cacheKey ).cachedImage; } void QgsMapRendererCache::layerRequestedRepaint() { QgsMapLayer* layer = qobject_cast( sender() ); - if ( layer ) - clearCacheImage( layer->id() ); -} + if ( !layer ) + return; -void QgsMapRendererCache::clearCacheImage( const QString& layerId ) -{ QMutexLocker lock( &mMutex ); - mCachedImages.remove( layerId ); - - QgsMapLayer* layer = QgsProject::instance()->mapLayer( layerId ); - if ( layer ) + // check through all cached images to clear any which depend on this layer + QMap::iterator it = mCachedImages.begin(); + for ( ; it != mCachedImages.end(); ) { - disconnect( layer, &QgsMapLayer::repaintRequested, this, &QgsMapRendererCache::layerRequestedRepaint ); + if ( !it.value().dependentLayers.contains( layer ) ) + { + ++it; + continue; + } + + it = mCachedImages.erase( it ); } + dropUnusedConnections(); +} + +void QgsMapRendererCache::clearCacheImage( const QString& cacheKey ) +{ + QMutexLocker lock( &mMutex ); + + mCachedImages.remove( cacheKey ); + dropUnusedConnections(); } diff --git a/src/core/qgsmaprenderercache.h b/src/core/qgsmaprenderercache.h index e5920f08e3d2..272754df8b77 100644 --- a/src/core/qgsmaprenderercache.h +++ b/src/core/qgsmaprenderercache.h @@ -22,14 +22,17 @@ #include #include "qgsrectangle.h" +#include "qgsmaplayer.h" /** \ingroup core - * This class is responsible for keeping cache of rendered images of individual layers. + * This class is responsible for keeping cache of rendered images resulting from + * a map rendering job. * - * Once a layer has rendered image stored in the cache (using setCacheImage(...)), - * the cache listens to repaintRequested() signals from layer. If triggered, the cache - * removes the rendered image (and disconnects from the layer). + * Once a job has a rendered image stored in the cache (using setCacheImage(...)), + * the cache listens to repaintRequested() signals from dependent layers. + * If triggered, the cache removes the rendered image (and disconnects from the + * layers). * * The class is thread-safe (multiple classes can access the same instance safely). * @@ -42,34 +45,71 @@ class CORE_EXPORT QgsMapRendererCache : public QObject QgsMapRendererCache(); - //! Invalidate the cache contents + /** + * Invalidates the cache contents, clearing all cached images. + * @see clearCacheImage() + */ void clear(); - //! Initialize cache: set new parameters and erase cache if parameters have changed - //! @return flag whether the parameters are the same as last time + /** + * Initialize cache: set new parameters and clears the cache if any + * parameters have changed since last initialization. + * @return flag whether the parameters are the same as last time + */ bool init( const QgsRectangle& extent, double scale ); - //! Set the cached image for the specified layer ID - void setCacheImage( const QString& layerId, const QImage& img ); - - //! Returns the cached image for the specified layer ID. Returns null image if it is not cached. - QImage cacheImage( const QString& layerId ); - - //! Removes a layer from the cache - void clearCacheImage( const QString& layerId ); + /** + * Set the cached \a image for a particular \a cacheKey. The \a cacheKey usually + * matches the QgsMapLayer::id() which the image is a render of. + * A list of \a dependentLayerIds should be passed containing all layer IDs + * on which this cache image is dependent. If any of these layers triggers a + * repaint then the cache image will be cleared. + * @see cacheImage() + */ + void setCacheImage( const QString& cacheKey, const QImage& image, const QStringList& dependentLayerIds = QStringList() ); + + /** + * Returns the cached image for the specified \a cacheKey. The \a cacheKey usually + * matches the QgsMapLayer::id() which the image is a render of. + * Returns a null image if it is not cached. + * @see setCacheImage() + */ + QImage cacheImage( const QString& cacheKey ) const; + + /** + * Removes an image from the cache with matching \a cacheKey. + * @see clear() + */ + void clearCacheImage( const QString& cacheKey ); private slots: //! Remove layer (that emitted the signal) from the cache void layerRequestedRepaint(); private: + + struct CacheParameters + { + QImage cachedImage; + QList< QPointer< QgsMapLayer > > dependentLayers; + }; + //! Invalidate cache contents (without locking) void clearInternal(); - QMutex mMutex; + //! Disconnects from layers we no longer care about + void dropUnusedConnections(); + + QSet dependentLayerIds() const; + + mutable QMutex mMutex; QgsRectangle mExtent; - double mScale; - QMap mCachedImages; + double mScale = 0; + + //! Map of cache key to cache parameters + QMap mCachedImages; + //! List of all layer ids on which this cache is currently connected + QSet< QString > mConnectedLayerIds; }; diff --git a/src/core/qgsmaprendererjob.cpp b/src/core/qgsmaprendererjob.cpp index dd1863c5f8e3..1290e1ac584c 100644 --- a/src/core/qgsmaprendererjob.cpp +++ b/src/core/qgsmaprendererjob.cpp @@ -326,7 +326,7 @@ void QgsMapRendererJob::cleanupJobs( LayerRenderJobs& jobs ) if ( mCache && !job.cached && !job.context.renderingStopped() ) { QgsDebugMsg( "caching image for " + job.layerId ); - mCache->setCacheImage( job.layerId, *job.img ); + mCache->setCacheImage( job.layerId, *job.img, QStringList() << job.layerId ); } delete job.img; diff --git a/tests/src/python/test_qgsmaprenderercache.py b/tests/src/python/test_qgsmaprenderercache.py index f2b91820c3d1..cb951e91a5e2 100644 --- a/tests/src/python/test_qgsmaprenderercache.py +++ b/tests/src/python/test_qgsmaprenderercache.py @@ -57,7 +57,7 @@ def testSetCacheImages(self): def testInit(self): cache = QgsMapRendererCache() extent = QgsRectangle(1, 2, 3, 4) - cache.init(extent, 1000) + self.assertFalse(cache.init(extent, 1000)) # add a cache image im = QImage(200, 200, QImage.Format_RGB32) @@ -65,13 +65,13 @@ def testInit(self): self.assertFalse(cache.cacheImage('layer').isNull()) # re init, without changing extent or scale - cache.init(extent, 1000) + self.assertTrue(cache.init(extent, 1000)) # image should still be in cache self.assertFalse(cache.cacheImage('layer').isNull()) # reinit with different scale - cache.init(extent, 2000) + self.assertFalse(cache.init(extent, 2000)) # cache should be cleared self.assertTrue(cache.cacheImage('layer').isNull()) @@ -80,11 +80,12 @@ def testInit(self): self.assertFalse(cache.cacheImage('layer').isNull()) # change extent - cache.init(QgsRectangle(11, 12, 13, 14), 2000) + self.assertFalse(cache.init(QgsRectangle(11, 12, 13, 14), 2000)) # cache should be cleared self.assertTrue(cache.cacheImage('layer').isNull()) - def testRequestRepaint(self): + def testRequestRepaintSimple(self): + """ test requesting repaint with a single dependent layer """ layer = QgsVectorLayer("Point?field=fldtxt:string", "layer", "memory") QgsProject.instance().addMapLayers([layer]) @@ -93,13 +94,77 @@ def testRequestRepaint(self): # add image to cache cache = QgsMapRendererCache() im = QImage(200, 200, QImage.Format_RGB32) - cache.setCacheImage(layer.id(), im) - self.assertFalse(cache.cacheImage(layer.id()).isNull()) + cache.setCacheImage('xxx', im, [layer.id()]) + self.assertFalse(cache.cacheImage('xxx').isNull()) # trigger repaint on layer layer.triggerRepaint() # cache image should be cleared - self.assertTrue(cache.cacheImage(layer.id()).isNull()) + self.assertTrue(cache.cacheImage('xxx').isNull()) + QgsProject.instance().removeMapLayer(layer.id()) + + def testRequestRepaintMultiple(self): + """ test requesting repaint with multiple dependent layers """ + layer1 = QgsVectorLayer("Point?field=fldtxt:string", + "layer1", "memory") + layer2 = QgsVectorLayer("Point?field=fldtxt:string", + "layer2", "memory") + QgsProject.instance().addMapLayers([layer1, layer2]) + self.assertTrue(layer1.isValid()) + self.assertTrue(layer2.isValid()) + + # add image to cache - no dependent layers + cache = QgsMapRendererCache() + im1 = QImage(200, 200, QImage.Format_RGB32) + cache.setCacheImage('nolayer', im1) + self.assertFalse(cache.cacheImage('nolayer').isNull()) + + # trigger repaint on layer + layer1.triggerRepaint() + layer1.triggerRepaint() # do this a couple of times - we don't want errors due to multiple disconnects, etc + layer2.triggerRepaint() + layer2.triggerRepaint() + # cache image should still exist - it's not dependent on layers + self.assertFalse(cache.cacheImage('nolayer').isNull()) + + # image depends on 1 layer + im_l1 = QImage(200, 200, QImage.Format_RGB32) + cache.setCacheImage('im1', im_l1, [layer1.id()]) + + # image depends on 2 layers + im_l1_l2 = QImage(200, 200, QImage.Format_RGB32) + cache.setCacheImage('im1_im2', im_l1_l2, [layer1.id(), layer2.id()]) + + # image depends on 2nd layer alone + im_l2 = QImage(200, 200, QImage.Format_RGB32) + cache.setCacheImage('im2', im_l2, [layer2.id()]) + + self.assertFalse(cache.cacheImage('im1').isNull()) + self.assertFalse(cache.cacheImage('im1_im2').isNull()) + self.assertFalse(cache.cacheImage('im2').isNull()) + + # trigger repaint layer 1 (check twice - don't want disconnect errors) + for i in range(2): + layer1.triggerRepaint() + #should be cleared + self.assertTrue(cache.cacheImage('im1').isNull()) + self.assertTrue(cache.cacheImage('im1_im2').isNull()) + # should be retained + self.assertFalse(cache.cacheImage('im2').isNull()) + self.assertEqual(cache.cacheImage('im2'), im_l2) + self.assertFalse(cache.cacheImage('nolayer').isNull()) + self.assertEqual(cache.cacheImage('nolayer'), im1) + + # trigger repaint layer 2 + for i in range(2): + layer2.triggerRepaint() + #should be cleared + self.assertTrue(cache.cacheImage('im1').isNull()) + self.assertTrue(cache.cacheImage('im1_im2').isNull()) + self.assertTrue(cache.cacheImage('im2').isNull()) + # should be retained + self.assertFalse(cache.cacheImage('nolayer').isNull()) + self.assertEqual(cache.cacheImage('nolayer'), im1) if __name__ == '__main__':