QgsMapRendererCache can have multiple dependent layers for a cache image

In some cases the render result for a layer may depend on more than
one layer.

This changes QgsMapRendererCache to use "cache keys" instead of layer ids
to identify images within the cache. When setting an image inside the
cache, a new list parameter is used to specify all layers on which
that image depends.

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}

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 );
 
 };

src/core/qgsmaprenderercache.cpp

@@ -35,16 +35,47 @@ void QgsMapRendererCache::clearInternal()
   mScale = 0;
 
   // make sure we are disconnected from all layers
-  QMap<QString, QImage>::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<QString, CacheParameters>::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<QgsMapLayer*>( 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<QString, CacheParameters>::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();
 }

src/core/qgsmaprenderercache.h

@@ -22,14 +22,17 @@
 #include <QMutex>
 
 #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<QString> dependentLayerIds() const;
+
+    mutable QMutex mMutex;
     QgsRectangle mExtent;
-    double mScale;
-    QMap<QString, QImage> mCachedImages;
+    double mScale = 0;
+
+    //! Map of cache key to cache parameters
+    QMap<QString, CacheParameters> mCachedImages;
+    //! List of all layer ids on which this cache is currently connected
+    QSet< QString > mConnectedLayerIds;
 };
 
 

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;

tests/src/python/test_qgsmaprenderercache.py

@@ -57,21 +57,21 @@ 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)
         cache.setCacheImage('layer', im)
         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__':