From 804bb7d0a7624bb6786045d0ab60a453e8267e7a Mon Sep 17 00:00:00 2001 From: =?utf8?q?Sami=20R=C3=A4m=C3=B6?= Date: Fri, 11 Jun 2010 16:25:51 +0300 Subject: [PATCH] Fixed defects found in the review - Fixes reviewed by Kaj Wallin --- src/map/baselocationitem.cpp | 2 +- src/map/mapcommon.h | 10 ++++++---- src/map/mapengine.cpp | 34 +++++++++++++++++++--------------- src/map/mapengine.h | 10 +++++++++- src/map/mapfetcher.cpp | 7 +------ src/map/mapscene.cpp | 27 +++++++++++++++------------ src/map/maptile.cpp | 22 ++++++++++++++-------- 7 files changed, 65 insertions(+), 47 deletions(-) diff --git a/src/map/baselocationitem.cpp b/src/map/baselocationitem.cpp index 539f9a1..2643cd5 100644 --- 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 --- a/src/map/mapcommon.h +++ b/src/map/mapcommon.h @@ -25,8 +25,9 @@ #include -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 --- 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(1 << MAX_MAP_ZOOM_LEVEL); + qreal z = static_cast(MapEngine::tilesPerSide(MAX_MAP_ZOOM_LEVEL)); qreal x = static_cast((longitude + 180.0) / 360.0); qreal y = static_cast((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 --- 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 --- 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 --- 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 spanItems = items(from, Qt::IntersectsItemBoundingRect); foreach (QGraphicsItem *item, spanItems) { - if (dynamic_cast(item)) - continue; - item->moveBy(distance, 0); + if (!dynamic_cast(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 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 --- 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); + } } -- 1.7.9.5