Add some debugging asserts to ensure that layer renderers

are used in a thread safe way
qgis · Nov 19, 2017 · af5b2a7 · af5b2a7
1 parent a0c6e05
commit af5b2a7
Show file tree

Hide file tree

Showing 14 changed files with 68 additions and 13 deletions.
diff --git a/python/core/symbology/qgsnullsymbolrenderer.sip b/python/core/symbology/qgsnullsymbolrenderer.sip
@@ -27,7 +27,6 @@ class QgsNullSymbolRenderer : QgsFeatureRenderer
     virtual QgsSymbol *originalSymbolForFeature( QgsFeature &feature, QgsRenderContext &context );
 
     virtual bool renderFeature( QgsFeature &feature, QgsRenderContext &context, int layer = -1, bool selected = false, bool drawVertexMarker = false );
-    virtual void startRender( QgsRenderContext &context, const QgsFields &fields );
     virtual void stopRender( QgsRenderContext &context );
     virtual bool willRenderFeature( QgsFeature &feat, QgsRenderContext &context );
 

diff --git a/python/core/symbology/qgsrenderer.sip b/python/core/symbology/qgsrenderer.sip
@@ -113,7 +113,7 @@ return a new renderer - used by default in vector layers
  :rtype: set of str
 %End
 
-    virtual void startRender( QgsRenderContext &context, const QgsFields &fields ) = 0;
+    virtual void startRender( QgsRenderContext &context, const QgsFields &fields );
 %Docstring
  Must be called when a new render cycle is started. A call to startRender() must always
  be followed by a corresponding call to stopRender() after all features have been rendered.
@@ -127,7 +127,7 @@ return a new renderer - used by default in vector layers
  the renderer should instead be cloned and startRender()/stopRender() called on the clone.
 %End
 
-    virtual void stopRender( QgsRenderContext &context ) = 0;
+    virtual void stopRender( QgsRenderContext &context );
 %Docstring
  Must be called when a render cycle has finished, to allow the renderer to clean up.
 

diff --git a/src/core/symbology/qgs25drenderer.cpp b/src/core/symbology/qgs25drenderer.cpp
@@ -138,11 +138,15 @@ QgsFeatureRenderer *Qgs25DRenderer::create( QDomElement &element, const QgsReadW
 
 void Qgs25DRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   mSymbol->startRender( context, fields );
 }
 
 void Qgs25DRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   mSymbol->stopRender( context );
 }
 

diff --git a/src/core/symbology/qgscategorizedsymbolrenderer.cpp b/src/core/symbology/qgscategorizedsymbolrenderer.cpp
@@ -377,6 +377,8 @@ void QgsCategorizedSymbolRenderer::sortByLabel( Qt::SortOrder order )
 
 void QgsCategorizedSymbolRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   mCounting = context.rendererScale() == 0.0;
 
   // make sure that the hash table is up to date
