Modernize code, RAII and performance improvements

Also removes QgsQuickUtils which is no longer needed
qgis · Jun 5, 2021 · 0dca7c9 · 0dca7c9
1 parent 7f69678
commit 0dca7c9
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 256 deletions.
diff --git a/src/quickgui/CMakeLists.txt b/src/quickgui/CMakeLists.txt
@@ -5,15 +5,13 @@ set(QGIS_QUICK_GUI_MOC_HDRS
   qgsquickmapcanvasmap.h
   qgsquickmapsettings.h
   qgsquickmaptransform.h
-  qgsquickutils.h
 )
 
 set(QGIS_QUICK_GUI_SRC
   qgsquickcoordinatetransformer.cpp
   qgsquickmapcanvasmap.cpp
   qgsquickmapsettings.cpp
   qgsquickmaptransform.cpp
-  qgsquickutils.cpp
 )
 
 include_directories(

diff --git a/src/quickgui/plugin/qgsquickplugin.cpp b/src/quickgui/plugin/qgsquickplugin.cpp
@@ -34,14 +34,6 @@
 #include "qgsquickmapsettings.h"
 #include "qgsquickmaptransform.h"
 #include "qgsquickplugin.h"
-#include "qgsquickutils.h"
-
-static QObject *_utilsProvider( QQmlEngine *engine, QJSEngine *scriptEngine )
-{
-  Q_UNUSED( engine )
-  Q_UNUSED( scriptEngine )
-  return new QgsQuickUtils();  // the object will be owned by QML engine and destroyed by the engine on exit
-}
 
 void QgsQuickPlugin::registerTypes( const char *uri )
 {
@@ -65,7 +57,5 @@ void QgsQuickPlugin::registerTypes( const char *uri )
   qmlRegisterType< QgsQuickMapSettings >( uri, 0, 1, "MapSettings" );
   qmlRegisterType< QgsQuickMapTransform >( uri, 0, 1, "MapTransform" );
   qmlRegisterType< QgsVectorLayer >( uri, 0, 1, "VectorLayer" );
-
-  qmlRegisterSingletonType< QgsQuickUtils >( uri, 0, 1, "Utils", _utilsProvider );
 }
 
diff --git a/src/quickgui/qgsquickmapcanvasmap.cpp b/src/quickgui/qgsquickmapcanvasmap.cpp
@@ -14,26 +14,26 @@
  ***************************************************************************/
 
 #include <QQuickWindow>
-#include <QScreen>
 #include <QSGSimpleTextureNode>
-#include <QtConcurrent>
-
-#include "qgslabelingresults.h"
-#include "qgsmaprendererparalleljob.h"
-#include "qgsmessagelog.h"
-#include "qgspallabeling.h"
-#include "qgsproject.h"
-#include "qgsvectorlayer.h"
-#include "qgis.h"
+#include <QScreen>
+#include <qgis.h>
+#include <qgsexpressioncontextutils.h>
+#include <qgsmaprenderercache.h>
+#include <qgsmaprendererparalleljob.h>
+#include <qgsmessagelog.h>
+#include <qgspallabeling.h>
+#include <qgsproject.h>
+#include <qgsvectorlayer.h>
+#include <qgslabelingresults.h>
 
 #include "qgsquickmapcanvasmap.h"
 #include "qgsquickmapsettings.h"
-#include "qgsexpressioncontextutils.h"
 
 
 QgsQuickMapCanvasMap::QgsQuickMapCanvasMap( QQuickItem *parent )
   : QQuickItem( parent )
-  , mMapSettings( new QgsQuickMapSettings() )
+  , mMapSettings( std::make_unique<QgsQuickMapSettings>() )
+  , mCache( std::make_unique<QgsMapRendererCache>() )
 {
   connect( this, &QQuickItem::windowChanged, this, &QgsQuickMapCanvasMap::onWindowChanged );
   connect( &mRefreshTimer, &QTimer::timeout, this, &QgsQuickMapCanvasMap::refreshMap );
@@ -52,6 +52,8 @@ QgsQuickMapCanvasMap::QgsQuickMapCanvasMap( QQuickItem *parent )
   setFlags( QQuickItem::ItemHasContents );
 }
 
+QgsQuickMapCanvasMap::~QgsQuickMapCanvasMap() = default;
+
 QgsQuickMapSettings *QgsQuickMapCanvasMap::mapSettings() const
 {
   return mMapSettings.get();
@@ -62,13 +64,13 @@ void QgsQuickMapCanvasMap::zoom( QPointF center, qreal scale )
   QgsRectangle extent = mMapSettings->extent();
   QgsPoint oldCenter( extent.center() );
   QgsPoint mousePos( mMapSettings->screenToCoordinate( center ) );
+
   QgsPointXY newCenter( mousePos.x() + ( ( oldCenter.x() - mousePos.x() ) * scale ),
                         mousePos.y() + ( ( oldCenter.y() - mousePos.y() ) * scale ) );
 
   // same as zoomWithCenter (no coordinate transformations are needed)
   extent.scale( scale, &newCenter );
   mMapSettings->setExtent( extent );
-  mNeedsRefresh = true;
 }
 
 void QgsQuickMapCanvasMap::pan( QPointF oldPos, QPointF newPos )
@@ -88,7 +90,6 @@ void QgsQuickMapCanvasMap::pan( QPointF oldPos, QPointF newPos )
   extent.setYMinimum( extent.yMinimum() + dy );
 
   mMapSettings->setExtent( extent );
-  mNeedsRefresh = true;
 }
 
 void QgsQuickMapCanvasMap::refreshMap()
