From 930f897e13c422add83722c6cd790f30f3ee1eac Mon Sep 17 00:00:00 2001 From: Paolo Angelelli Date: Mon, 27 Feb 2017 16:20:30 +0100 Subject: Fix incorrect handling of provider resolution for offline storage This patch fixes the incorrect handling of the population/repopulation of OSM offline tiles when the providers are initially populated. In this way it is now fine if the user selects high dpi tiles, and has both high dpi and low dpi offline tiles, and the high dpi provider is unavailable. Change-Id: I4d7080d6f259663bf6ca7a985fd9784fb81cfec3 Reviewed-by: Alex Blasche --- .../geoservices/osm/qgeofiletilecacheosm.cpp | 72 +++++++++++++--------- src/plugins/geoservices/osm/qgeofiletilecacheosm.h | 5 +- 2 files changed, 45 insertions(+), 32 deletions(-) (limited to 'src/plugins/geoservices') diff --git a/src/plugins/geoservices/osm/qgeofiletilecacheosm.cpp b/src/plugins/geoservices/osm/qgeofiletilecacheosm.cpp index a36f15b3..f932d3de 100644 --- a/src/plugins/geoservices/osm/qgeofiletilecacheosm.cpp +++ b/src/plugins/geoservices/osm/qgeofiletilecacheosm.cpp @@ -49,13 +49,14 @@ QGeoFileTileCacheOsm::QGeoFileTileCacheOsm(const QVector const QString &offlineDirectory, const QString &directory, QObject *parent) -: QGeoFileTileCache(directory, parent), m_offlineDirectory(offlineDirectory), m_requestCancel(0), m_providers(providers) +: QGeoFileTileCache(directory, parent), m_offlineDirectory(offlineDirectory), m_providers(providers) { m_highDpi.resize(providers.size()); for (int i = 0; i < providers.size(); i++) { providers[i]->setParent(this); m_highDpi[i] = providers[i]->isHighDpi(); m_mapIdFutures[providers[i]->mapType().mapId()].isFinished(); // To construct a default future for this mapId + m_requestCancel[providers[i]->mapType().mapId()] = 0; connect(providers[i], &QGeoTileProviderOsm::resolutionFinished, this, &QGeoFileTileCacheOsm::onProviderResolutionFinished); connect(providers[i], &QGeoTileProviderOsm::resolutionError, this, &QGeoFileTileCacheOsm::onProviderResolutionFinished); } @@ -63,10 +64,10 @@ QGeoFileTileCacheOsm::QGeoFileTileCacheOsm(const QVector QGeoFileTileCacheOsm::~QGeoFileTileCacheOsm() { - m_requestCancel = 1; - m_future.waitForFinished(); - for (const QGeoTileProviderOsm *p : m_providers) + for (const QGeoTileProviderOsm *p : m_providers) { + m_requestCancel[p->mapType().mapId()] = 1; m_mapIdFutures[p->mapType().mapId()].waitForFinished(); + } } QSharedPointer QGeoFileTileCacheOsm::get(const QGeoTileSpec &spec) @@ -88,12 +89,20 @@ void QGeoFileTileCacheOsm::onProviderResolutionFinished(const QGeoTileProviderOs int mapId = m_providers[i]->mapType().mapId(); m_highDpi[i] = m_providers[i]->isHighDpi(); + // Terminate initOfflineRegistry future for mapId. + if (!m_offlineDirectory.isEmpty()) { + m_requestCancel[mapId] = 1; + m_mapIdFutures[mapId].waitForFinished(); + m_requestCancel[mapId] = 0; + } + // reload cache for mapId i dropTiles(mapId); loadTiles(mapId); // reload offline registry for mapId i - m_mapIdFutures[mapId] = QtConcurrent::run(this, &QGeoFileTileCacheOsm::initOfflineRegistry, mapId); + if (!m_offlineDirectory.isEmpty()) + m_mapIdFutures[mapId] = QtConcurrent::run(this, &QGeoFileTileCacheOsm::initOfflineRegistry, mapId); // send signal to clear scene in all maps created through this provider that use the reloaded tiles emit mapDataUpdated(mapId); @@ -101,7 +110,8 @@ void QGeoFileTileCacheOsm::onProviderResolutionFinished(const QGeoTileProviderOs } } -// On resolution error the provider is removed ONLY if there is no enabled hardcoded fallback. +// On resolution error the provider is removed. +// This happens ONLY if there is no enabled hardcoded fallback for the mapId. // Hardcoded fallbacks also have a timestamp, that can get updated with Qt releases. void QGeoFileTileCacheOsm::onProviderResolutionError(const QGeoTileProviderOsm *provider, QNetworkReply::NetworkError error) { @@ -109,6 +119,7 @@ void QGeoFileTileCacheOsm::onProviderResolutionError(const QGeoTileProviderOsm * clearObsoleteTiles(provider); // this still removes tiles who happen to be older than qgeotileproviderosm.cpp defaultTs } +// init() is always called before the provider resolution starts void QGeoFileTileCacheOsm::init() { if (directory_.isEmpty()) @@ -141,11 +152,11 @@ void QGeoFileTileCacheOsm::init() // Base class ::init() QGeoFileTileCache::init(); - for (QGeoTileProviderOsm * p: m_providers) + for (QGeoTileProviderOsm * p: m_providers) { clearObsoleteTiles(p); - - if (!m_offlineDirectory.isEmpty()) - m_future = QtConcurrent::run(this, &QGeoFileTileCacheOsm::initOfflineRegistry, -1); + if (!m_offlineDirectory.isEmpty()) + m_mapIdFutures[p->mapType().mapId()] = QtConcurrent::run(this, &QGeoFileTileCacheOsm::initOfflineRegistry, p->mapType().mapId()); + } } QSharedPointer QGeoFileTileCacheOsm::getFromOfflineStorage(const QGeoTileSpec &spec) @@ -190,6 +201,11 @@ void QGeoFileTileCacheOsm::dropTiles(int mapId) for (const QGeoTileSpec &k : keys) if (k.mapId() == mapId) diskCache_.remove(k); + + keys = m_tilespecToOfflineFilepath.keys(); + for (const QGeoTileSpec &k : keys) + if (k.mapId() == mapId) + m_tilespecToOfflineFilepath.remove(k); } void QGeoFileTileCacheOsm::loadTiles(int mapId) @@ -211,6 +227,9 @@ void QGeoFileTileCacheOsm::loadTiles(int mapId) void QGeoFileTileCacheOsm::initOfflineRegistry(int mapId) { + if (mapId < 1) // map ids in osm start from 1 + return; + // Dealing with duplicates: picking the newest QMap > fileDates; // key is filename, value is QDirIterator it(m_offlineDirectory, QStringList() << "*.*", QDir::Files, QDirIterator::Subdirectories | QDirIterator::FollowSymlinks ); @@ -219,42 +238,37 @@ void QGeoFileTileCacheOsm::initOfflineRegistry(int mapId) QFileInfo f(path); if (!fileDates.contains(f.fileName()) || fileDates[f.fileName()].second < f.lastModified()) fileDates[f.fileName()] = QPair(f.filePath(), f.lastModified()); - if (m_requestCancel) + if (m_requestCancel[mapId]) return; } // Clear the content of the index. Entirely (at startup), or selectively (when a provider resolution changes the highDpi status). - if (mapId < 0) { - storageLock.lock(); - m_tilespecToOfflineFilepath.clear(); - storageLock.unlock(); - } else { - QList toRemove; - for (auto i = m_tilespecToOfflineFilepath.constBegin(); i != m_tilespecToOfflineFilepath.constEnd(); ++i) { - if (i.key().mapId() == mapId) - toRemove.append(i.key()); - } - storageLock.lock(); - for (const auto &i : toRemove) - m_tilespecToOfflineFilepath.remove(i); - storageLock.unlock(); + QList toRemove; + for (auto i = m_tilespecToOfflineFilepath.constBegin(); i != m_tilespecToOfflineFilepath.constEnd(); ++i) { + if (i.key().mapId() == mapId) + toRemove.append(i.key()); } - if (m_requestCancel) + storageLock.lock(); + for (const auto &i : toRemove) + m_tilespecToOfflineFilepath.remove(i); + storageLock.unlock(); + + if (m_requestCancel[mapId]) return; - // Fill the index entirely or selectively + // Fill the index by mapId int count = 0; for (auto i= fileDates.constBegin(); i != fileDates.constEnd(); ++i) { QGeoTileSpec spec = filenameToTileSpec(i.key()); if (spec.zoom() == -1) continue; - if (mapId >= 0 && spec.mapId() != mapId) // if mapId != -1, pick up only those files with that mapId. + if (spec.mapId() != mapId) continue; count++; storageLock.lock(); m_tilespecToOfflineFilepath[spec] = i.value().first; storageLock.unlock(); - if (m_requestCancel) + if (m_requestCancel[mapId]) return; } //qInfo() << "OSM plugin has found and is using "<< count <<" offline tiles"; diff --git a/src/plugins/geoservices/osm/qgeofiletilecacheosm.h b/src/plugins/geoservices/osm/qgeofiletilecacheosm.h index d26cad4a..f26e7f10 100644 --- a/src/plugins/geoservices/osm/qgeofiletilecacheosm.h +++ b/src/plugins/geoservices/osm/qgeofiletilecacheosm.h @@ -72,13 +72,12 @@ protected: void dropTiles(int mapId); void loadTiles(int mapId); - void initOfflineRegistry(int mapId = -1); + void initOfflineRegistry(int mapId); void clearObsoleteTiles(const QGeoTileProviderOsm *p); QString m_offlineDirectory; QHash m_tilespecToOfflineFilepath; - QAtomicInt m_requestCancel; - QFuture m_future; + QMap m_requestCancel; QMap> m_mapIdFutures; QMutex storageLock; QVector m_providers; -- cgit v1.2.1