Ensure that LayerRenderJob objects are never copied

When a LayerRenderJob object is copied, it would create a new copy-constructed QgsRenderContext member as a result. But we store references to the LayerRenderJob's context in the QgsMapLayerRenderer instances, so this potentially results in using references to deleted objects. In Qt5 builds this wasn't an issue, as LayerRenderJob objects weren't being copied. But Qt6's revamped containers WERE detaching and copying LayerRenderJob objects, resulting in crashes in all the map renderer tests. So... explicitly block copying of LayerRenderJob to fix this. And then use std::vector instead of QList for storage of job lists as we can't use Qt containers with non-copy-constructible classes. (This is a step forward anyway, because the ownership of the raw pointers in LayerRenderJob has always been somewhat opaque, and by avoiding copies of LayerRenderJob we have the opportunity to later clean this up and make them unique_ptrs were appropriate) Fixes all map renders crashing on Qt6 builds
qgis · Jul 29, 2021 · 7bc0ac1 · 7bc0ac1
1 parent 3382faf
commit 7bc0ac1
Show file tree

Hide file tree

Showing 8 changed files with 257 additions and 168 deletions.
diff --git a/src/core/maprenderer/qgsmaprenderercustompainterjob.cpp b/src/core/maprenderer/qgsmaprenderercustompainterjob.cpp
@@ -161,9 +161,9 @@ void QgsMapRendererCustomPainterJob::cancelWithoutBlocking()
   }
 
   mLabelJob.context.setRenderingStopped( true );
-  for ( LayerRenderJobs::iterator it = mLayerJobs.begin(); it != mLayerJobs.end(); ++it )
+  for ( auto it = mLayerJobs.begin(); it != mLayerJobs.end(); ++it )
   {
-    it->context.setRenderingStopped( true );
+    it->context()->setRenderingStopped( true );
     if ( it->renderer && it->renderer->feedback() )
       it->renderer->feedback()->cancel();
   }
@@ -284,19 +284,19 @@ void QgsMapRendererCustomPainterJob::staticRender( QgsMapRendererCustomPainterJo
 
 void QgsMapRendererCustomPainterJob::doRender()
 {
-  bool hasSecondPass = ! mSecondPassLayerJobs.isEmpty();
+  bool hasSecondPass = ! mSecondPassLayerJobs.empty();
   QgsDebugMsgLevel( QStringLiteral( "Starting to render layer stack." ), 5 );
   QElapsedTimer renderTime;
   renderTime.start();
 
-  for ( LayerRenderJobs::iterator it = mLayerJobs.begin(); it != mLayerJobs.end(); ++it )
+  for ( auto it = mLayerJobs.begin(); it != mLayerJobs.end(); ++it )
   {
     LayerRenderJob &job = *it;
 
-    if ( job.context.renderingStopped() )
+    if ( job.context()->renderingStopped() )
       break;
 
-    if ( ! hasSecondPass && job.context.useAdvancedEffects() )
+    if ( ! hasSecondPass && job.context()->useAdvancedEffects() )
     {
       // Set the QPainter composition mode so that this layer is rendered using
       // the desired blending mode
@@ -371,7 +371,7 @@ void QgsMapRendererCustomPainterJob::doRender()
   {
     for ( LayerRenderJob &job : mSecondPassLayerJobs )
     {
-      if ( job.context.renderingStopped() )
+      if ( job.context()->renderingStopped() )
         break;
 
       if ( !job.cached )

diff --git a/src/core/maprenderer/qgsmaprenderercustompainterjob.h b/src/core/maprenderer/qgsmaprenderercustompainterjob.h
@@ -75,7 +75,7 @@ class CORE_EXPORT QgsMapRendererCustomPainterJob : public QgsMapRendererAbstract
     QgsLabelingResults *takeLabelingResults() SIP_TRANSFER override;
 
     //! \note not available in Python bindings
-    const LayerRenderJobs &jobs() const { return mLayerJobs; } SIP_SKIP
+    const std::vector< LayerRenderJob > &jobs() const { return mLayerJobs; } SIP_SKIP
 
     /**
      * Wait for the job to be finished - and keep the thread's event loop running while waiting.
@@ -145,13 +145,13 @@ class CORE_EXPORT QgsMapRendererCustomPainterJob : public QgsMapRendererAbstract
     std::unique_ptr< QgsLabelingEngine > mLabelingEngineV2;
 
     bool mActive;
-    LayerRenderJobs mLayerJobs;
+    std::vector< LayerRenderJob > mLayerJobs;
     LabelRenderJob mLabelJob;
     bool mRenderSynchronously = false;
     bool mPrepared = false;
     bool mPrepareOnly = false;
 
-    LayerRenderJobs mSecondPassLayerJobs;
+    std::vector< LayerRenderJob > mSecondPassLayerJobs;
 };