Reporting of rendering errors, safer isActive() for custom painter job

qgis · Nov 26, 2013 · 879f051 · 879f051
1 parent 9a4f326
commit 879f051
Show file tree

Hide file tree

Showing 8 changed files with 104 additions and 43 deletions.
diff --git a/src/core/qgsmaplayerrenderer.h b/src/core/qgsmaplayerrenderer.h
@@ -1,8 +1,7 @@
 #ifndef QGSMAPLAYERRENDERER_H
 #define QGSMAPLAYERRENDERER_H
 
-#include <QList>
-#include <QString>
+#include <QStringList>
 
 /**
  * Base class for utility classes that encapsulate information necessary
@@ -27,24 +26,21 @@
 class QgsMapLayerRenderer
 {
 public:
+  QgsMapLayerRenderer( const QString& layerID ) : mLayerID( layerID ) {}
   virtual ~QgsMapLayerRenderer() {}
 
   //! Do the rendering (based on data stored in the class)
   virtual bool render() = 0;
 
-  //! Container for errors (@todo instead of simple message could have error codes + custom data)
-  struct Error
-  {
-    QString message;
-  };
-
-  typedef QList<Error> ErrorList;
-
   //! Return list of errors (problems) that happened during the rendering
-  ErrorList errors() const { return mErrors; }
+  QStringList errors() const { return mErrors; }
+
+  //! Get access to the ID of the layer rendered by this class
+  QString layerID() const { return mLayerID; }
 
 protected:
-  ErrorList mErrors;
+  QStringList mErrors;
+  QString mLayerID;
 };
 
 #endif // QGSMAPLAYERRENDERER_H
diff --git a/src/core/qgsmaprendererjob.cpp b/src/core/qgsmaprendererjob.cpp
@@ -19,6 +19,11 @@ QgsMapRendererJob::QgsMapRendererJob( const QgsMapSettings& settings )
 {
 }
 
+QgsMapRendererJob::Errors QgsMapRendererJob::errors() const
+{
+  return mErrors;
+}
+
 
 QgsMapRendererQImageJob::QgsMapRendererQImageJob( const QgsMapSettings& settings )
   : QgsMapRendererJob( settings )
@@ -59,6 +64,8 @@ void QgsMapRendererSequentialJob::start()
   if ( isActive() )
     return; // do nothing if we are already running
 
+  mErrors.clear();
+
   qDebug("SEQUENTIAL START");
   qDebug("%d,%d", mSettings.outputSize().width(), mSettings.outputSize().height());
 
@@ -122,6 +129,8 @@ void QgsMapRendererSequentialJob::internalFinished()
 
   mLabelingResults = mInternalJob->takeLabelingResults();
 
+  mErrors = mInternalJob->errors();
+
   delete mInternalJob;
   mInternalJob = 0;
 
@@ -137,6 +146,7 @@ QgsMapRendererCustomPainterJob::QgsMapRendererCustomPainterJob(const QgsMapSetti
   : QgsMapRendererJob( settings )
   , mPainter(painter)
   , mLabelingEngine( 0 )
+  , mActive( false )
 {
   qDebug("QPAINTER construct");
 }
@@ -153,6 +163,13 @@ QgsMapRendererCustomPainterJob::~QgsMapRendererCustomPainterJob()
 
 void QgsMapRendererCustomPainterJob::start()
 {
+  if ( isActive() )
+    return;
+
+  mActive = true;
+
+  mErrors.clear();
+
   qDebug("QPAINTER run!");
 
   QgsDebugMsg( "Preparing list of layer jobs for rendering" );
@@ -200,9 +217,12 @@ void QgsMapRendererCustomPainterJob::start()
 
 void QgsMapRendererCustomPainterJob::cancel()
 {
-  // TODO: custom flag indicating that some rendering has started?
-  //if (mFuture.isRunning())
+  if ( !isActive() )
   {
+    qDebug("QPAINTER not running!");
+    return;
+  }
+
     qDebug("QPAINTER cancelling");
     disconnect(&mFutureWatcher, SIGNAL(finished()), this, SLOT(futureFinished()));
 
@@ -222,14 +242,11 @@ void QgsMapRendererCustomPainterJob::cancel()
     futureFinished();
 
     qDebug("QPAINTER cancelled");
-  }
-  //else
-  //  qDebug("QPAINTER not running!");
 }
 
 void QgsMapRendererCustomPainterJob::waitForFinished()
 {
-  if (!mFuture.isRunning())
+  if ( !isActive() )
     return;
 
   disconnect(&mFutureWatcher, SIGNAL(finished()), this, SLOT(futureFinished()));
@@ -246,7 +263,7 @@ void QgsMapRendererCustomPainterJob::waitForFinished()
 
 bool QgsMapRendererCustomPainterJob::isActive() const
 {
-  return mFutureWatcher.isRunning();
+  return mActive;
 }
 
 
@@ -258,6 +275,7 @@ QgsLabelingResults* QgsMapRendererCustomPainterJob::takeLabelingResults()
 
 void QgsMapRendererCustomPainterJob::futureFinished()
 {
+  mActive = false;
   qDebug("QPAINTER futureFinished");
   emit finished();
 }
@@ -473,7 +491,7 @@ LayerRenderJobs QgsMapRendererJob::prepareJobs( QPainter* painter, QgsPalLabelin
 
     if ( !ml )
     {
-      QgsDebugMsg( "Layer not found in registry!" );
+      mErrors.append( Error( layerId, "Layer not found in registry." ) );
       continue;
     }
 
@@ -502,8 +520,11 @@ LayerRenderJobs QgsMapRendererJob::prepareJobs( QPainter* painter, QgsPalLabelin
       split = reprojectToLayerExtent( ct, ml->crs().geographicFlag(), r1, r2 );
       QgsDebugMsg( "  extent 1: " + r1.toString() );
       QgsDebugMsg( "  extent 2: " + r2.toString() );
-      if ( !r1.isFinite() || !r2.isFinite() ) //there was a problem transforming the extent. Skip the layer
+      if ( !r1.isFinite() || !r2.isFinite() )
+      {
+        mErrors.append( Error( layerId, "There was a problem transforming layer's' extent. Layer skipped." ) );
         continue;
+      }
     }
 
     // Flattened image for drawing when a blending mode is set
@@ -518,8 +539,7 @@ LayerRenderJobs QgsMapRendererJob::prepareJobs( QPainter* painter, QgsPalLabelin
                                       mSettings.outputSize().height(), QImage::Format_ARGB32 );
       if ( mypFlattenedImage->isNull() )
       {
-        QgsDebugMsg( "insufficient memory for image " + QString::number( mSettings.outputSize().width() ) + "x" + QString::number( mSettings.outputSize().height() ) );
-        // TODO: add to the list of errors!
+        mErrors.append( Error( layerId, "Insufficient memory for image " + QString::number( mSettings.outputSize().width() ) + "x" + QString::number( mSettings.outputSize().height() ) ) );
         delete mypFlattenedImage;
         continue;
       }
@@ -549,22 +569,10 @@ LayerRenderJobs QgsMapRendererJob::prepareJobs( QPainter* painter, QgsPalLabelin
 
     /*
     // TODO: split extent
-    if ( !ml->draw( mRenderContext ) )
-    {
-      // TODO emit drawError( ml );
-    }
-    else
-    {
-      QgsDebugMsg( "Layer rendered without issues" );
-    }
-
     if ( split )
     {
       mRenderContext.setExtent( r2 );
-      if ( !ml->draw( mRenderContext ) )
-      {
-        // TODO emit drawError( ml );
-      }
+      ml->draw( mRenderContext );
     }*/
 
   } // while (li.hasPrevious())
