[layouts] Fix crash when using maps containing AMS layers

Implement a similar hack to what the canvas uses to workaround the badly behaved AMS and WCS providers Fixes #17959
qgis · Jan 29, 2018 · bbe974c · bbe974c
1 parent c10438e
commit bbe974c
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 12 deletions.
diff --git a/src/core/layout/qgslayoutitemmap.cpp b/src/core/layout/qgslayoutitemmap.cpp
@@ -34,8 +34,11 @@
 QgsLayoutItemMap::QgsLayoutItemMap( QgsLayout *layout )
   : QgsLayoutItem( layout )
 {
-  assignFreeId();
+  mBackgroundUpdateTimer = new QTimer( this );
+  mBackgroundUpdateTimer->setSingleShot( true );
+  connect( mBackgroundUpdateTimer, &QTimer::timeout, this, &QgsLayoutItemMap::recreateCachedImageInBackground );
 
+  assignFreeId();
 
   if ( layout )
   {
@@ -774,18 +777,20 @@ void QgsLayoutItemMap::paint( QPainter *painter, const QStyleOptionGraphicsItem
       painter->setFont( messageFont );
       painter->setPen( QColor( 255, 255, 255, 255 ) );
       painter->drawText( thisPaintRect, Qt::AlignCenter | Qt::AlignHCenter, tr( "Rendering map" ) );
-      if ( !mPainterJob )
+      if ( !mPainterJob && !mDrawingPreview )
       {
         // this is the map's very first paint - trigger a cache update
-        recreateCachedImageInBackground( style->matrix.m11() );
+        mPreviewScaleFactor = style->matrix.m11();
+        mBackgroundUpdateTimer->start( 1 );
       }
     }
     else
     {
-      if ( mCacheInvalidated )
+      if ( mCacheInvalidated && !mDrawingPreview )
       {
         // cache was invalidated - trigger a background update
-        recreateCachedImageInBackground( style->matrix.m11() );
+        mPreviewScaleFactor = style->matrix.m11();
+        mBackgroundUpdateTimer->start( 1 );
       }
 
       //Background color is already included in cached image, so no need to draw
@@ -966,7 +971,7 @@ void QgsLayoutItemMap::drawMap( QPainter *painter, const QgsRectangle &extent, Q
   job.renderSynchronously();
 }
 
-void QgsLayoutItemMap::recreateCachedImageInBackground( double viewScaleFactor )
+void QgsLayoutItemMap::recreateCachedImageInBackground()
 {
   if ( mPainterJob )
   {
@@ -995,8 +1000,8 @@ void QgsLayoutItemMap::recreateCachedImageInBackground( double viewScaleFactor )
   double widthLayoutUnits = ext.width() * mapUnitsToLayoutUnits();
   double heightLayoutUnits = ext.height() * mapUnitsToLayoutUnits();
 
-  int w = widthLayoutUnits * viewScaleFactor;
-  int h = heightLayoutUnits * viewScaleFactor;
+  int w = widthLayoutUnits * mPreviewScaleFactor;
+  int h = heightLayoutUnits * mPreviewScaleFactor;
 
   // limit size of image for better performance
   if ( w > 5000 || h > 5000 )
@@ -1040,6 +1045,18 @@ void QgsLayoutItemMap::recreateCachedImageInBackground( double viewScaleFactor )
   mPainterJob.reset( new QgsMapRendererCustomPainterJob( settings, mPainter.get() ) );
   connect( mPainterJob.get(), &QgsMapRendererCustomPainterJob::finished, this, &QgsLayoutItemMap::painterJobFinished );
   mPainterJob->start();
+
+  // from now on we can accept refresh requests again
+  // this must be reset only after the job has been started, because
+  // some providers (yes, it's you WCS and AMS!) during preparation
+  // do network requests and start an internal event loop, which may
+  // end up calling refresh() and would schedule another refresh,
+  // deleting the one we have just started.
+
+  // ^^ that comment was directly copied from a similar fix in QgsMapCanvas. And
+  // with little surprise, both those providers are still badly behaved and causing
+  // annoying bugs for us to deal with...
+  mDrawingPreview = false;
 }
 
 QgsMapSettings QgsLayoutItemMap::mapSettings( const QgsRectangle &extent, QSizeF size, double dpi, bool includeLayerSettings ) const

diff --git a/src/core/layout/qgslayoutitemmap.h b/src/core/layout/qgslayoutitemmap.h
@@ -485,6 +485,9 @@ class CORE_EXPORT QgsLayoutItemMap : public QgsLayoutItem
 
     void mapThemeChanged( const QString &theme );
 
+    //! Create cache image
+    void recreateCachedImageInBackground();
+
   private:
 
 
@@ -527,6 +530,11 @@ class CORE_EXPORT QgsLayoutItemMap : public QgsLayoutItem
     //! \brief set to true if in state of drawing. Concurrent requests to draw method are returned if set to true
     bool mDrawing = false;
 
+    QTimer *mBackgroundUpdateTimer = nullptr;
+    double mPreviewScaleFactor = 0;
+
+    bool mDrawingPreview = false;
+
     //! Offset in x direction for showing map cache image
     double mXOffset = 0.0;
     //! Offset in y direction for showing map cache image
@@ -578,10 +586,6 @@ class CORE_EXPORT QgsLayoutItemMap : public QgsLayoutItem
      */
     void drawMap( QPainter *painter, const QgsRectangle &extent, QSizeF size, double dpi );
 
-
-    //! Create cache image
-    void recreateCachedImageInBackground( double viewScaleFactor );
-
     //! Establishes signal/slot connection for update in case of layer change
     void connectUpdateSlot();