Don't keep pointer to QgsPointCloudLayer in QgsPointCloudLayerRenderer

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

Hide file tree

Showing 2 changed files with 30 additions and 23 deletions.
diff --git a/src/core/pointcloud/qgspointcloudlayerrenderer.cpp b/src/core/pointcloud/qgspointcloudlayerrenderer.cpp
@@ -43,42 +43,47 @@
 
 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->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();
+  if ( const QgsVirtualPointCloudProvider *vpcProvider = dynamic_cast<QgsVirtualPointCloudProvider *>( layer->dataProvider() ) )
+  {
+    mAverageSubIndexWidth = vpcProvider->averageSubIndexWidth();
+    mAverageSubIndexHeight = vpcProvider->averageSubIndexHeight();
+    mOverviewIndex = vpcProvider->overview();
+  }
+
+  mCloudExtent = layer->dataProvider()->polygonBounds();
 
   mClippingRegions = QgsMapClippingUtils::collectClippingRegionsForLayer( *renderContext(), layer );
 
@@ -129,9 +134,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,9 +198,9 @@ 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() ) )
+  else if ( mOverviewIndex )
   {
     QVector< QgsPointCloudSubIndex > visibleIndexes;
     for ( const QgsPointCloudSubIndex &si : mSubIndexes )
@@ -207,23 +210,22 @@ bool QgsPointCloudLayerRenderer::render()
         visibleIndexes.append( si );
       }
     }
-    const bool zoomedOut = renderExtent.width() > vpcProvider->averageSubIndexWidth() ||
-                           renderExtent.height() > vpcProvider->averageSubIndexHeight();
-    QgsPointCloudIndex overviewIndex = vpcProvider->overview();
+    const bool zoomedOut = renderExtent.width() > mAverageSubIndexWidth ||
+                           renderExtent.height() > mAverageSubIndexHeight;
     // if the overview of virtual point cloud exists, and we are zoomed out, we render just overview
-    if ( vpcProvider->overview() && zoomedOut &&
+    if ( mOverviewIndex && zoomedOut &&
          mRenderer->zoomOutBehavior() == Qgis::PointCloudZoomOutRenderBehavior::RenderOverview )
     {
-      renderIndex( overviewIndex );
+      renderIndex( *mOverviewIndex );
     }
     else
     {
       // if the overview of virtual point cloud exists, and we are zoomed out, but we want both overview and extents,
       // we render overview
-      if ( vpcProvider->overview() && zoomedOut &&
+      if ( mOverviewIndex && zoomedOut &&
            mRenderer->zoomOutBehavior() == Qgis::PointCloudZoomOutRenderBehavior::RenderOverviewAndExtents )
       {
-        renderIndex( overviewIndex );
+        renderIndex( *mOverviewIndex );
       }
       mSubIndexExtentRenderer->startRender( context );
       for ( const QgsPointCloudSubIndex &si : visibleIndexes )

diff --git a/src/core/pointcloud/qgspointcloudlayerrenderer.h b/src/core/pointcloud/qgspointcloudlayerrenderer.h
@@ -37,6 +37,7 @@
 #include <QString>
 #include <QPainter>
 #include <QElapsedTimer>
+#include <optional>
 
 class QgsRenderContext;
 class QgsPointCloudLayer;
@@ -79,7 +80,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;
@@ -94,7 +95,11 @@ class CORE_EXPORT QgsPointCloudLayerRenderer: public QgsMapLayerRenderer
     QgsPointCloudAttributeCollection mAttributes;
     QgsGeometry mCloudExtent;
     QList< QgsMapClippingRegion > mClippingRegions;
+
     const QVector< QgsPointCloudSubIndex > mSubIndexes;
+    std::optional<QgsPointCloudIndex> mOverviewIndex;
+    double mAverageSubIndexWidth;
+    double mAverageSubIndexHeight;
 
     int mRenderTimeHint = 0;
     bool mBlockRenderUpdates = false;