@@ -126,7 +127,7 @@ void QgsQuickMapCanvasMap::refreshMap()
 
   connect( mJob, &QgsMapRendererJob::renderingLayersFinished, this, &QgsQuickMapCanvasMap::renderJobUpdated );
   connect( mJob, &QgsMapRendererJob::finished, this, &QgsQuickMapCanvasMap::renderJobFinished );
-  mJob->setCache( mCache );
+  mJob->setCache( mCache.get() );
 
   mJob->start();
 
@@ -135,6 +136,9 @@ void QgsQuickMapCanvasMap::refreshMap()
 
 void QgsQuickMapCanvasMap::renderJobUpdated()
 {
+  if ( !mJob )
+    return;
+
   mImage = mJob->renderedImage();
   mImageMapSettings = mJob->mapSettings();
   mDirty = true;
@@ -150,6 +154,9 @@ void QgsQuickMapCanvasMap::renderJobUpdated()
 
 void QgsQuickMapCanvasMap::renderJobFinished()
 {
+  if ( !mJob )
+    return;
+
   const QgsMapRendererJob::Errors errors = mJob->errors();
   for ( const QgsMapRendererJob::Error &error : errors )
   {
@@ -183,20 +190,31 @@ void QgsQuickMapCanvasMap::renderJobFinished()
 
 void QgsQuickMapCanvasMap::onWindowChanged( QQuickWindow *window )
 {
-  // FIXME? the above disconnect is done potentially on a nullptr
-  // cppcheck-suppress nullPointerRedundantCheck
-  disconnect( window, &QQuickWindow::screenChanged, this, &QgsQuickMapCanvasMap::onScreenChanged );
+  if ( mWindow == window )
+    return;
+
+  if ( mWindow )
+    disconnect( mWindow, &QQuickWindow::screenChanged, this, &QgsQuickMapCanvasMap::onScreenChanged );
+
   if ( window )
   {
     connect( window, &QQuickWindow::screenChanged, this, &QgsQuickMapCanvasMap::onScreenChanged );
     onScreenChanged( window->screen() );
   }
+
+  mWindow = window;
 }
 
 void QgsQuickMapCanvasMap::onScreenChanged( QScreen *screen )
 {
   if ( screen )
+  {
+    if ( screen->devicePixelRatio() > 0 )
+    {
+      mMapSettings->setDevicePixelRatio( screen->devicePixelRatio() );
+    }
     mMapSettings->setOutputDpi( screen->physicalDotsPerInch() );
+  }
 }
 
 void QgsQuickMapCanvasMap::onExtentChanged()
@@ -209,14 +227,11 @@ void QgsQuickMapCanvasMap::onExtentChanged()
 
 void QgsQuickMapCanvasMap::updateTransform()
 {
-  QgsMapSettings currentMapSettings = mMapSettings->mapSettings();
-  QgsMapToPixel mtp = currentMapSettings.mapToPixel();
-
   QgsRectangle imageExtent = mImageMapSettings.visibleExtent();
-  QgsRectangle newExtent = currentMapSettings.visibleExtent();
-  QgsPointXY pixelPt = mtp.transform( imageExtent.xMinimum(), imageExtent.yMaximum() );
+  QgsRectangle newExtent = mMapSettings->mapSettings().visibleExtent();
   setScale( imageExtent.width() / newExtent.width() );
 
+  QgsPointXY pixelPt = mMapSettings->coordinateToScreen( QgsPoint( imageExtent.xMinimum(), imageExtent.yMaximum() ) );
   setX( pixelPt.x() );
   setY( pixelPt.y() );
 }
@@ -262,13 +277,10 @@ void QgsQuickMapCanvasMap::setFreeze( bool freeze )
 
   mFreeze = freeze;
 
-  if ( !mFreeze && mNeedsRefresh )
-  {
+  if ( mFreeze )
+    stopRendering();
+  else
     refresh();
-  }
-
-  // we are freezing or unfreezing - either way we can reset "needs refresh"
-  mNeedsRefresh = false;
 
   emit freezeChanged();
 }
@@ -297,19 +309,20 @@ QSGNode *QgsQuickMapCanvasMap::updatePaintNode( QSGNode *oldNode, QQuickItem::Up
   }
 
   QRectF rect( boundingRect() );
+  QSizeF size = mImage.size();
+  if ( !size.isEmpty() )
+    size /= mMapSettings->devicePixelRatio();
 
   // Check for resizes that change the w/h ratio
-  if ( !rect.isEmpty() &&
-       !mImage.size().isEmpty() &&
-       !qgsDoubleNear( rect.width() / rect.height(), mImage.width() / mImage.height() ) )
+  if ( !rect.isEmpty() && !size.isEmpty() && !qgsDoubleNear( rect.width() / rect.height(), ( size.width() ) / static_cast<double>( size.height() ), 3 ) )
   {
     if ( qgsDoubleNear( rect.height(), mImage.height() ) )
     {
-      rect.setHeight( rect.width() / mImage.width() * mImage.height() );
+      rect.setHeight( rect.width() / size.width() * size.height() );
     }
     else
     {
-      rect.setWidth( rect.height() / mImage.height() * mImage.width() );
+      rect.setWidth( rect.height() / size.height() * size.width() );
     }
   }
 
@@ -320,15 +333,12 @@ QSGNode *QgsQuickMapCanvasMap::updatePaintNode( QSGNode *oldNode, QQuickItem::Up
 
 void QgsQuickMapCanvasMap::geometryChanged( const QRectF &newGeometry, const QRectF &oldGeometry )
 {
-  Q_UNUSED( oldGeometry )
-  // The Qt documentation advices to call the base method here.
-  // However, this introduces instabilities and heavy performance impacts on Android.
-  // It seems on desktop disabling it prevents us from downsizing the window...
-  // Be careful when re-enabling it.
-  // QQuickItem::geometryChanged( newGeometry, oldGeometry );
-
-  mMapSettings->setOutputSize( newGeometry.size().toSize() );
-  refresh();
+  QQuickItem::geometryChanged( newGeometry, oldGeometry );
+  if ( newGeometry.size() != oldGeometry.size() )
+  {
+    mMapSettings->setOutputSize( newGeometry.size().toSize() );
+    refresh();
+  }
 }
 
 void QgsQuickMapCanvasMap::onLayersChanged()
@@ -378,7 +388,14 @@ void QgsQuickMapCanvasMap::zoomToFullExtent()
     if ( mMapSettings->destinationCrs() != layer->crs() )
     {
       QgsCoordinateTransform transform( layer->crs(), mMapSettings->destinationCrs(), mMapSettings->transformContext() );
-      extent.combineExtentWith( transform.transformBoundingBox( layer->extent() ) );
+      try
+      {
+        extent.combineExtentWith( transform.transformBoundingBox( layer->extent() ) );
+      }
+      catch ( const QgsCsException &exp )
+      {
+        // Ignore extent if it can't be transformed
+      }
     }
     else
     {
@@ -393,7 +410,7 @@ void QgsQuickMapCanvasMap::zoomToFullExtent()
 void QgsQuickMapCanvasMap::refresh()
 {
   if ( mMapSettings->outputSize().isNull() )
-    return;  // the map image size has not been set yet
+    return; // the map image size has not been set yet
 
   if ( !mFreeze )
     mRefreshTimer.start( 1 );

diff --git a/src/quickgui/qgsquickmapcanvasmap.h b/src/quickgui/qgsquickmapcanvasmap.h
@@ -16,24 +16,23 @@
 #ifndef QGSQUICKMAPCANVASMAP_H
 #define QGSQUICKMAPCANVASMAP_H
 
-#include <memory>
+#include "qgsquickmapsettings.h"
 
-#include <QtQuick/QQuickItem>
 #include <QFutureSynchronizer>
 #include <QTimer>
+#include <QtQuick/QQuickItem>
+#include <qgsmapsettings.h>
+#include <qgspoint.h>
 
-#include "qgsmapsettings.h"
-#include "qgspoint.h"
-
-#include "qgis_quick.h"
-#include "qgsquickmapsettings.h"
+#include <memory>
 
 class QgsMapRendererParallelJob;
 class QgsMapRendererCache;
 class QgsLabelingResults;
 
 /**
  * \ingroup quick
+ *
  * \brief This class implements a visual Qt Quick Item that does map rendering
  * according to the current map settings. Client code is expected to use
  * MapCanvas item rather than using this class directly.
@@ -90,8 +89,8 @@ class QUICK_EXPORT QgsQuickMapCanvasMap : public QQuickItem
 
   public:
     //! Create map canvas map
-    QgsQuickMapCanvasMap( QQuickItem *parent = nullptr );
-    ~QgsQuickMapCanvasMap() = default;
+    explicit QgsQuickMapCanvasMap( QQuickItem *parent = nullptr );
+    ~QgsQuickMapCanvasMap();
 
     QSGNode *updatePaintNode( QSGNode *oldNode, QQuickItem::UpdatePaintNodeData * ) override;
 
@@ -122,12 +121,12 @@ class QUICK_EXPORT QgsQuickMapCanvasMap : public QQuickItem
   signals:
 
     /**
-     * Emitted when a rendering is starting.
+     * Signal is emitted when a rendering is starting
      */
     void renderStarting();
 
     /**
-     * Emitted when the canvas is refreshed.
+     * Signal is emitted when a canvas is refreshed
      */
     void mapCanvasRefreshed();
 
@@ -189,17 +188,20 @@ class QUICK_EXPORT QgsQuickMapCanvasMap : public QQuickItem
     bool mPinching = false;
     QPoint mPinchStartPoint;
     QgsMapRendererParallelJob *mJob = nullptr;
-    QgsMapRendererCache *mCache = nullptr;
+    std::unique_ptr<QgsMapRendererCache> mCache;
     QgsLabelingResults *mLabelingResults = nullptr;
     QImage mImage;
     QgsMapSettings mImageMapSettings;
     QTimer mRefreshTimer;
     bool mDirty = false;
     bool mFreeze = false;
-    bool mNeedsRefresh = false;  //!< Whether refresh is needed after unfreezing
     QList<QMetaObject::Connection> mLayerConnections;
     QTimer mMapUpdateTimer;
     bool mIncrementalRendering = false;
+
+    QQuickWindow *mWindow = nullptr;
+
+    QSizeF mOutputSize;
 };
 
 #endif // QGSQUICKMAPCANVASMAP_H