Refactor fetching calls in QGeoCameraTiles

Introduce dirty flags, remove redundant getters.
Change fetch ifdefs to switch case, so all code
gets compiled.

Change-Id: I610f20098c67089bd2c2684910283f165e1109e0
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

6 files changed, 93 insertions, 96 deletions

diff --git a/src/location/maps/qgeocameratiles.cpp b/src/location/maps/qgeocameratiles.cpp
index 9eb3873b..2c7a7505 100644
--- a/src/location/maps/qgeocameratiles.cpp
+++ b/src/location/maps/qgeocameratiles.cpp
@@ -49,11 +49,7 @@
 #include <cmath>
 
 QT_BEGIN_NAMESPACE
-#define ENABLE_PREFETCHING
-// larger values fetch a bigger set when the camera stops
 #define PREFETCH_FRUSTUM_SCALE 2.0
-// #define PREFETCH_NEIGHBOUR_LAYER
-#define PREFETCH_TWO_NEIGHBOUR_LAYERS
 
 struct Frustum
 {
@@ -87,10 +83,13 @@ public:
     int m_intZoomLevel;
     int m_sideLength;
 
+    bool m_dirtyGeometry;
+    bool m_dirtyMetadata;
+
     void updateMetadata();
     void updateGeometry(double viewExpansion = 1.0);
 
-    Frustum frustum(double fieldOfViewGradient) const;
+    Frustum createFrustum(double fieldOfViewGradient) const;
 
     class LengthSorter
     {
@@ -128,73 +127,66 @@ QGeoCameraTiles::~QGeoCameraTiles()
 {
 }
 
-void QGeoCameraTiles::findPrefetchTiles()
+QSet<QGeoTileSpec> QGeoCameraTiles::prefetchTiles(PrefetchStle style)
 {
-#if defined(ENABLE_PREFETCHING)
+    int currentIntZoom = static_cast<int>(std::floor(d_ptr->m_camera.zoomLevel()));
+    double currentFloatZoom = d_ptr->m_camera.zoomLevel();
+
     d_ptr->m_tiles.clear();
-    int zoom = static_cast<int>(std::floor(d_ptr->m_camera.zoomLevel()));
-    d_ptr->m_intZoomLevel = zoom;
-    d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
+    d_ptr->m_intZoomLevel = currentIntZoom;
+    d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel ;
     d_ptr->updateGeometry(PREFETCH_FRUSTUM_SCALE);
 
-#if defined(PREFETCH_NEIGHBOUR_LAYER)
-    double zoomFraction = d_ptr->m_camera.zoomLevel() - zoom;
-    int nearestNeighbourLayer = zoomFraction > 0.5 ? zoom + 1 : zoom - 1;
-    if (nearestNeighbourLayer <= d_ptr->m_maxZoom && nearestNeighbourLayer >= 0)
-    {
-        double oldZoom = d_ptr->m_camera.zoomLevel();
-        d_ptr->m_intZoomLevel = nearestNeighbourLayer;
-        d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
-        d_ptr->m_camera.setZoomLevel(d_ptr->m_intZoomLevel);
+    switch (style) {
 
-        // Approx heuristic, keeping total # prefetched tiles roughly independent of the
-        // fractional zoom level.
-        double neighbourScale = (1.0 + zoomFraction)/2.0;
+    case PrefetchNeighbourLayer: {
 
-        d_ptr->updateGeometry(PREFETCH_FRUSTUM_SCALE * neighbourScale);
-        d_ptr->m_camera.setZoomLevel(oldZoom);
+        double zoomFraction = d_ptr->m_camera.zoomLevel() - currentIntZoom;
+        int nearestNeighbourLayer = zoomFraction > 0.5 ? currentIntZoom + 1 : currentIntZoom - 1;
+        if (nearestNeighbourLayer <= d_ptr->m_maxZoom && nearestNeighbourLayer >= 0) {
+            d_ptr->m_intZoomLevel = nearestNeighbourLayer;
+            d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
+            d_ptr->m_camera.setZoomLevel(d_ptr->m_intZoomLevel);
+            // Approx heuristic, keeping total # prefetched tiles roughly independent of the
+            // fractional zoom level.
+            double neighbourScale = (1.0 + zoomFraction)/2.0;
+            d_ptr->updateGeometry(PREFETCH_FRUSTUM_SCALE * neighbourScale);
+        }
     }
-#elif defined(PREFETCH_TWO_NEIGHBOUR_LAYERS)
- //   int size1 = d->tiles_.size();
 
-    // This is a simpler strategy, we just prefetch from layer above and below
-    // for the layer below we only use half the size as this fills the screen
-    double oldZoom = d_ptr->m_camera.zoomLevel();
-    if (zoom > 0)
-    {
-        d_ptr->m_intZoomLevel = zoom-1;
-        d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
-        d_ptr->m_camera.setZoomLevel(d_ptr->m_intZoomLevel);
-        d_ptr->updateGeometry(0.5);
+    case PrefetchTwoNeighbourLayers: {
+        // This is a simpler strategy, we just prefetch from layer above and below
+        // for the layer below we only use half the size as this fills the screen
+        if (currentIntZoom > 0) {
+            d_ptr->m_intZoomLevel = currentIntZoom - 1;
+            d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
+            d_ptr->m_camera.setZoomLevel(d_ptr->m_intZoomLevel);
+            d_ptr->updateGeometry(0.5);
+        }
+        if (currentIntZoom < d_ptr->m_maxZoom) {
+            d_ptr->m_intZoomLevel = currentIntZoom + 1;
+            d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
+            d_ptr->m_camera.setZoomLevel(d_ptr->m_intZoomLevel);
+            d_ptr->updateGeometry(1.0);
+        }
     }
- //   int size2 = d->tiles_.size();
-    if (zoom < d_ptr->m_maxZoom)
-    {
-        d_ptr->m_intZoomLevel = zoom+1;
-        d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
-        d_ptr->m_camera.setZoomLevel(d_ptr->m_intZoomLevel);
-        d_ptr->updateGeometry(1.0);
     }
- //   qDebug() << "prefetched main tiles: " << size1 << " higher detail layer: " << d->tiles_.size() - size2 << " low detail layer: " << size2 - size1;
-    d_ptr->m_intZoomLevel = zoom;
+
+    d_ptr->m_intZoomLevel = currentIntZoom;
     d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
-    d_ptr->m_camera.setZoomLevel(oldZoom);
-#endif
-#endif
+    d_ptr->m_camera.setZoomLevel(currentFloatZoom);
+    return d_ptr->m_tiles;
 }
 
-
 void QGeoCameraTiles::setCamera(const QGeoCameraData &camera)
 {
     if (d_ptr->m_camera == camera)
         return;
-    d_ptr->m_camera = camera;
 
+    d_ptr->m_dirtyGeometry = true;
+    d_ptr->m_camera = camera;
     d_ptr->m_intZoomLevel = static_cast<int>(std::floor(d_ptr->m_camera.zoomLevel()));
     d_ptr->m_sideLength = 1 << d_ptr->m_intZoomLevel;
-
-    d_ptr->m_tiles.clear();
-    d_ptr->updateGeometry();
 }
 
 void QGeoCameraTiles::setScreenSize(const QSize &size)
@@ -202,9 +194,8 @@ void QGeoCameraTiles::setScreenSize(const QSize &size)
     if (d_ptr->m_screenSize == size)
         return;
 
+    d_ptr->m_dirtyGeometry = true;
     d_ptr->m_screenSize = size;
-    d_ptr->m_tiles.clear();
-    d_ptr->updateGeometry();
 }
 
 void QGeoCameraTiles::setPluginString(const QString &pluginString)
@@ -212,8 +203,8 @@ void QGeoCameraTiles::setPluginString(const QString &pluginString)
     if (d_ptr->m_pluginString == pluginString)
         return;
 
+    d_ptr->m_dirtyMetadata = true;
     d_ptr->m_pluginString = pluginString;
-    d_ptr->updateMetadata();
 }
 
 void QGeoCameraTiles::setMapType(const QGeoMapType &mapType)
@@ -221,8 +212,8 @@ void QGeoCameraTiles::setMapType(const QGeoMapType &mapType)
     if (d_ptr->m_mapType == mapType)
         return;
 
+    d_ptr->m_dirtyMetadata = true;
     d_ptr->m_mapType = mapType;
-    d_ptr->updateMetadata();
 }
 
 void QGeoCameraTiles::setMapVersion(const int mapVersion)
@@ -230,8 +221,8 @@ void QGeoCameraTiles::setMapVersion(const int mapVersion)
     if (d_ptr->m_mapVersion == mapVersion)
         return;
 
+    d_ptr->m_dirtyMetadata = true;
     d_ptr->m_mapVersion = mapVersion;
-    d_ptr->updateMetadata();
 }
 
 void QGeoCameraTiles::setTileSize(int tileSize)
@@ -239,9 +230,8 @@ void QGeoCameraTiles::setTileSize(int tileSize)
     if (d_ptr->m_tileSize == tileSize)
         return;
 
+    d_ptr->m_dirtyGeometry = true;
     d_ptr->m_tileSize = tileSize;
-    d_ptr->m_tiles.clear();
-    d_ptr->updateGeometry();
 }
 
 int QGeoCameraTiles::tileSize() const
