Fixed defects found in the review

 - Fixes reviewed by Kaj Wallin

diff --git a/src/map/baselocationitem.cpp b/src/map/baselocationitem.cpp
index 539f9a1..2643cd5 100644 (file)
--- a/src/map/baselocationitem.cpp
+++ b/src/map/baselocationitem.cpp
@@ -51,7 +51,7 @@ QRect BaseLocationItem::sceneTransformedBoundingRect(int zoomLevel) const
     // make sure that left edge of the returned rect is always inside the world coordinates
     // so collisions are detected even if the items are spanned around
 
-    while (rect.left() < 0)
+    while (rect.left() < MAP_MIN_PIXEL_X)
         rect.translate(MAP_PIXELS_X, 0);
 
     while (rect.left() > MAP_PIXELS_X - 1)

diff --git a/src/map/mapcommon.h b/src/map/mapcommon.h
index fb02134..1c00ec4 100644 (file)
--- a/src/map/mapcommon.h
+++ b/src/map/mapcommon.h
@@ -25,8 +25,9 @@
 
 #include <QtCore>
 
-const int TILE_SIZE_X = 256; ///< Tile image size in x direction
-const int TILE_SIZE_Y = 256; ///< Tile image size in y direction
+const int TILE_SIZE_X = 256;      ///< Tile image size in x direction
+const int TILE_SIZE_Y = 256;      ///< Tile image size in y direction
+const int MAP_TILE_MIN_INDEX = 0; ///< First index number of map tiles
 
 const int MIN_MAP_ZOOM_LEVEL = 0; ///< Minimum zoom level
 const int MAX_MAP_ZOOM_LEVEL = 18; ///< Maximum zoom level
@@ -39,8 +40,9 @@ const int MAP_PIXELS_X = MAX_TILES_PER_SIDE * TILE_SIZE_X;
 const int MAP_MIN_PIXEL_X = 0;
 const int MAP_MAX_PIXEL_X = MAP_PIXELS_X - 1;
 
