Don't keep pointer to QgsPointCloudLayer in QgsPointCloudLayerRenderer

qgis · Jan 8, 2025 · f481092 · f481092
1 parent 9e2efff
commit f481092
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 15 deletions.
diff --git a/src/core/pointcloud/qgspointcloudlayerrenderer.cpp b/src/core/pointcloud/qgspointcloudlayerrenderer.cpp
@@ -43,42 +43,40 @@
 
 QgsPointCloudLayerRenderer::QgsPointCloudLayerRenderer( QgsPointCloudLayer *layer, QgsRenderContext &context )
   : QgsMapLayerRenderer( layer->id(), &context )
-  , mLayer( layer )
+  , mIndex( layer->dataProvider()->index() )
   , mLayerName( layer->name() )
   , mLayerAttributes( layer->attributes() )
   , mSubIndexes( layer && layer->dataProvider() ? layer->dataProvider()->subIndexes() : QVector<QgsPointCloudSubIndex>() )
   , mFeedback( new QgsFeedback )
   , mEnableProfile( context.flags() & Qgis::RenderContextFlag::RecordProfile )
 {
-  // TODO: we must not keep pointer to mLayer (it's dangerous) - we must copy anything we need for rendering
-  // or use some locking to prevent read/write from multiple threads
-  if ( !mLayer || !mLayer->dataProvider() || !mLayer->renderer() )
+  if ( !layer || !layer->dataProvider() || !layer->renderer() )
     return;
 
   QElapsedTimer timer;
   timer.start();
 
-  mRenderer.reset( mLayer->renderer()->clone() );
+  mRenderer.reset( layer->renderer()->clone() );
   if ( !mSubIndexes.isEmpty() )
   {
     mSubIndexExtentRenderer.reset( new QgsPointCloudExtentRenderer() );
     mSubIndexExtentRenderer->setShowLabels( mRenderer->showLabels() );
     mSubIndexExtentRenderer->setLabelTextFormat( mRenderer->labelTextFormat() );
   }
 
-  if ( mLayer->dataProvider()->index() )
+  if ( mIndex )
   {
-    mScale = mLayer->dataProvider()->index().scale();
-    mOffset = mLayer->dataProvider()->index().offset();
+    mScale = mIndex.scale();
+    mOffset = mIndex.offset();
   }
 
-  if ( const QgsPointCloudLayerElevationProperties *elevationProps = qobject_cast< const QgsPointCloudLayerElevationProperties * >( mLayer->elevationProperties() ) )
+  if ( const QgsPointCloudLayerElevationProperties *elevationProps = qobject_cast< const QgsPointCloudLayerElevationProperties * >( layer->elevationProperties() ) )
   {
     mZOffset = elevationProps->zOffset();
     mZScale = elevationProps->zScale();
   }
 
-  mCloudExtent = mLayer->dataProvider()->polygonBounds();
+  mCloudExtent = layer->dataProvider()->polygonBounds();
 
   mClippingRegions = QgsMapClippingUtils::collectClippingRegionsForLayer( *renderContext(), layer );
 
@@ -129,9 +127,7 @@ bool QgsPointCloudLayerRenderer::render()
     return true;
   }
 
-  // TODO cache!?
-  QgsPointCloudIndex pc = mLayer->dataProvider()->index();
-  if ( mSubIndexes.isEmpty() && ( !pc || !pc.isValid() ) )
+  if ( mSubIndexes.isEmpty() && ( !mIndex || !mIndex.isValid() ) )
   {
     mReadyToCompose = true;
     return false;
@@ -195,7 +191,7 @@ bool QgsPointCloudLayerRenderer::render()
   bool canceled = false;
   if ( mSubIndexes.isEmpty() )
   {
-    canceled = !renderIndex( pc );
+    canceled = !renderIndex( mIndex );
   }
   else if ( const QgsVirtualPointCloudProvider *vpcProvider = dynamic_cast<QgsVirtualPointCloudProvider *>( mLayer->dataProvider() ) )
   {

diff --git a/src/core/pointcloud/qgspointcloudlayerrenderer.h b/src/core/pointcloud/qgspointcloudlayerrenderer.h
@@ -79,7 +79,7 @@ class CORE_EXPORT QgsPointCloudLayerRenderer: public QgsMapLayerRenderer
     void renderTriangulatedSurface( QgsPointCloudRenderContext &context );
     bool renderIndex( QgsPointCloudIndex &pc );
 
-    QgsPointCloudLayer *mLayer = nullptr;
+    QgsPointCloudIndex mIndex;
     QString mLayerName;
 
     std::unique_ptr< QgsPointCloudRenderer > mRenderer;