Revert "Asyncronously render composer map previews "

Author: nyalldawson

src/core/composer/qgscomposermap.cpp

@@ -115,16 +115,9 @@ QgsComposerMap::~QgsComposerMap()
 {
   delete mOverviewStack;
   delete mGridStack;
-
-  if ( mPainterJob )
-  {
-    disconnect( mPainterJob.get(), &QgsMapRendererCustomPainterJob::finished, this, &QgsComposerMap::painterJobFinished );
-    mPainterJob->cancel();
-    mPainter->end();
-  }
 }
 
-/* This function is called by paint() to render the map.  It does not override any functions
+/* This function is called by paint() and cache() to render the map.  It does not override any functions
 from QGraphicsItem. */
 void QgsComposerMap::draw( QPainter *painter, const QgsRectangle &extent, QSizeF size, double dpi, double *forceWidthScale )
 {
@@ -213,28 +206,12 @@ void QgsComposerMap::cache()
     return;
   }
 
-  if ( mPainterJob )
-  {
-    disconnect( mPainterJob.get(), &QgsMapRendererCustomPainterJob::finished, this, &QgsComposerMap::painterJobFinished );
-    QgsMapRendererCustomPainterJob *oldJob = mPainterJob.release();
-    QPainter *oldPainter = mPainter.release();
-    QImage *oldImage = mCacheRenderingImage.release();
-    connect( oldJob, &QgsMapRendererCustomPainterJob::finished, this, [oldPainter, oldJob, oldImage]
-    {
-      oldJob->deleteLater();
-      delete oldPainter;
-      delete oldImage;
-    } );
-    oldJob->cancelWithoutBlocking();
-  }
-  else
+  if ( mDrawing )
   {
-    mCacheRenderingImage.reset( nullptr );
+    return;
   }
 
-  Q_ASSERT( !mPainterJob );
-  Q_ASSERT( !mPainter );
-  Q_ASSERT( !mCacheRenderingImage );
+  mDrawing = true;
 
   double horizontalVScaleFactor = horizontalViewScaleFactor();
   if ( horizontalVScaleFactor < 0 )
@@ -265,51 +242,38 @@ void QgsComposerMap::cache()
     }
   }
 
-  if ( w <= 0 || h <= 0 )
-    return;
-
-  mCacheRenderingImage.reset( new QImage( w, h, QImage::Format_ARGB32 ) );
+  mCacheImage = QImage( w, h, QImage::Format_ARGB32 );
 
   // set DPI of the image
-  mCacheRenderingImage->setDotsPerMeterX( 1000 * w / widthMM );
-  mCacheRenderingImage->setDotsPerMeterY( 1000 * h / heightMM );
+  mCacheImage.setDotsPerMeterX( 1000 * w / widthMM );
+  mCacheImage.setDotsPerMeterY( 1000 * h / heightMM );
 
   if ( hasBackground() )
   {
     //Initially fill image with specified background color. This ensures that layers with blend modes will
     //preview correctly
-    mCacheRenderingImage->fill( backgroundColor().rgba() );
+    mCacheImage.fill( backgroundColor().rgba() );
   }
   else
   {
     //no background, but start with empty fill to avoid artifacts
-    mCacheRenderingImage->fill( QColor( 255, 255, 255, 0 ).rgba() );
+    mCacheImage.fill( QColor( 255, 255, 255, 0 ).rgba() );
   }
 
-  mPainter.reset( new QPainter( mCacheRenderingImage.get() ) );
-  QgsMapSettings settings( mapSettings( ext, QSizeF( w, h ), mCacheRenderingImage->logicalDpiX() ) );
-  mPainterJob.reset( new QgsMapRendererCustomPainterJob( settings, mPainter.get() ) );
-  connect( mPainterJob.get(), &QgsMapRendererCustomPainterJob::finished, this, &QgsComposerMap::painterJobFinished );
-  mPainterJob->start();
-}
+  QPainter p( &mCacheImage );
 