-const int MAP_SCENE_MIN_PIXEL_X = -MAP_PIXELS_X / 2;
-const int MAP_SCENE_MAX_PIXEL_X = MAP_PIXELS_X * 2;
+const double MAP_SCENE_VERTICAL_OVERSIZE_FACTOR = 0.5;
+const int MAP_SCENE_MIN_PIXEL_X = -MAP_PIXELS_X * MAP_SCENE_VERTICAL_OVERSIZE_FACTOR;
+const int MAP_SCENE_MAX_PIXEL_X = MAP_PIXELS_X * (1 + MAP_SCENE_VERTICAL_OVERSIZE_FACTOR) - 1;
 
 /**
 * @var DEFAULT_START_ZOOM_LEVEL

diff --git a/src/map/mapengine.cpp b/src/map/mapengine.cpp
index 7f0eae9..43b9355 100644 (file)
--- a/src/map/mapengine.cpp
+++ b/src/map/mapengine.cpp
@@ -129,7 +129,7 @@ QPoint MapEngine::convertLatLonToSceneCoordinate(QPointF latLonCoordinate)
     if ((latitude > MAX_LATITUDE) || (latitude < MIN_LATITUDE))
         return QPoint(UNDEFINED, UNDEFINED);
 
-    qreal z = static_cast<qreal>(1 << MAX_MAP_ZOOM_LEVEL);
+    qreal z = static_cast<qreal>(MapEngine::tilesPerSide(MAX_MAP_ZOOM_LEVEL));
 
     qreal x = static_cast<qreal>((longitude + 180.0) / 360.0);
     qreal y = static_cast<qreal>((1.0 - log(tan(latitude * M_PI / 180.0) + 1.0
@@ -216,17 +216,16 @@ void MapEngine::getTiles(QPoint sceneCoordinate)
     int bottomRightX = m_viewTilesGrid.bottomRight().x();
     int bottomRightY = m_viewTilesGrid.bottomRight().y();
 
-    int tileMaxVal = tileMaxValue(m_zoomLevel);
+    int tileMaxVal = tileMaxIndex(m_zoomLevel);
 
     for (int x = topLeftX; x <= bottomRightX; ++x) {
         for (int y = topLeftY; y <= bottomRightY; ++y) {
 
-            // map doesn't span in vertical direction
-            if (y < 0 || y > tileMaxVal)
-                continue;
-
-            if (!m_mapScene->tileInScene(tilePath(m_zoomLevel, x, y)))
-                emit fetchImage(m_zoomLevel, normalize(x, 0, tileMaxVal), y);
+            // map doesn't span in vertical direction, so y index must be inside the limits
+            if (y >= MAP_TILE_MIN_INDEX && y <= tileMaxVal) {
+                if (!m_mapScene->tileInScene(tilePath(m_zoomLevel, x, y)))
+                    emit fetchImage(m_zoomLevel, normalize(x, MAP_TILE_MIN_INDEX, tileMaxVal), y);
+            }
         }
     }
 }
@@ -324,13 +323,13 @@ void MapEngine::mapImageReceived(int zoomLevel, int x, int y, const QPixmap &ima
 
     // duplicate to east side? (don't need to duplicate over padding)
     if (tileNumber.x() < (tilesGridWidthHalf - GRID_PADDING)) {
-        QPoint adjustedTileNumber(tileNumber.x() + tileMaxValue(zoomLevel) + 1, tileNumber.y());
+        QPoint adjustedTileNumber(tileNumber.x() + tileMaxIndex(zoomLevel) + 1, tileNumber.y());
         m_mapScene->addTile(zoomLevel, adjustedTileNumber, image, m_zoomLevel);
     }
 
     // duplicate to west side? (don't need to duplicate over padding)
-    if (tileNumber.x() > (tileMaxValue(zoomLevel) - tilesGridWidthHalf + GRID_PADDING - 1)) {
-        QPoint adjustedTileNumber(tileNumber.x() - tileMaxValue(zoomLevel) - 1, tileNumber.y());
+    if (tileNumber.x() > (tileMaxIndex(zoomLevel) - tilesGridWidthHalf + GRID_PADDING)) {
+        QPoint adjustedTileNumber(tileNumber.x() - tileMaxIndex(zoomLevel) - 1, tileNumber.y());
         m_mapScene->addTile(zoomLevel, adjustedTileNumber, image, m_zoomLevel);
     }
 }
@@ -456,11 +455,12 @@ void MapEngine::setViewLocation(QPointF latLonCoordinate)
     setLocation(sceneCoordinate);
 }
 
-int MapEngine::tileMaxValue(int zoomLevel)
+int MapEngine::tileMaxIndex(int zoomLevel)
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    return (1 << zoomLevel) - 1;
+    // subtract one because first tile index is zero
+    return tilesPerSide(zoomLevel) - 1;
 }
 
 QString MapEngine::tilePath(int zoomLevel, int x, int y)
@@ -474,6 +474,11 @@ QString MapEngine::tilePath(int zoomLevel, int x, int y)
     return tilePathString;
 }
 
+int MapEngine::tilesPerSide(int zoomLevel)
+{
+    return (1 << zoomLevel);
+}
+
 void MapEngine::updateViewTilesSceneRect()
 {
     qDebug() << __PRETTY_FUNCTION__;
@@ -517,14 +522,13 @@ void MapEngine::viewZoomFinished()
         emit maxZoomLevelReached();
     else if (m_zoomLevel == MIN_VIEW_ZOOM_LEVEL)
         emit minZoomLevelReached();
-
-    m_mapScene->setZoomLevel(m_zoomLevel);
 }
 
 void MapEngine::zoomed()
 {
     emit zoomLevelChanged(m_zoomLevel);
     m_mapScene->setTilesDrawingLevels(m_zoomLevel);
+    m_mapScene->setZoomLevel(m_zoomLevel);
     getTiles(m_sceneCoordinate);
     m_mapScene->setSceneVerticalOverlap(m_viewSize.height(), m_zoomLevel);
     m_mapScene->spanItems(m_zoomLevel, m_sceneCoordinate, m_viewSize);

diff --git a/src/map/mapengine.h b/src/map/mapengine.h
index 56fc234..1aeccc2 100644 (file)
--- a/src/map/mapengine.h
+++ b/src/map/mapengine.h
@@ -169,6 +169,14 @@ public:
     */
     static QString tilePath(int zoomLevel, int x, int y);
 
+    /**
+    * @brief Maximum number of individual tiles per side at given zoom level
+    *
+    * @param zoomLevel Zoom level
+    * @return amount of tiles per side at given zoom level
+    */
+    static int tilesPerSide(int zoomLevel);
+
 public slots:
     /**
     * @brief Slot to catch user own location data
@@ -215,7 +223,7 @@ public slots:
     * @param zoomLevel zoom level
     * @return int tile's maximum value
     */