@@ -587,6 +595,9 @@ void QgsMapRendererJob::cleanupJobs( LayerRenderJobs& jobs )
       job.img = 0;
     }
 
+    foreach ( QString message, job.renderer->errors() )
+      mErrors.append( Error( job.renderer->layerID(), message ) );
+
     delete job.renderer;
     job.renderer = 0;
   }

diff --git a/src/core/qgsmaprendererjob.h b/src/core/qgsmaprendererjob.h
@@ -55,6 +55,19 @@ class QgsMapRendererJob : public QObject
   //! Get pointer to internal labeling engine (in order to get access to the results)
   virtual QgsLabelingResults* takeLabelingResults() = 0;
 
+  struct Error
+  {
+    Error( const QString& lid, const QString& msg ) : layerID( lid ), message( msg ) {}
+
+    QString layerID;
+    QString message;
+  };
+
+  typedef QList<Error> Errors;
+
+  //! List of errors that happened during the rendering job - available when the rendering has been finished
+  Errors errors() const;
+
 signals:
 
   //! emitted when asynchronous rendering is finished (or canceled).
@@ -72,12 +85,13 @@ class QgsMapRendererJob : public QObject
 
   LayerRenderJobs prepareJobs( QPainter* painter, QgsPalLabeling* labelingEngine );
 
-  static void cleanupJobs( LayerRenderJobs& jobs );
+  void cleanupJobs( LayerRenderJobs& jobs );
 
   bool needTemporaryImage( QgsMapLayer* ml );
 
 
   QgsMapSettings mSettings;
+  Errors mErrors;
 };
 
 
@@ -203,6 +217,7 @@ protected slots:
   QgsRenderContext mRenderContext;  // used just for labeling!
   QgsPalLabeling* mLabelingEngine;
 
+  bool mActive;
   LayerRenderJobs mLayerJobs;
 };
 