@@ -254,18 +244,34 @@ void QGeoCameraTiles::setMaximumZoomLevel(int maxZoom)
     if (d_ptr->m_maxZoom == maxZoom)
         return;
 
+    d_ptr->m_dirtyGeometry = true;
     d_ptr->m_maxZoom = maxZoom;
-    d_ptr->m_tiles.clear();
-    d_ptr->updateGeometry();
 }
 
-QSet<QGeoTileSpec> QGeoCameraTiles::tiles() const
+QSet<QGeoTileSpec> QGeoCameraTiles::visibleTiles()
 {
+    if (d_ptr->m_dirtyGeometry) {
+        d_ptr->m_tiles.clear();
+        d_ptr->updateGeometry();
+        d_ptr->m_dirtyGeometry = false;
+    }
+
+    if (d_ptr->m_dirtyMetadata) {
+        d_ptr->updateMetadata();
+        d_ptr->m_dirtyMetadata = false;
+    }
+
     return d_ptr->m_tiles;
 }
 
 QGeoCameraTilesPrivate::QGeoCameraTilesPrivate()
-:   m_mapVersion(-1), m_tileSize(0), m_maxZoom(0), m_intZoomLevel(0), m_sideLength(0)
+:   m_mapVersion(-1),
+    m_tileSize(0),
+    m_maxZoom(0),
+    m_intZoomLevel(0),
+    m_sideLength(0),
+    m_dirtyGeometry(false),
+    m_dirtyMetadata(false)
 {
 }
 