@@ -398,6 +400,8 @@ void QgsCategorizedSymbolRenderer::startRender( QgsRenderContext &context, const
 
 void QgsCategorizedSymbolRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   Q_FOREACH ( const QgsRendererCategory &cat, mCategories )
   {
     cat.symbol()->stopRender( context );

diff --git a/src/core/symbology/qgsgraduatedsymbolrenderer.cpp b/src/core/symbology/qgsgraduatedsymbolrenderer.cpp
@@ -364,6 +364,8 @@ QgsSymbol *QgsGraduatedSymbolRenderer::originalSymbolForFeature( QgsFeature &fea
 
 void QgsGraduatedSymbolRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   mCounting = context.rendererScale() == 0.0;
 
   // find out classification attribute index from name
@@ -386,6 +388,8 @@ void QgsGraduatedSymbolRenderer::startRender( QgsRenderContext &context, const Q
 
 void QgsGraduatedSymbolRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   Q_FOREACH ( const QgsRendererRange &range, mRanges )
   {
     if ( !range.symbol() )

diff --git a/src/core/symbology/qgsheatmaprenderer.cpp b/src/core/symbology/qgsheatmaprenderer.cpp
@@ -54,7 +54,8 @@ void QgsHeatmapRenderer::initializeValues( QgsRenderContext &context )
 
 void QgsHeatmapRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
-  Q_UNUSED( fields );
+  QgsFeatureRenderer::startRender( context, fields );
+
   if ( !context.painter() )
   {
     return;
@@ -212,6 +213,8 @@ double QgsHeatmapRenderer::triangularKernel( const double distance, const int ba
 
 void QgsHeatmapRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   renderImage( context );
   mWeightExpression.reset();
 }

diff --git a/src/core/symbology/qgsinvertedpolygonrenderer.cpp b/src/core/symbology/qgsinvertedpolygonrenderer.cpp
@@ -93,6 +93,8 @@ void QgsInvertedPolygonRenderer::checkLegendSymbolItem( const QString &key, bool
 
 void QgsInvertedPolygonRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   if ( !mSubRenderer )
   {
     return;
@@ -243,6 +245,8 @@ bool QgsInvertedPolygonRenderer::renderFeature( QgsFeature &feature, QgsRenderCo
 
 void QgsInvertedPolygonRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   if ( !mSubRenderer )
   {
     return;

diff --git a/src/core/symbology/qgsnullsymbolrenderer.cpp b/src/core/symbology/qgsnullsymbolrenderer.cpp
@@ -60,14 +60,10 @@ bool QgsNullSymbolRenderer::renderFeature( QgsFeature &feature, QgsRenderContext
   return true;
 }
 
-void QgsNullSymbolRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
-{
-  Q_UNUSED( context );
-  Q_UNUSED( fields );
-}
-
 void QgsNullSymbolRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   if ( mSymbol )
   {
     mSymbol->stopRender( context );

diff --git a/src/core/symbology/qgsnullsymbolrenderer.h b/src/core/symbology/qgsnullsymbolrenderer.h
@@ -38,7 +38,6 @@ class CORE_EXPORT QgsNullSymbolRenderer : public QgsFeatureRenderer
     virtual QgsSymbol *originalSymbolForFeature( QgsFeature &feature, QgsRenderContext &context ) override;
 
     virtual bool renderFeature( QgsFeature &feature, QgsRenderContext &context, int layer = -1, bool selected = false, bool drawVertexMarker = false ) override;
-    virtual void startRender( QgsRenderContext &context, const QgsFields &fields ) override;
     virtual void stopRender( QgsRenderContext &context ) override;
     virtual bool willRenderFeature( QgsFeature &feat, QgsRenderContext &context ) override;
 

diff --git a/src/core/symbology/qgspointdistancerenderer.cpp b/src/core/symbology/qgspointdistancerenderer.cpp
@@ -287,6 +287,8 @@ bool QgsPointDistanceRenderer::willRenderFeature( QgsFeature &feat, QgsRenderCon
 
 void QgsPointDistanceRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   mRenderer->startRender( context, fields );
 
   mClusteredGroups.clear();
@@ -315,6 +317,8 @@ void QgsPointDistanceRenderer::startRender( QgsRenderContext &context, const Qgs
 
 void QgsPointDistanceRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   //printInfoDisplacementGroups(); //just for debugging
 
   Q_FOREACH ( const ClusteredGroup &group, mClusteredGroups )

diff --git a/src/core/symbology/qgsrenderer.cpp b/src/core/symbology/qgsrenderer.cpp
@@ -89,13 +89,38 @@ QSet< QString > QgsFeatureRenderer::legendKeysForFeature( QgsFeature &feature, Q
   return QSet< QString >();
 }
 
+void QgsFeatureRenderer::startRender( QgsRenderContext &, const QgsFields & )
+{
+#ifdef QGISDEBUG
+  if ( !mThread )
+  {
+    mThread = QThread::currentThread();
+  }
+  else
+  {
+    Q_ASSERT_X( mThread == QThread::currentThread(), "QgsFeatureRenderer::startRender", "startRender called in a different thread - use a cloned renderer instead" );
+  }
+#endif
+}
+
+void QgsFeatureRenderer::stopRender( QgsRenderContext & )
+{
+#ifdef QGISDEBUG
+  Q_ASSERT_X( mThread == QThread::currentThread(), "QgsFeatureRenderer::stopRender", "stopRender called in a different thread - use a cloned renderer instead" );
+#endif
+}
+
 bool QgsFeatureRenderer::filterNeedsGeometry() const
 {
   return false;
 }
 
 bool QgsFeatureRenderer::renderFeature( QgsFeature &feature, QgsRenderContext &context, int layer, bool selected, bool drawVertexMarker )
 {
+#ifdef QGISDEBUG
+  Q_ASSERT_X( mThread == QThread::currentThread(), "QgsFeatureRenderer::renderFeature", "renderFeature called in a different thread - use a cloned renderer instead" );
+#endif
+
   QgsSymbol *symbol = symbolForFeature( feature, context );
   if ( !symbol )
     return false;

diff --git a/src/core/symbology/qgsrenderer.h b/src/core/symbology/qgsrenderer.h
@@ -162,7 +162,7 @@ class CORE_EXPORT QgsFeatureRenderer
      * \warning This method is not thread safe. Before calling startRender() in a non-main thread,
      * the renderer should instead be cloned and startRender()/stopRender() called on the clone.
      */
-    virtual void startRender( QgsRenderContext &context, const QgsFields &fields ) = 0;
+    virtual void startRender( QgsRenderContext &context, const QgsFields &fields );
 
     /**
      * Must be called when a render cycle has finished, to allow the renderer to clean up.
@@ -174,7 +174,7 @@ class CORE_EXPORT QgsFeatureRenderer
      *
      * \see startRender()
      */
-    virtual void stopRender( QgsRenderContext &context ) = 0;
+    virtual void stopRender( QgsRenderContext &context );
 
     /**
      * If a renderer does not require all the features this method may be overridden
@@ -524,6 +524,11 @@ class CORE_EXPORT QgsFeatureRenderer
     QgsFeatureRenderer &operator=( const QgsFeatureRenderer & );
 #endif
 
+#ifdef QGISDEBUG
+    //! Pointer to thread in which startRender was first called
+    QThread *mThread = nullptr;
+#endif
+
     Q_DISABLE_COPY( QgsFeatureRenderer )
 };
 

diff --git a/src/core/symbology/qgsrulebasedrenderer.cpp b/src/core/symbology/qgsrulebasedrenderer.cpp
@@ -826,6 +826,8 @@ bool QgsRuleBasedRenderer::renderFeature( QgsFeature &feature,
 
 void QgsRuleBasedRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   // prepare active children
   mRootRule->startRender( context, fields, mFilter );
 
@@ -849,6 +851,8 @@ void QgsRuleBasedRenderer::startRender( QgsRenderContext &context, const QgsFiel
 
 void QgsRuleBasedRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   //
   // do the actual rendering
   //

diff --git a/src/core/symbology/qgssinglesymbolrenderer.cpp b/src/core/symbology/qgssinglesymbolrenderer.cpp
@@ -54,6 +54,8 @@ QgsSymbol *QgsSingleSymbolRenderer::originalSymbolForFeature( QgsFeature &featur
 
 void QgsSingleSymbolRenderer::startRender( QgsRenderContext &context, const QgsFields &fields )
 {
+  QgsFeatureRenderer::startRender( context, fields );
+
   if ( !mSymbol )
     return;
 
@@ -62,6 +64,8 @@ void QgsSingleSymbolRenderer::startRender( QgsRenderContext &context, const QgsF
 
 void QgsSingleSymbolRenderer::stopRender( QgsRenderContext &context )
 {
+  QgsFeatureRenderer::stopRender( context );
+
   if ( !mSymbol )
     return;