diff --git a/src/core/qgsvectorlayerrenderer.cpp b/src/core/qgsvectorlayerrenderer.cpp
@@ -19,16 +19,16 @@
 
 
 QgsVectorLayerRenderer::QgsVectorLayerRenderer( QgsVectorLayer* layer, QgsRenderContext& context )
-  : mContext( context )
+  : QgsMapLayerRenderer( layer->id() )
+  , mContext( context )
   , mFields( layer->pendingFields() )
-  , mLayerID( layer->id() )
   , mRendererV2( 0 )
   , mCache( 0 )
   , mLabeling( false )
   , mDiagrams( false )
   , mLayerTransparency( 0 )
 {
-  mRendererV2 = layer->rendererV2()->clone();
+  mRendererV2 = layer->rendererV2() ? layer->rendererV2()->clone() : 0;
   mSelectedFeatureIds = layer->selectedFeaturesIds();
 
   mDrawVertexMarkers = ( layer->editBuffer() != 0 );
@@ -101,7 +101,10 @@ bool QgsVectorLayerRenderer::render()
     return true;
 
   if ( !mRendererV2 )
+  {
+    mErrors.append( "No renderer for drawing." );
     return false;
+  }
 
   // Per feature blending mode
   if ( mContext.useAdvancedEffects() && mFeatureBlendMode != QPainter::CompositionMode_SourceOver )

diff --git a/src/core/qgsvectorlayerrenderer.h b/src/core/qgsvectorlayerrenderer.h
@@ -58,8 +58,6 @@ class QgsVectorLayerRenderer : public QgsMapLayerRenderer
 
   QgsFields mFields;
 
-  QString mLayerID;
-
   QgsFeatureIds mSelectedFeatureIds;
 
   QgsFeatureIterator mFit;

diff --git a/src/core/raster/qgsrasterlayerrenderer.cpp b/src/core/raster/qgsrasterlayerrenderer.cpp
@@ -7,7 +7,8 @@
 
 
 QgsRasterLayerRenderer::QgsRasterLayerRenderer( QgsRasterLayer* layer, QgsRenderContext& rendererContext )
-  : mRasterViewPort( 0 )
+  : QgsMapLayerRenderer( layer->id() )
+  , mRasterViewPort( 0 )
   , mPipe( 0 )
 {
 

diff --git a/src/gui/qgsmapcanvas.cpp b/src/gui/qgsmapcanvas.cpp
@@ -496,6 +496,12 @@ void QgsMapCanvas::rendererJobFinished()
 
   mDirty = false;
 
+  // TODO: would be better to show the errors in message bar
+  foreach ( const QgsMapRendererJob::Error& error, mJob->errors() )
+  {
+    QgsMessageLog::logMessage( error.layerID + " :: " + error.message, tr( "Rendering" ) );
+  }
+
   if ( !mJobCancelled )
   {
     QImage img = mJob->renderedImage();

diff --git a/tests/src/core/testmaprendererjob.cpp b/tests/src/core/testmaprendererjob.cpp
@@ -24,6 +24,8 @@ class TestQgsMapRendererJob : public QObject
     void testStartWhileRunning();
     void testDestructWhileRunning();
 
+    void testErrors();
+
   private:
     QStringList mLayerIds;
 };
@@ -202,6 +204,35 @@ void TestQgsMapRendererJob::testDestructWhileRunning()
   jobP.start();
 }
 
+void TestQgsMapRendererJob::testErrors()
+{
+  QgsVectorLayer* l = new QgsVectorLayer( "/data/gis/sas/trans-trail-l.dbf", "test", "ogr" );
+  QVERIFY( l->isValid() );
+  QgsMapLayerRegistry::instance()->addMapLayer( l );
+  QgsMapSettings settings( _mapSettings( QStringList( l->id() ) ) );
+
+  l->setRendererV2( 0 ); // this has to produce an error
+
+  QgsMapRendererSequentialJob job( settings );
+  job.start();
+  job.waitForFinished();
+
+  QCOMPARE( job.errors().count(), 1 );
+  QCOMPARE( job.errors()[0].layerID, l->id() );
+
+  QgsMapLayerRegistry::instance()->removeMapLayer( l->id() );
+
+  QString fakeLayerID = "non-existing layer ID";
+  QgsMapSettings settings2( _mapSettings( QStringList( fakeLayerID ) ) );
+
+  QgsMapRendererSequentialJob job2( settings2 );
+  job2.start();
+  job2.waitForFinished();
+
+  QCOMPARE( job2.errors().count(), 1 );
+  QCOMPARE( job2.errors()[0].layerID, fakeLayerID );
+}
+
 
 QTEST_MAIN( TestQgsMapRendererJob )
 #include "moc_testmaprendererjob.cxx"