@@ -292,7 +298,7 @@ void QGeoCameraTilesPrivate::updateGeometry(double viewExpansion)
 {
     // Find the frustum from the camera / screen / viewport information
     // The larger frustum when stationary is a form of prefetching
-    Frustum f = frustum(viewExpansion);
+    Frustum f = createFrustum(viewExpansion);
 
     // Find the polygon where the frustum intersects the plane of the map
     PolygonVector footprint = frustumFootprint(f);
@@ -311,7 +317,7 @@ void QGeoCameraTilesPrivate::updateGeometry(double viewExpansion)
     }
 }
 
-Frustum QGeoCameraTilesPrivate::frustum(double fieldOfViewGradient) const
+Frustum QGeoCameraTilesPrivate::createFrustum(double fieldOfViewGradient) const
 {
     QDoubleVector3D center = m_sideLength * QGeoProjection::coordToMercator(m_camera.center());
     center.setZ(0.0);
@@ -333,10 +339,7 @@ Frustum QGeoCameraTilesPrivate::frustum(double fieldOfViewGradient) const
 
     double aspectRatio = 1.0 * m_screenSize.width() / m_screenSize.height();
 
-    double hn = 0.0;
-    double wn = 0.0;
-    double hf = 0.0;
-    double wf = 0.0;
+    double hn,wn,hf,wf = 0.0;
 
     // fixes field of view at 45 degrees
     // this assumes that viewSize = 2*nearPlane x 2*nearPlane
diff --git a/src/location/maps/qgeocameratiles_p.h b/src/location/maps/qgeocameratiles_p.h
index 2a913e38..e3260283 100644
--- a/src/location/maps/qgeocameratiles_p.h
+++ b/src/location/maps/qgeocameratiles_p.h
@@ -63,6 +63,8 @@ public:
     QGeoCameraTiles();
     ~QGeoCameraTiles();
 
+    enum PrefetchStle { PrefetchNeighbourLayer, PrefetchTwoNeighbourLayers};
+
     void setCamera(const QGeoCameraData &camera);
     void setScreenSize(const QSize &size);
     void setTileSize(int tileSize);
@@ -74,8 +76,8 @@ public:
     void setMapType(const QGeoMapType &mapType);
     void setMapVersion(int mapVersion);
 
-    QSet<QGeoTileSpec> tiles() const;
-    void findPrefetchTiles();
+    QSet<QGeoTileSpec> visibleTiles();
+    QSet<QGeoTileSpec> prefetchTiles(PrefetchStle style);
 
 protected:
     QScopedPointer<QGeoCameraTilesPrivate> d_ptr;
diff --git a/src/location/maps/qgeotiledmap.cpp b/src/location/maps/qgeotiledmap.cpp
index 9ce357c7..2b35cbe1 100644
--- a/src/location/maps/qgeotiledmap.cpp
+++ b/src/location/maps/qgeotiledmap.cpp
@@ -177,10 +177,9 @@ QGeoTiledMapPrivate::~QGeoTiledMapPrivate()
 
 void QGeoTiledMapPrivate::prefetchTiles()
 {
-    m_cameraTiles->findPrefetchTiles();
-
     if (m_tileRequests)
-        m_tileRequests->requestTiles(m_cameraTiles->tiles() - m_mapScene->texturedTiles());
+        m_tileRequests->requestTiles(m_cameraTiles->prefetchTiles(QGeoCameraTiles::PrefetchTwoNeighbourLayers)
+                                     - m_mapScene->texturedTiles());
 }
 
 void QGeoTiledMapPrivate::changeCameraData(const QGeoCameraData &oldCameraData)
@@ -211,12 +210,12 @@ void QGeoTiledMapPrivate::changeCameraData(const QGeoCameraData &oldCameraData)
     m_cameraTiles->setCamera(cam);
 
     m_mapScene->setCameraData(cam);
-    m_mapScene->setVisibleTiles(m_cameraTiles->tiles());
+    m_mapScene->setVisibleTiles(m_cameraTiles->visibleTiles());
 
     if (m_tileRequests) {
         // don't request tiles that are already built and textured
         QList<QSharedPointer<QGeoTileTexture> > cachedTiles =
-                m_tileRequests->requestTiles(m_cameraTiles->tiles() - m_mapScene->texturedTiles());
+                m_tileRequests->requestTiles(m_cameraTiles->visibleTiles() - m_mapScene->texturedTiles());
 
         foreach (const QSharedPointer<QGeoTileTexture> &tex, cachedTiles) {
             m_mapScene->addTile(tex->spec, tex);
@@ -260,14 +259,14 @@ void QGeoTiledMapPrivate::mapResized(int width, int height)
         int newSize = qMax(m_cache->minTextureUsage(), texCacheSize);
         m_cache->setMinTextureUsage(newSize);
     }
-    q->evaluateCopyrights(visibleTiles());
+    q->evaluateCopyrights(m_cameraTiles->visibleTiles());
 }
 
 void QGeoTiledMapPrivate::newTileFetched(const QGeoTileSpec &spec)
 {
      Q_Q(QGeoTiledMap);
     // Only promote the texture up to GPU if it is visible
-    if (m_cameraTiles->tiles().contains(spec)){
+    if (m_cameraTiles->visibleTiles().contains(spec)){
         QSharedPointer<QGeoTileTexture> tex = m_engine.data()->getTileTexture(spec);
         if (tex) {
             m_mapScene->addTile(spec, tex);
@@ -276,11 +275,6 @@ void QGeoTiledMapPrivate::newTileFetched(const QGeoTileSpec &spec)
     }
 }
 
-QSet<QGeoTileSpec> QGeoTiledMapPrivate::visibleTiles()
-{
-    return m_cameraTiles->tiles();
-}
-
 QSGNode *QGeoTiledMapPrivate::updateSceneGraph(QSGNode *oldNode, QQuickWindow *window)
 {
     return m_mapScene->updateSceneGraph(oldNode, window);
diff --git a/src/location/maps/qgeotiledmap_p_p.h b/src/location/maps/qgeotiledmap_p_p.h
index 8f2999ef..4c69efe9 100644
--- a/src/location/maps/qgeotiledmap_p_p.h
+++ b/src/location/maps/qgeotiledmap_p_p.h
@@ -82,8 +82,6 @@ public:
     QDoubleVector2D coordinateToItemPosition(const QGeoCoordinate &coordinate) const;
 
     void newTileFetched(const QGeoTileSpec &spec);
-    QSet<QGeoTileSpec> visibleTiles();
-
     void prefetchTiles();
 
 protected:
diff --git a/tests/auto/qgeocameratiles/tst_qgeocameratiles.cpp b/tests/auto/qgeocameratiles/tst_qgeocameratiles.cpp
index 08884ad1..17639c06 100644
--- a/tests/auto/qgeocameratiles/tst_qgeocameratiles.cpp
+++ b/tests/auto/qgeocameratiles/tst_qgeocameratiles.cpp
@@ -126,11 +126,11 @@ private slots:
         ct.setScreenSize(QSize(32, 32));
         ct.setMapType(QGeoMapType(QGeoMapType::StreetMap, "street map", "street map", false, false, 1));
 
-        QSet<QGeoTileSpec> tiles1 = ct.tiles();
+        QSet<QGeoTileSpec> tiles1 = ct.visibleTiles();
 
         ct.setPluginString("pluginA");
 
-        QSet<QGeoTileSpec> tiles2 = ct.tiles();
+        QSet<QGeoTileSpec> tiles2 = ct.visibleTiles();
 
         typedef QSet<QGeoTileSpec>::const_iterator iter;
         iter i1 = tiles1.constBegin();
@@ -147,7 +147,7 @@ private slots:
 
         ct.setPluginString("pluginB");
 
-        QSet<QGeoTileSpec> tiles3 = ct.tiles();
+        QSet<QGeoTileSpec> tiles3 = ct.visibleTiles();
 
         iter i2 = tiles2.constBegin();
         iter end2 = tiles2.constEnd();
@@ -175,12 +175,12 @@ private slots:
         ct.setScreenSize(QSize(32, 32));
         ct.setPluginString("pluginA");
 
-        QSet<QGeoTileSpec> tiles1 = ct.tiles();
+        QSet<QGeoTileSpec> tiles1 = ct.visibleTiles();
 
         QGeoMapType mapType1 = QGeoMapType(QGeoMapType::StreetMap, "street map", "street map", false, false, 1);
         ct.setMapType(mapType1);
 
-        QSet<QGeoTileSpec> tiles2 = ct.tiles();
+        QSet<QGeoTileSpec> tiles2 = ct.visibleTiles();
 
         typedef QSet<QGeoTileSpec>::const_iterator iter;
         iter i1 = tiles1.constBegin();
@@ -198,7 +198,7 @@ private slots:
         QGeoMapType mapType2 = QGeoMapType(QGeoMapType::StreetMap, "satellite map", "satellite map", false, false, 2);
         ct.setMapType(mapType2);
 
-        QSet<QGeoTileSpec> tiles3 = ct.tiles();
+        QSet<QGeoTileSpec> tiles3 = ct.visibleTiles();
 
         iter i2 = tiles2.constBegin();
         iter end2 = tiles2.constEnd();
@@ -240,7 +240,7 @@ private slots:
         for (int i = 0; i < tilesX.size(); ++i)
             tiles.insert(QGeoTileSpec("", 0, static_cast<int>(std::floor(zoom)), tilesX.at(i), tilesY.at(i)));
 
-        QCOMPARE(ct.tiles(), tiles);
+        QCOMPARE(ct.visibleTiles(), tiles);
     }
 
     void tilesPositions_data()
diff --git a/tests/auto/qgeomapscene/tst_qgeomapscene.cpp b/tests/auto/qgeomapscene/tst_qgeomapscene.cpp
index 5fe8ae2e..e76e2d2a 100644
--- a/tests/auto/qgeomapscene/tst_qgeomapscene.cpp
+++ b/tests/auto/qgeomapscene/tst_qgeomapscene.cpp
@@ -293,7 +293,7 @@ class tst_QGeoMapScene : public QObject
             mapScene.setCameraData(camera);
             QVERIFY(!mapScene.verticalLock());
             mapScene.setUseVerticalLock(true);
-            mapScene.setVisibleTiles(ct.tiles());
+            mapScene.setVisibleTiles(ct.visibleTiles());
             QVERIFY(mapScene.verticalLock());
 
             // Test the case when setting vertical lock has no effect
@@ -303,7 +303,7 @@ class tst_QGeoMapScene : public QObject
             mapScene2.setCameraData(camera);
             QVERIFY(!mapScene2.verticalLock());
             mapScene2.setUseVerticalLock(true);
-            mapScene2.setVisibleTiles(ct.tiles());
+            mapScene2.setVisibleTiles(ct.visibleTiles());
             QVERIFY(!mapScene2.verticalLock());
         }
 
@@ -333,7 +333,7 @@ class tst_QGeoMapScene : public QObject
             mapGeometry.setTileSize(tileSize);
             mapGeometry.setScreenSize(QSize(screenWidth,screenHeight));
             mapGeometry.setCameraData(camera);
-            mapGeometry.setVisibleTiles(ct.tiles());
+            mapGeometry.setVisibleTiles(ct.visibleTiles());
 
             QDoubleVector2D point(screenX,screenY);
             QDoubleVector2D mecartorPos = mapGeometry.itemPositionToMercator(point);
@@ -373,7 +373,7 @@ class tst_QGeoMapScene : public QObject
             mapGeometry.setTileSize(tileSize);
             mapGeometry.setScreenSize(QSize(screenWidth,screenHeight));
             mapGeometry.setCameraData(camera);
-            mapGeometry.setVisibleTiles(ct.tiles());
+            mapGeometry.setVisibleTiles(ct.visibleTiles());
 
             QDoubleVector2D mercatorPos(mercatorX, mercatorY);
             QPointF point = mapGeometry.mercatorToItemPosition(mercatorPos).toPointF();