-    static int tileMaxValue(int zoomLevel);
+    static int tileMaxIndex(int zoomLevel);
 
     /**
     * @brief Slot for view resizing.

diff --git a/src/map/mapfetcher.cpp b/src/map/mapfetcher.cpp
index 3eb173e..7d4ce6d 100644 (file)
--- a/src/map/mapfetcher.cpp
+++ b/src/map/mapfetcher.cpp
@@ -196,9 +196,6 @@ bool MapFetcher::loadImageFromCache(const QUrl &url)
         parseURL(url, &zoomLevel, &x, &y);
         int originalZoomLevel = zoomLevel;
 
-//        QTime time;
-//        time.start();
-
         // try to fetch requested and upper level images until found or MAX_UPPER_LEVELS
         // limit is reached
         const int MAX_UPPER_LEVELS = 4;
@@ -214,11 +211,9 @@ bool MapFetcher::loadImageFromCache(const QUrl &url)
                 delete cacheImage;
             }
         } while (!imageFound
-                 && (originalZoomLevel - zoomLevel) < MAX_UPPER_LEVELS
+                 && ((originalZoomLevel - zoomLevel) < MAX_UPPER_LEVELS)
                  && translateIndexesToUpperLevel(zoomLevel, x, y));
 
-//        qWarning() << __PRETTY_FUNCTION__ << "time:" << time.elapsed() << "ms";
-
         // check expiration if image was found from requested level
         if (imageFound && (originalZoomLevel == zoomLevel)) {
             // check if image is expired

diff --git a/src/map/mapscene.cpp b/src/map/mapscene.cpp
index 29edb27..b7b9508 100644 (file)
--- a/src/map/mapscene.cpp
+++ b/src/map/mapscene.cpp
@@ -38,19 +38,16 @@ MapScene::MapScene(QObject *parent)
     qDebug() << __PRETTY_FUNCTION__;
 
     setBackgroundBrush(Qt::lightGray);
-    setSceneRect(MAP_SCENE_MIN_PIXEL_X, MAP_MIN_PIXEL_Y, MAP_SCENE_MAX_PIXEL_X, MAP_MAX_PIXEL_Y);
+    setSceneRect(QRect(QPoint(MAP_SCENE_MIN_PIXEL_X, MAP_MIN_PIXEL_Y),
+                       QPoint(MAP_SCENE_MAX_PIXEL_X, MAP_MAX_PIXEL_Y)));
 }
 
 void MapScene::addTile(int tileZoomLevel, QPoint tileNumber, const QPixmap &image, int viewZoomLevel)
 {
 //    qWarning() << __PRETTY_FUNCTION__ << "x:" << tileNumber.x() << "y:" << tileNumber.y();
 
-    // tile might already be in the scene if:
-    //    - expired tile was returned from the cache to be temporarily displayed while downloading
-    //      the fresh one.
-    //    - upper level tile is not cleaned after zooming in and back to out (not scrolled enough)
-    //    - upper level tile was earlier returned from cache while downloading the requested tile
-    //      and user has zoomed up to that level
+    // tile might already be in the scene if expired tile was returned from the cache to be
+    // temporarily shown while downloading the fresh one.
     QString hashKey = MapEngine::tilePath(tileZoomLevel, tileNumber.x(), tileNumber.y());
     MapTile *oldTile = tileInScene(hashKey);
     if (oldTile)
@@ -87,9 +84,8 @@ void MapScene::moveIntersectingItemsHorizontally(QRect from, int distance)
 
     QList<QGraphicsItem *> spanItems = items(from, Qt::IntersectsItemBoundingRect);
     foreach (QGraphicsItem *item, spanItems) {
-        if (dynamic_cast<MapTile *>(item))
-            continue;
-        item->moveBy(distance, 0);
+        if (!dynamic_cast<MapTile *>(item))
+            item->moveBy(distance, 0);
     }
 }
 
@@ -154,7 +150,9 @@ void MapScene::removeOutOfViewTiles(QRect tilesGrid, int zoomLevel)
     // note: add 1 so odd values are rounded up
     int tilesGridWidthHalf = (tilesGrid.width() + 1) / 2;
 
-    if (tilesGrid.right() > (MapEngine::tileMaxValue(zoomLevel) - tilesGridWidthHalf + GRID_PADDING)) {
+    // if view is near east limit of the map, then there is duplicate tiles also on the opposite
+    // side of the world which are removed from allTiles
+    if (tilesGrid.right() > (MapEngine::tileMaxIndex(zoomLevel) - tilesGridWidthHalf + GRID_PADDING)) {
         QRect oppositeRect = m_tilesSceneRect;
         oppositeRect.translate(-MAP_PIXELS_X, 0);
         QList<QGraphicsItem *> oppositeItems = items(oppositeRect, Qt::IntersectsItemBoundingRect);
@@ -162,6 +160,8 @@ void MapScene::removeOutOfViewTiles(QRect tilesGrid, int zoomLevel)
             allItems.removeOne(item);
     }
 
+    // if view is near west limit of the map, then there is duplicate tiles also on the opposite
+    // side of the world which are removed from allTiles
     if (tilesGrid.left() < (tilesGridWidthHalf - GRID_PADDING)) {
         QRect oppositeRect = m_tilesSceneRect;
         oppositeRect.translate(MAP_PIXELS_X, 0);
@@ -243,13 +243,16 @@ void MapScene::setTilesDrawingLevels(int zoomLevel)
 
 void MapScene::setTilesGrid(QRect grid)
 {
+    qDebug() << __PRETTY_FUNCTION__ ;
+
     m_viewTilesGrid = grid;
 }
 
 void MapScene::setZoomLevel(int zoomLevel)
 {
+    qDebug() << __PRETTY_FUNCTION__ ;
+
     m_zoomLevel = zoomLevel;
-//    QTimer::singleShot(10000, this, SLOT(removeOtherLevelTiles()));
 }
 
 void MapScene::spanItems(int zoomLevel, QPoint sceneCoordinate, QSize viewSize)

diff --git a/src/map/maptile.cpp b/src/map/maptile.cpp
index ca8c7ae..90ce801 100644 (file)
--- a/src/map/maptile.cpp
+++ b/src/map/maptile.cpp
@@ -88,16 +88,22 @@ void MapTile::setPosition()
 {
     qDebug() << __PRETTY_FUNCTION__;
 
-    const int MAX_TILE_NUMBER = MapEngine::tileMaxValue(m_zoomLevel);
-    const int MIN_TILE_NUMBER_X = -(MAX_TILE_NUMBER + 1) / 2;
-    const int MAX_TULE_NUMBER_X = MAX_TILE_NUMBER * 1.5 + 1;
+    const int MAX_TILE_NUMBER = MapEngine::tileMaxIndex(m_zoomLevel);
 
-    if ((m_zoomLevel >= MIN_MAP_ZOOM_LEVEL) && (m_zoomLevel <= MAX_MAP_ZOOM_LEVEL)
-        && (m_tileNumber.x() >= MIN_TILE_NUMBER_X) && (m_tileNumber.x() <= MAX_TULE_NUMBER_X)
-        && (m_tileNumber.y() >= 0) && (m_tileNumber.y() <= MAX_TILE_NUMBER))
+    // calculate min & max horizontal tile index numbers
+    const int INDEX_0_TILE = 1;
+    const int MIN_TILE_NUMBER_X = -MapEngine::tilesPerSide(m_zoomLevel)
+                                  * MAP_SCENE_VERTICAL_OVERSIZE_FACTOR;
+    const int MAX_TILE_NUMBER_X = MapEngine::tilesPerSide(m_zoomLevel)
+                                  * (1 + MAP_SCENE_VERTICAL_OVERSIZE_FACTOR)
+                                  - INDEX_0_TILE;
 
-        setPos(MapEngine::convertTileNumberToSceneCoordinate(m_zoomLevel, m_tileNumber));
-    else
+    if ((m_zoomLevel >= MIN_MAP_ZOOM_LEVEL) && (m_zoomLevel <= MAX_MAP_ZOOM_LEVEL)
+        && (m_tileNumber.x() >= MIN_TILE_NUMBER_X) && (m_tileNumber.x() <= MAX_TILE_NUMBER_X)
+        && (m_tileNumber.y() >= 0) && (m_tileNumber.y() <= MAX_TILE_NUMBER)) {
+            setPos(MapEngine::convertTileNumberToSceneCoordinate(m_zoomLevel, m_tileNumber));
+    } else {
         setPos(UNDEFINED, UNDEFINED);
+    }
 }