QtLocationPlugin: Qt Integration Improvements & Fixes

Author: HTRamsey

src/QmlControls/OfflineMapEditor.qml

@@ -334,7 +334,7 @@ FlightMap {
                         text:       qsTr("Delete")
                         width:      ScreenTools.defaultFontPixelWidth * (infoView._extraButton ? 6 : 10)
                         onClicked:  deleteConfirmationDialogComponent.createObject(mainWindow).open()
-                        enabled:    tileSet ? (tileSet.savedTileSize > 0) : false
+                        enabled:    tileSet ? (!tileSet.deleting && (_defaultSet ? tileSet.savedTileSize > 0 : true)) : false
                     }
                     QGCButton {
                         text:       qsTr("Ok")

src/QtLocationPlugin/CMakeLists.txt

@@ -51,14 +51,6 @@ qt_add_plugin(QGCLocation
     QGeoTileFetcherQGC.h
 )
 
-# ----------------------------------------------------------------------------
-# Platform-Specific Configuration
-# ----------------------------------------------------------------------------
-# Google Maps not available on iOS
-if(IOS)
-    target_compile_definitions(QGCLocation PRIVATE QGC_NO_GOOGLE_MAPS)
-endif()
-
 # ----------------------------------------------------------------------------
 # Dependencies
 # ----------------------------------------------------------------------------

src/QtLocationPlugin/QGCCachedTileSet.cpp

@@ -126,10 +126,10 @@ void QGCCachedTileSet::_doneWithDownload()
         setTotalTileSize(_savedTileSize);
 
         quint32 avg = 0;
-        if (_savedTileSize != 0) {
+        if (_savedTileCount != 0) {
             avg = _savedTileSize / _savedTileCount;
         } else {
-            qCWarning(QGCCachedTileSetLog) << "_savedTileSize=0";
+            qCWarning(QGCCachedTileSetLog) << "_savedTileCount=0";
         }
 
         setUniqueTileSize(_uniqueTileCount * avg);
@@ -151,11 +151,22 @@ void QGCCachedTileSet::_prepareDownload()
         return;
     }
 
-    for (qsizetype i = _replies.count(); i < QGeoTileFetcherQGC::concurrentDownloads(_type); i++) {
+    const qsizetype maxConcurrent = QGeoTileFetcherQGC::concurrentDownloads(_type);
+
+    for (qsizetype i = 0; i < maxConcurrent; i++) {
         if (_tilesToDownload.isEmpty()) {
             break;
         }
 
+        // Check if we've reached the concurrent limit
+        {
+            QMutexLocker lock(&_repliesMutex);
+            if (_replies.count() >= maxConcurrent) {
+                break;
+            }
+        }
+
+        // Dequeue and prepare tile request without holding the mutex
         QGCTile* const tile = _tilesToDownload.dequeue();
         const int mapId = UrlFactory::getQtMapIdFromProviderType(tile->type);
         QNetworkRequest request = QGeoTileFetcherQGC::getNetworkRequest(mapId, tile->x, tile->y, tile->z);
@@ -167,10 +178,15 @@ void QGCCachedTileSet::_prepareDownload()
         QGCFileDownload::setIgnoreSSLErrorsIfNeeded(*reply);
         (void) connect(reply, &QNetworkReply::finished, this, &QGCCachedTileSet::_networkReplyFinished);
         (void) connect(reply, &QNetworkReply::errorOccurred, this, &QGCCachedTileSet::_networkReplyError);
-        (void) _replies.insert(tile->hash, reply);
+
+        // Insert into replies map
+        {
+            QMutexLocker lock(&_repliesMutex);
+            (void) _replies.insert(tile->hash, reply);
+        }
 
         delete tile;
-        if (!_batchRequested && !_noMoreTiles && (_tilesToDownload.count() < (QGeoTileFetcherQGC::concurrentDownloads(_type) * 10))) {
+        if (!_batchRequested && !_noMoreTiles && (_tilesToDownload.count() < (maxConcurrent * 10))) {
             createDownloadTask();
         }
     }
@@ -200,10 +216,13 @@ void QGCCachedTileSet::_networkReplyFinished()
         return;
     }
 
-    if (_replies.contains(hash)) {
-        (void) _replies.remove(hash);
-    } else {
-        qCWarning(QGCCachedTileSetLog) << "Reply not in list: " << hash;
+    {
+        QMutexLocker lock(&_repliesMutex);
+        if (_replies.contains(hash)) {
+            (void) _replies.remove(hash);
+        } else {
+            qCWarning(QGCCachedTileSetLog) << "Reply not in list: " << hash;
+        }
     }
     qCDebug(QGCCachedTileSetLog) << "Tile fetched:" << hash;
 
@@ -215,7 +234,10 @@ void QGCCachedTileSet::_networkReplyFinished()
 
     const QString type = UrlFactory::tileHashToType(hash);
     const SharedMapProvider mapProvider = UrlFactory::getMapProviderFromProviderType(type);
-    Q_CHECK_PTR(mapProvider);
+    if (!mapProvider) {
+        qCWarning(QGCCachedTileSetLog) << "Invalid map provider for type:" << type;
+        return;
+    }
 
     if (mapProvider->isElevationProvider()) {
         const SharedElevationProvider elevationProvider = std::dynamic_pointer_cast<const ElevationProvider>(mapProvider);
@@ -242,7 +264,7 @@ void QGCCachedTileSet::_networkReplyFinished()
     setSavedTileSize(_savedTileSize + image.size());
     setSavedTileCount(_savedTileCount + 1);
 
-    if (_savedTileCount % 10 == 0) {
+    if (_savedTileCount % kSizeEstimateInterval == 0) {
         const quint32 avg = _savedTileSize / _savedTileCount;
         setTotalTileSize(avg * _totalTileCount);
         setUniqueTileSize(avg * _uniqueTileCount);
@@ -267,10 +289,13 @@ void QGCCachedTileSet::_networkReplyError(QNetworkReply::NetworkError error)
         return;
     }
 
-    if (_replies.contains(hash)) {
-        (void) _replies.remove(hash);
-    } else {
-        qCWarning(QGCCachedTileSetLog) << "Reply not in list:" << hash;
+    {
+        QMutexLocker lock(&_repliesMutex);
+        if (_replies.contains(hash)) {
+            (void) _replies.remove(hash);
+        } else {
+            qCWarning(QGCCachedTileSetLog) << "Reply not in list:" << hash;
+        }
     }
 
     if (error != QNetworkReply::OperationCanceledError) {

src/QtLocationPlugin/QGCCachedTileSet.h

@@ -12,6 +12,7 @@
 #include <QtCore/QDateTime>
 #include <QtCore/QHash>
 #include <QtCore/QLoggingCategory>
+#include <QtCore/QMutex>
 #include <QtCore/QObject>
 #include <QtCore/QQueue>
 #include <QtCore/QString>
@@ -182,6 +183,8 @@ private slots:
     QQueue<QGCTile*> _tilesToDownload;
     QGCMapEngineManager *_manager = nullptr;
     QNetworkAccessManager *_networkManager = nullptr;
+    QMutex _repliesMutex;
 
     static constexpr uint32_t kTileBatchSize = 256;
+    static constexpr uint32_t kSizeEstimateInterval = 10;
 };

src/QtLocationPlugin/QGCMapEngineManager.cc

@@ -401,7 +401,7 @@ bool QGCMapEngineManager::exportSets(const QString &path)
 
     for (qsizetype i = 0; i < _tileSets->count(); i++) {
         QGCCachedTileSet* const set = qobject_cast<QGCCachedTileSet*>(_tileSets->get(i));
-        if (set->selected()) {
+        if (set && set->selected()) {
             sets.append(set);
         }
     }
@@ -443,8 +443,6 @@ QString QGCMapEngineManager::getUniqueName() const
             return name;
         }
     }
-
-    return QString("");
 }
 
 QStringList QGCMapEngineManager::mapList()

src/QtLocationPlugin/QGCMapUrlEngine.cpp

@@ -23,13 +23,12 @@
 QGC_LOGGING_CATEGORY(QGCMapUrlEngineLog, "QtLocationPlugin.QGCMapUrlEngine")
 
 const QList<SharedMapProvider> UrlFactory::_providers = {
-#ifndef QGC_NO_GOOGLE_MAPS
     std::make_shared<GoogleStreetMapProvider>(),
     std::make_shared<GoogleSatelliteMapProvider>(),
     std::make_shared<GoogleTerrainMapProvider>(),
     std::make_shared<GoogleHybridMapProvider>(),
     std::make_shared<GoogleLabelsMapProvider>(),
-#endif
+
     std::make_shared<BingRoadMapProvider>(),
     std::make_shared<BingSatelliteMapProvider>(),
     std::make_shared<BingHybridMapProvider>(),
@@ -173,9 +172,8 @@ QGCTileSet UrlFactory::getTileCount(int zoom, double topleftLon, double topleftL
 
 QString UrlFactory::getProviderTypeFromQtMapId(int qtMapId)
 {
-    // Default Set
-    if (qtMapId == -1) {
-        return nullptr;
+    if (qtMapId == defaultSetMapId()) {
+        return QString();
     }
 
     for (const SharedMapProvider &provider : _providers) {
@@ -190,8 +188,7 @@ QString UrlFactory::getProviderTypeFromQtMapId(int qtMapId)
 
 SharedMapProvider UrlFactory::getMapProviderFromQtMapId(int qtMapId)
 {
-    // Default Set
-    if (qtMapId == -1) {
+    if (qtMapId == defaultSetMapId()) {
         return nullptr;
     }
 

src/QtLocationPlugin/QGCMapUrlEngine.h

@@ -51,6 +51,9 @@ class UrlFactory
     static QString tileHashToType(QStringView tileHash);
     static QString getTileHash(QStringView type, int x, int y, int z);
 
+    static constexpr int defaultSetMapId() { return -1; }
+    static constexpr quint64 defaultTileSetId() { return 1; }
+
 private:
     static const QList<std::shared_ptr<const MapProvider>> _providers;
 };