Fix crash when very large coordinates are stored in

QgsRenderedItemResults spatial index
qgis · Sep 7, 2021 · 9b477ba · 9b477ba
1 parent 5345903
commit 9b477ba
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 10 deletions.
diff --git a/python/core/auto_generated/maprenderer/qgsrendereditemresults.sip.in b/python/core/auto_generated/maprenderer/qgsrendereditemresults.sip.in
@@ -23,7 +23,14 @@ Stores collated details of rendered items during a map rendering operation.
 #include "qgsrendereditemresults.h"
 %End
   public:
-    QgsRenderedItemResults();
+
+    QgsRenderedItemResults( const QgsRectangle &extent = QgsRectangle() );
+%Docstring
+Constructor for QgsRenderedItemResults.
+
+The ``extent`` argument can be used to specify an expected maximal extent for items which
+will be stored in the results. This helps to optimise the spatial indices used by the object.
+%End
     ~QgsRenderedItemResults();
 
 

diff --git a/src/core/maprenderer/qgsmaprendererjob.cpp b/src/core/maprenderer/qgsmaprendererjob.cpp
@@ -131,7 +131,7 @@ bool LayerRenderJob::imageCanBeComposed() const
 
 QgsMapRendererJob::QgsMapRendererJob( const QgsMapSettings &settings )
   : mSettings( settings )
-  , mRenderedItemResults( std::make_unique< QgsRenderedItemResults >() )
+  , mRenderedItemResults( std::make_unique< QgsRenderedItemResults >( settings.extent() ) )
 {}
 
 QgsMapRendererJob::~QgsMapRendererJob() = default;

