From 5777b8e31d12c7e8864ed3a99f0ef2bb1bfafd67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Ko=C5=88a=C5=99=C3=ADk?= Date: Wed, 8 Jan 2025 20:16:19 +0100 Subject: [PATCH] Don't keep pointer to QgsPointCloudLayer in QgsPointCloudLayerRenderer --- .../pointcloud/qgspointcloudlayerrenderer.cpp | 46 ++++++++++--------- .../pointcloud/qgspointcloudlayerrenderer.h | 7 ++- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/core/pointcloud/qgspointcloudlayerrenderer.cpp b/src/core/pointcloud/qgspointcloudlayerrenderer.cpp index 7194fe8184876..7c2195ead186b 100644 --- a/src/core/pointcloud/qgspointcloudlayerrenderer.cpp +++ b/src/core/pointcloud/qgspointcloudlayerrenderer.cpp @@ -43,22 +43,20 @@ 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() ) , 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() ); @@ -66,19 +64,26 @@ QgsPointCloudLayerRenderer::QgsPointCloudLayerRenderer( QgsPointCloudLayer *laye 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( 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( 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 index 8f94afc753ffb..731a34c4615d6 100644 --- a/src/core/pointcloud/qgspointcloudlayerrenderer.h +++ b/src/core/pointcloud/qgspointcloudlayerrenderer.h @@ -37,6 +37,7 @@ #include #include #include +#include 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 mOverviewIndex; + double mAverageSubIndexWidth; + double mAverageSubIndexHeight; int mRenderTimeHint = 0; bool mBlockRenderUpdates = false;