Consolidate smart pointer usage

src/core/map.cpp

@@ -128,7 +128,7 @@ mbgl::MapOptions mapOptionsFromSettings(const QMapLibre::Settings &settings, con
 }
 
 mbgl::ResourceOptions resourceOptionsFromSettings(const QMapLibre::Settings &settings) {
-    if (settings.tileServerOptions() == nullptr) {
+    if (!settings.customTileServerOptions()) {
         return std::move(mbgl::ResourceOptions()
                              .withAssetPath(settings.assetPath().toStdString())
                              .withCachePath(settings.cacheDatabasePath().toStdString())
@@ -138,7 +138,7 @@ mbgl::ResourceOptions resourceOptionsFromSettings(const QMapLibre::Settings &set
     return std::move(mbgl::ResourceOptions()
                          .withApiKey(settings.apiKey().toStdString())
                          .withAssetPath(settings.assetPath().toStdString())
-                         .withTileServerOptions(*settings.tileServerOptions())
+                         .withTileServerOptions(settings.tileServerOptions())
                          .withCachePath(settings.cacheDatabasePath().toStdString())
                          .withMaximumCacheSize(settings.cacheDatabaseMaximumSize()));
 }
@@ -312,15 +312,13 @@ Map::Map(QObject *parent_, const Settings &settings, const QSize &size, qreal pi
         loop.setLocalData(std::make_shared<mbgl::util::RunLoop>());
     }
 
-    d_ptr = new MapPrivate(this, settings, size, pixelRatio);
+    d_ptr = std::make_unique<MapPrivate>(this, settings, size, pixelRatio);
 }
 
 /*!
     Destroys this QMapLibre::Map.
 */
-Map::~Map() {
-    delete d_ptr;
-}
+Map::~Map() = default;
 
 /*!
     \property QMapLibre::Map::styleJson
@@ -921,16 +919,16 @@ void Map::updateSource(const QString &id, const QVariantMap &params) {
         return;
     }
 
-    auto sourceGeoJSON = source->as<GeoJSONSource>();
-    auto sourceImage = source->as<ImageSource>();
-    if (!sourceGeoJSON && !sourceImage) {
+    auto *sourceGeoJSON = source->as<GeoJSONSource>();
+    auto *sourceImage = source->as<ImageSource>();
+    if (sourceGeoJSON == nullptr && sourceImage == nullptr) {
         qWarning() << "Unable to update source: only GeoJSON and Image sources are mutable.";
         return;
     }
 
-    if (sourceImage && params.contains("url")) {
+    if (sourceImage != nullptr && params.contains("url")) {
         sourceImage->setURL(params["url"].toString().toStdString());
-    } else if (sourceGeoJSON && params.contains("data")) {
+    } else if (sourceGeoJSON != nullptr && params.contains("data")) {
         Error error;
         auto result = convert<mbgl::GeoJSON>(params["data"], error);
         if (result) {
@@ -1106,7 +1104,7 @@ void Map::setFilter(const QString &layer, const QVariant &filter) {
     using namespace mbgl::style::conversion;
 
     Layer *layer_ = d_ptr->mapObj->getStyle().getLayer(layer.toStdString());
-    if (!layer_) {
+    if (layer_ == nullptr) {
         qWarning() << "Layer not found:" << layer;
         return;
     }
@@ -1158,7 +1156,7 @@ QVariant Map::getFilter(const QString &layer) const {
     using namespace mbgl::style::conversion;
 
     Layer *layer_ = d_ptr->mapObj->getStyle().getLayer(layer.toStdString());
-    if (!layer_) {
+    if (layer_ == nullptr) {
         qWarning() << "Layer not found:" << layer;
         return QVariant();
     }
@@ -1341,7 +1339,7 @@ void MapPrivate::update(std::shared_ptr<mbgl::UpdateParameters> parameters) {
 
     m_updateParameters = std::move(parameters);
 
-    if (!m_mapRenderer) {
+    if (m_mapRenderer == nullptr) {
         return;
     }
 
@@ -1399,7 +1397,7 @@ void MapPrivate::render() {
 void MapPrivate::setFramebufferObject(quint32 fbo, const QSize &size) {
     std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
 
-    if (!m_mapRenderer) {
+    if (m_mapRenderer == nullptr) {
         createRenderer();
     }
 
@@ -1419,7 +1417,7 @@ bool MapPrivate::setProperty(const PropertySetter &setter,
     using namespace mbgl::style;
 
     Layer *layerObject = mapObj->getStyle().getLayer(layer.toStdString());
-    if (!layerObject) {
+    if (layerObject == nullptr) {
         qWarning() << "Layer not found:" << layer;
         return false;
     }

src/core/map.hpp

@@ -188,7 +188,7 @@ public slots:
 private:
     Q_DISABLE_COPY(Map)
 
-    MapPrivate *d_ptr;
+    std::unique_ptr<MapPrivate> d_ptr;
 };
 
 } // namespace QMapLibre

src/core/map_observer.cpp

@@ -14,9 +14,9 @@
 namespace QMapLibre {
 
 MapObserver::MapObserver(MapPrivate *d)
-    : d_ptr(d) {}
+    : d_ptrRef(d) {}
 
-MapObserver::~MapObserver() {}
+MapObserver::~MapObserver() = default;
 
 void MapObserver::onCameraWillChange(mbgl::MapObserver::CameraChangeMode mode) {
     if (mode == mbgl::MapObserver::CameraChangeMode::Immediate) {
@@ -99,7 +99,7 @@ void MapObserver::onDidFinishLoadingStyle() {
 
 void MapObserver::onSourceChanged(mbgl::style::Source &) {
     std::string attribution;
-    for (const auto &source : d_ptr->mapObj->getStyle().getSources()) {
+    for (const auto &source : d_ptrRef->mapObj->getStyle().getSources()) {
         // Avoid duplicates by using the most complete attribution HTML snippet.
         if (source->getAttribution() && (attribution.size() < source->getAttribution()->size()))
             attribution = *source->getAttribution();

src/core/map_observer_p.hpp

@@ -48,7 +48,7 @@ class MapObserver : public QObject, public mbgl::MapObserver {
 private:
     Q_DISABLE_COPY(MapObserver)
 
-    MapPrivate *d_ptr;
+    MapPrivate *d_ptrRef;
 };
 
 } // namespace QMapLibre

src/core/settings.cpp

@@ -9,7 +9,6 @@
 #include <mbgl/gfx/renderer_backend.hpp>
 #include <mbgl/map/mode.hpp>
 #include <mbgl/util/constants.hpp>
-#include <mbgl/util/tile_server_options.hpp>
 #include <mbgl/util/traits.hpp>
 
 #include <QtCore/QCoreApplication>
@@ -149,37 +148,23 @@ namespace QMapLibre {
     but a provider template can be provided.
 */
 Settings::Settings(ProviderTemplate provider)
-    : d_ptr(new SettingsPrivate) {
+    : d_ptr(std::make_unique<SettingsPrivate>()) {
     d_ptr->setProviderTemplate(provider);
 }
 
-Settings::~Settings() {
-    delete d_ptr;
-}
+Settings::~Settings() = default;
 
 Settings::Settings(const Settings &s)
-    : d_ptr(new SettingsPrivate) {
-    *d_ptr = *s.d_ptr;
-}
+    : d_ptr(std::make_unique<SettingsPrivate>(*s.d_ptr)) {}
 
-Settings::Settings(Settings &&s) noexcept
-    : d_ptr(s.d_ptr) {
-    s.d_ptr = nullptr;
-}
+Settings::Settings(Settings &&s) noexcept = default;
 
 Settings &Settings::operator=(const Settings &s) {
-    if (d_ptr != nullptr) delete d_ptr;
-    d_ptr = new SettingsPrivate;
-    *d_ptr = *s.d_ptr;
+    d_ptr = std::make_unique<SettingsPrivate>(*s.d_ptr);
     return *this;
 }
 
-Settings &Settings::operator=(Settings &&s) noexcept {
-    if (d_ptr != nullptr) delete d_ptr;
-    d_ptr = s.d_ptr;
-    s.d_ptr = nullptr;
-    return *this;
-}
+Settings &Settings::operator=(Settings &&s) noexcept = default;
 
 /*!
     Returns the OpenGL context mode. This is specially important when mixing
@@ -340,11 +325,11 @@ void Settings::setApiKey(const QString &key) {
     Returns the API base URL.
 */
 QString Settings::apiBaseUrl() const {
-    if (tileServerOptions() == nullptr) {
+    if (!customTileServerOptions()) {
         return {};
     }
 
-    return QString::fromStdString(tileServerOptions()->baseURL());
+    return QString::fromStdString(tileServerOptions().baseURL());
 }
 
 /*!
@@ -463,11 +448,11 @@ void Settings::setStyles(const Styles &styles) {
 */
 Styles Settings::providerStyles() const {
     Styles styles;
-    if (tileServerOptions() == nullptr) {
+    if (!customTileServerOptions()) {
         return styles;
     }
 
-    for (const auto &style : tileServerOptions()->defaultStyles()) {
+    for (const auto &style : tileServerOptions().defaultStyles()) {
         styles.append(Style(QString::fromStdString(style.getUrl()), QString::fromStdString(style.getName())));
     }
     return styles;
@@ -501,12 +486,19 @@ void Settings::setDefaultZoom(double zoom) {
     d_ptr->m_defaultZoom = zoom;
 }
 
+/*!
+    Returns whether the tile server options have been set by the user.
+*/
+bool Settings::customTileServerOptions() const {
+    return d_ptr->m_customTileServerOptions;
+}
+
 /*!
     Returns the provider tile server options.
 
     Note that this is mainly for internal use.
 */
-mbgl::TileServerOptions *Settings::tileServerOptions() const {
+const mbgl::TileServerOptions &Settings::tileServerOptions() const {
     return d_ptr->m_tileServerOptions;
 }
 
@@ -523,29 +515,30 @@ SettingsPrivate::SettingsPrivate()
       m_apiKey(qgetenv("MLN_API_KEY")) {}
 
 void SettingsPrivate::setProviderTemplate(Settings::ProviderTemplate providerTemplate) {
-    if (m_tileServerOptions) {
-        delete m_tileServerOptions;
-        m_tileServerOptions = nullptr;
-    }
-
     m_providerTemplate = providerTemplate;
 
     if (providerTemplate == Settings::MapLibreProvider) {
-        m_tileServerOptions = new mbgl::TileServerOptions(mbgl::TileServerOptions::MapLibreConfiguration());
+        m_tileServerOptions = mbgl::TileServerOptions::MapLibreConfiguration();
+        m_customTileServerOptions = true;
     } else if (providerTemplate == Settings::MapTilerProvider) {
-        m_tileServerOptions = new mbgl::TileServerOptions(mbgl::TileServerOptions::MapTilerConfiguration());
+        m_tileServerOptions = mbgl::TileServerOptions::MapTilerConfiguration();
+        m_customTileServerOptions = true;
     } else if (providerTemplate == Settings::MapboxProvider) {
-        m_tileServerOptions = new mbgl::TileServerOptions(mbgl::TileServerOptions::MapboxConfiguration());
+        m_tileServerOptions = mbgl::TileServerOptions::MapboxConfiguration();
+        m_customTileServerOptions = true;
+    } else {
+        m_tileServerOptions = mbgl::TileServerOptions();
+        m_customTileServerOptions = false;
     }
 }
 
 void SettingsPrivate::setProviderApiBaseUrl(const QString &url) {
-    if (m_tileServerOptions == nullptr) {
+    if (!m_customTileServerOptions) {
         qWarning() << "No provider set so not setting API URL.";
         return;
     }
 
-    m_tileServerOptions = &m_tileServerOptions->withBaseURL(url.toStdString());
+    m_tileServerOptions = std::move(m_tileServerOptions.withBaseURL(url.toStdString()));
 }
 
 } // namespace QMapLibre

src/core/settings.hpp

@@ -13,6 +13,7 @@
 #include <QtGui/QImage>
 
 #include <functional>
+#include <memory>
 
 // TODO: this will be wrapped at some point
 namespace mbgl {
@@ -110,10 +111,11 @@ class Q_MAPLIBRE_CORE_EXPORT Settings {
     double defaultZoom() const;
     void setDefaultZoom(double);
 
-    mbgl::TileServerOptions *tileServerOptions() const;
+    bool customTileServerOptions() const;
+    const mbgl::TileServerOptions &tileServerOptions() const;
 
 private:
-    SettingsPrivate *d_ptr;
+    std::unique_ptr<SettingsPrivate> d_ptr;
 };
 
 } // namespace QMapLibre

src/core/settings_p.hpp

@@ -8,10 +8,13 @@
 #include "settings.hpp"
 #include "types.hpp"
 
+#include <mbgl/util/tile_server_options.hpp>
+
 #include <QtCore/QString>
 #include <QtCore/QVector>
 
 #include <functional>
+#include <memory>
 
 namespace mbgl {
 class TileServerOptions;
@@ -47,7 +50,8 @@ class SettingsPrivate {
 
     std::function<std::string(const std::string &)> m_resourceTransform;
 
-    mbgl::TileServerOptions *m_tileServerOptions{};
+    bool m_customTileServerOptions{};
+    mbgl::TileServerOptions m_tileServerOptions{};
 };
 
 } // namespace QMapLibre

src/location/qgeomap.cpp

@@ -49,7 +49,7 @@ namespace QMapLibre {
 QGeoMapMapLibrePrivate::QGeoMapMapLibrePrivate(QGeoMappingManagerEngine *engine)
     : QGeoMapPrivate(engine, new QGeoProjectionWebMercator) {}
 
-QGeoMapMapLibrePrivate::~QGeoMapMapLibrePrivate() {}
+QGeoMapMapLibrePrivate::~QGeoMapMapLibrePrivate() = default;
 
 QSGNode *QGeoMapMapLibrePrivate::updateSceneGraph(QSGNode *node, QQuickWindow *window) {
     Q_Q(QGeoMapMapLibre);
@@ -60,9 +60,9 @@ QSGNode *QGeoMapMapLibrePrivate::updateSceneGraph(QSGNode *node, QQuickWindow *w
     }
 
     Map *map{};
-    if (!node) {
+    if (node == nullptr) {
         QOpenGLContext *currentCtx = QOpenGLContext::currentContext();
-        if (!currentCtx) {
+        if (currentCtx == nullptr) {
             qWarning("QOpenGLContext is NULL!");
             qWarning() << "You are running on QSG backend " << QSGContext::backend();
             qWarning("The MapLibre plugin works with both Desktop and ES 2.0+ OpenGL versions.");
@@ -74,10 +74,10 @@ QSGNode *QGeoMapMapLibrePrivate::updateSceneGraph(QSGNode *node, QQuickWindow *w
             return node;
         }
 
-        auto *mbglNode = new TextureNode(m_settings, m_viewportSize, window->devicePixelRatio(), q);
+        auto mbglNode = std::make_unique<TextureNode>(m_settings, m_viewportSize, window->devicePixelRatio(), q);
         QObject::connect(mbglNode->map(), &Map::mapChanged, q, &QGeoMapMapLibre::onMapChanged);
         m_syncState = MapTypeSync | CameraDataSync | ViewportSync | VisibleAreaSync;
-        node = mbglNode;
+        node = mbglNode.release();
     }
     map = static_cast<TextureNode *>(node)->map();
 
@@ -354,7 +354,7 @@ QGeoMapMapLibre::QGeoMapMapLibre(QGeoMappingManagerEngine *engine, QObject *pare
     d->m_refresh.setInterval(250);
 }
 
-QGeoMapMapLibre::~QGeoMapMapLibre() {}
+QGeoMapMapLibre::~QGeoMapMapLibre() = default;
 
 void QGeoMapMapLibre::setSettings(const Settings &settings) {
     Q_D(QGeoMapMapLibre);