-void QgsComposerMap::painterJobFinished()
-{
-  mPainter->end();
-  mPainterJob.reset( nullptr );
-  mPainter.reset( nullptr );
+  draw( &p, ext, QSizeF( w, h ), mCacheImage.logicalDpiX() );
+  p.end();
   mCacheUpdated = true;
-  mCacheFinalImage = std::move( mCacheRenderingImage );
-  mLastRenderedImageOffsetX = 0;
-  mLastRenderedImageOffsetY = 0;
-  updateItem();
+
+  mDrawing = false;
 }
 
 void QgsComposerMap::paint( QPainter *painter, const QStyleOptionGraphicsItem *, QWidget *pWidget )
 {
   Q_UNUSED( pWidget );
 
-  if ( !mComposition || !painter || !painter->device() )
+  if ( !mComposition || !painter )
   {
     return;
   }
@@ -319,9 +283,6 @@ void QgsComposerMap::paint( QPainter *painter, const QStyleOptionGraphicsItem *,
   }
 
   QRectF thisPaintRect = QRectF( 0, 0, QGraphicsRectItem::rect().width(), QGraphicsRectItem::rect().height() );
-  if ( thisPaintRect.width() == 0 || thisPaintRect.height() == 0 )
-    return;
-
   painter->save();
   painter->setClipRect( thisPaintRect );
 
@@ -336,40 +297,22 @@ void QgsComposerMap::paint( QPainter *painter, const QStyleOptionGraphicsItem *,
   }
   else if ( mComposition->plotStyle() == QgsComposition::Preview )
   {
-    if ( !mCacheFinalImage || mCacheFinalImage->isNull() )
-    {
-      // No initial render available - so draw some preview text alerting user
-      drawBackground( painter );
-      painter->setBrush( QBrush( QColor( 125, 125, 125, 125 ) ) );
-      painter->drawRect( thisPaintRect );
-      painter->setBrush( Qt::NoBrush );
-      QFont messageFont;
-      messageFont.setPointSize( 12 );
-      painter->setFont( messageFont );
-      painter->setPen( QColor( 255, 255, 255, 255 ) );
-      painter->drawText( thisPaintRect, Qt::AlignCenter | Qt::AlignHCenter, tr( "Rendering map" ) );
-      if ( !mPainterJob )
-      {
-        // this is the map's very first paint - trigger a cache update
-        cache();
-      }
-    }
-    else
-    {
-      //Background color is already included in cached image, so no need to draw
+    if ( mCacheImage.isNull() )
+      cache();
 
-      double imagePixelWidth = mCacheFinalImage->width(); //how many pixels of the image are for the map extent?
-      double scale = rect().width() / imagePixelWidth;
+    //Background color is already included in cached image, so no need to draw
 
-      painter->save();
+    double imagePixelWidth = mCacheImage.width(); //how many pixels of the image are for the map extent?
+    double scale = rect().width() / imagePixelWidth;
 
-      painter->translate( mLastRenderedImageOffsetX + mXOffset, mLastRenderedImageOffsetY + mYOffset );
-      painter->scale( scale, scale );
-      painter->drawImage( 0, 0, *mCacheFinalImage );
+    painter->save();
 
-      //restore rotation
-      painter->restore();
-    }
+    painter->translate( mXOffset, mYOffset );
+    painter->scale( scale, scale );
+    painter->drawImage( 0, 0, mCacheImage );
+
+    //restore rotation
+    painter->restore();
   }
   else if ( mComposition->plotStyle() == QgsComposition::Print ||
             mComposition->plotStyle() == QgsComposition::Postscript )
@@ -622,8 +565,6 @@ void QgsComposerMap::resize( double dx, double dy )
 
 void QgsComposerMap::moveContent( double dx, double dy )
 {
-  mLastRenderedImageOffsetX -= dx;
-  mLastRenderedImageOffsetY -= dy;
   if ( !mDrawing )
   {
     transformShift( dx, dy );
@@ -732,7 +673,7 @@ void QgsComposerMap::setSceneRect( const QRectF &rectangle )
   mCacheUpdated = false;
 
   updateBoundingRect();
-  updateItem();
+  update();
   emit itemChanged();
   emit extentChanged();
 }

src/core/composer/qgscomposermap.h

@@ -43,7 +43,6 @@ class QgsFillSymbol;
 class QgsLineSymbol;
 class QgsVectorLayer;
 class QgsAnnotation;
-class QgsMapRendererCustomPainterJob;
 
 /** \ingroup core
  *  \class QgsComposerMap
@@ -492,8 +491,6 @@ class CORE_EXPORT QgsComposerMap : public QgsComposerItem
   private slots:
     void layersAboutToBeRemoved( QList<QgsMapLayer *> layers );
 
-    void painterJobFinished();
-
   private:
 
     //! Unique identifier
@@ -516,15 +513,8 @@ class CORE_EXPORT QgsComposerMap : public QgsComposerItem
     // to manually tweak each atlas preview page without affecting the actual original map extent.
     QgsRectangle mAtlasFeatureExtent;
 
-    // We have two images used for rendering/storing cached map images.
-    // the first (mCacheFinalImage) is used ONLY for storing the most recent completed map render. It's always
-    // used when drawing map item previews. The second (mCacheRenderingImage) is used temporarily while
-    // rendering a new preview image in the background. If (and only if) the background render completes, then
-    // mCacheRenderingImage is pushed into mCacheFinalImage, and used from then on when drawing the item preview.
-    // This ensures that something is always shown in the map item, even while refreshing the preview image in the
-    // background
-    std::unique_ptr< QImage > mCacheFinalImage;
-    std::unique_ptr< QImage > mCacheRenderingImage;
+    // Cache used in composer preview
+    QImage mCacheImage;
 
     // Is cache up to date
     bool mCacheUpdated = false;
@@ -543,9 +533,6 @@ class CORE_EXPORT QgsComposerMap : public QgsComposerItem
     //! Offset in y direction for showing map cache image
     double mYOffset = 0.0;
 
-    double mLastRenderedImageOffsetX = 0.0;
-    double mLastRenderedImageOffsetY = 0.0;
-
     //! Map rotation
     double mMapRotation = 0;
 
@@ -600,10 +587,6 @@ class CORE_EXPORT QgsComposerMap : public QgsComposerItem
     //! Margin size for atlas driven extents (percentage of feature size) - when in auto scaling mode
     double mAtlasMargin = 0.10;
 
-    std::unique_ptr< QPainter > mPainter;
-    std::unique_ptr< QgsMapRendererCustomPainterJob > mPainterJob;
-    bool mPainterCancelWait = false;
-
     void init();
 
     //! Resets the item tooltip to reflect current map id

tests/src/python/test_qgspallabeling_composer.py

@@ -23,7 +23,7 @@
 import os
 import subprocess
 
-from qgis.PyQt.QtCore import QRect, QRectF, QSize, QSizeF, qDebug, QThreadPool
+from qgis.PyQt.QtCore import QRect, QRectF, QSize, QSizeF, qDebug
 from qgis.PyQt.QtGui import QImage, QColor, QPainter
 from qgis.PyQt.QtPrintSupport import QPrinter
 from qgis.PyQt.QtSvg import QSvgRenderer, QSvgGenerator
@@ -88,8 +88,6 @@ def tearDownClass(cls):
         TestQgsPalLabeling.tearDownClass()
         cls.removeMapLayer(cls.layer)
         cls.layer = None
-        # avoid crash on finish, probably related to https://bugreports.qt.io/browse/QTBUG-35760
-        QThreadPool.globalInstance().waitForDone()
 
     def setUp(self):
         """Run before each test."""