diff --git a/src/core/maprenderer/qgsrendereditemresults.cpp b/src/core/maprenderer/qgsrendereditemresults.cpp
@@ -26,6 +26,15 @@ class QgsRenderedItemResultsSpatialIndex : public RTree<const QgsRenderedItemDet
 {
   public:
 
+    explicit QgsRenderedItemResultsSpatialIndex( const QgsRectangle &maxBounds )
+      : mXMin( maxBounds.xMinimum() )
+      , mYMin( maxBounds.yMinimum() )
+      , mXRes( ( std::numeric_limits< float >::max() - 1 ) / ( maxBounds.xMaximum() - maxBounds.xMinimum() ) )
+      , mYRes( ( std::numeric_limits< float >::max() - 1 ) / ( maxBounds.yMaximum() - maxBounds.yMinimum() ) )
+      , mMaxBounds( maxBounds )
+      , mUseScale( !maxBounds.isNull() )
+    {}
+
     void insert( const QgsRenderedItemDetails *details, const QgsRectangle &bounds )
     {
       std::array< float, 4 > scaledBounds = scaleBounds( bounds );
@@ -59,9 +68,25 @@ class QgsRenderedItemResultsSpatialIndex : public RTree<const QgsRenderedItemDet
     }
 
   private:
+    double mXMin = 0;
+    double mYMin = 0;
+    double mXRes = 1;
+    double mYRes = 1;
+    QgsRectangle mMaxBounds;
+    bool mUseScale = false;
+
     std::array<float, 4> scaleBounds( const QgsRectangle &bounds ) const
     {
-      return
+      if ( mUseScale )
+        return
+      {
+        static_cast< float >( ( std::max( bounds.xMinimum(), mMaxBounds.xMinimum() ) - mXMin ) / mXRes ),
+        static_cast< float >( ( std::max( bounds.yMinimum(), mMaxBounds.yMinimum() ) - mYMin ) / mYRes ),
+        static_cast< float >( ( std::min( bounds.xMaximum(), mMaxBounds.xMaximum() ) - mXMin ) / mXRes ),
+        static_cast< float >( ( std::min( bounds.yMaximum(), mMaxBounds.yMaximum() ) - mYMin ) / mYRes )
+      };
+      else
+        return
       {
         static_cast< float >( bounds.xMinimum() ),
         static_cast< float >( bounds.yMinimum() ),
@@ -72,8 +97,9 @@ class QgsRenderedItemResultsSpatialIndex : public RTree<const QgsRenderedItemDet
 };
 ///@endcond
 
-QgsRenderedItemResults::QgsRenderedItemResults()
-  : mAnnotationItemsIndex( std::make_unique< QgsRenderedItemResultsSpatialIndex >() )
+QgsRenderedItemResults::QgsRenderedItemResults( const QgsRectangle &extent )
+  : mExtent( extent )
+  , mAnnotationItemsIndex( std::make_unique< QgsRenderedItemResultsSpatialIndex >( mExtent ) )
 {
 
 }

diff --git a/src/core/maprenderer/qgsrendereditemresults.h b/src/core/maprenderer/qgsrendereditemresults.h
@@ -20,14 +20,14 @@
 
 #include "qgis_core.h"
 #include "qgis_sip.h"
+#include "qgsrectangle.h"
 #include <memory>
 #include <QList>
 #include <vector>
 
 class QgsRenderedItemDetails;
 class QgsRenderContext;
 class QgsRenderedAnnotationItemDetails;
-class QgsRectangle;
 
 ///@cond PRIVATE
 class QgsRenderedItemResultsSpatialIndex;
@@ -41,7 +41,14 @@ class QgsRenderedItemResultsSpatialIndex;
 class CORE_EXPORT QgsRenderedItemResults
 {
   public:
-    QgsRenderedItemResults();
+
+    /**
+     * Constructor for QgsRenderedItemResults.
+     *
+     * The \a extent argument can be used to specify an expected maximal extent for items which
+     * will be stored in the results. This helps to optimise the spatial indices used by the object.
+     */
+    QgsRenderedItemResults( const QgsRectangle &extent = QgsRectangle() );
     ~QgsRenderedItemResults();
 
     //! QgsRenderedItemResults cannot be copied.
@@ -100,6 +107,8 @@ class CORE_EXPORT QgsRenderedItemResults
     QgsRenderedItemResults( const QgsRenderedItemResults & );
 #endif
 
+    QgsRectangle mExtent;
+
     std::vector< std::unique_ptr< QgsRenderedItemDetails > > mDetails;
     std::unique_ptr< QgsRenderedItemResultsSpatialIndex > mAnnotationItemsIndex;
 

diff --git a/src/gui/qgsmapcanvas.cpp b/src/gui/qgsmapcanvas.cpp
@@ -496,14 +496,13 @@ void QgsMapCanvas::setCachingEnabled( bool enabled )
   if ( enabled )
   {
     mCache = new QgsMapRendererCache;
-    mPreviousRenderedItemResults = std::make_unique< QgsRenderedItemResults >();
   }
   else
   {
     delete mCache;
     mCache = nullptr;
-    mPreviousRenderedItemResults.reset();
   }
+  mPreviousRenderedItemResults.reset();
 }
 
 bool QgsMapCanvas::isCachingEnabled() const
@@ -517,7 +516,9 @@ void QgsMapCanvas::clearCache()
     mCache->clear();
 
   if ( mPreviousRenderedItemResults )
-    mPreviousRenderedItemResults = std::make_unique< QgsRenderedItemResults >();
+    mPreviousRenderedItemResults.reset();
+  if ( mRenderedItemResults )
+    mRenderedItemResults.reset();
 }
 
 void QgsMapCanvas::setParallelRenderingEnabled( bool enabled )
@@ -724,6 +725,10 @@ void QgsMapCanvas::rendererJobFinished()
       // also transfer any results from previous renders which happened before this
       renderedItemResults->transferResults( mPreviousRenderedItemResults.get(), mJob->layersRedrawnFromCache() );
     }
+
+    if ( mCache && !mPreviousRenderedItemResults )
+      mPreviousRenderedItemResults = std::make_unique< QgsRenderedItemResults >( mJob->mapSettings().extent() );
+
     if ( mRenderedItemResults && mPreviousRenderedItemResults )
     {
       // for other layers which ARE present in the most recent rendered item results BUT were not part of this render, we