[qt] Make sure that methods are being called on the right thread

4 files changed, 109 insertions, 61 deletions

diff --git a/platform/qt/src/qmapboxgl.cpp b/platform/qt/src/qmapboxgl.cpp
index df5673e35b..c37e1bc9ba 100644
--- a/platform/qt/src/qmapboxgl.cpp
+++ b/platform/qt/src/qmapboxgl.cpp
@@ -1453,19 +1453,12 @@ void QMapboxGL::setFilter(const QString& layer, const QVariant& filter)
 
 void QMapboxGL::createRenderer()
 {
-    d_ptr->mapRenderer = std::make_unique<QMapboxGLMapRenderer>(
-        d_ptr->pixelRatio,
-        *d_ptr->fileSourceObj,
-        *d_ptr->threadPool,
-        d_ptr->mode
-    );
-
-    d_ptr->mapRenderer->setObserver(d_ptr->rendererObserver);
+    d_ptr->createRenderer();
 }
 
 void QMapboxGL::destroyRenderer()
 {
-    d_ptr->mapRenderer.reset();
+    d_ptr->destroyRenderer();
 }
 
 /*!
@@ -1479,20 +1472,7 @@ void QMapboxGL::destroyRenderer()
 */
 void QMapboxGL::render()
 {
-    if (!d_ptr->mapRenderer) {
-        createRenderer();
-    }
-
-#if defined(__APPLE__) && QT_VERSION < 0x050000
-    // FIXME Qt 4.x provides an incomplete FBO at start.
-    // See https://bugreports.qt.io/browse/QTBUG-36802 for details.
-    if (glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) {
-        return;
-    }
-#endif
-
-    d_ptr->renderQueued.clear();
-    d_ptr->mapRenderer->render();
+    d_ptr->render();
 }
 
 /*!
@@ -1502,11 +1482,7 @@ void QMapboxGL::render()
 */
 void QMapboxGL::setFramebufferObject(quint32 fbo, const QSize& size)
 {
-    if (!d_ptr->mapRenderer) {
-        createRenderer();
-    }
-
-    d_ptr->mapRenderer->updateFramebuffer(fbo, sanitizedSize(size));
+    d_ptr->setFramebufferObject(fbo, size);
 }
 
 /*!
@@ -1547,47 +1523,46 @@ void QMapboxGL::connectionEstablished()
 
 QMapboxGLPrivate::QMapboxGLPrivate(QMapboxGL *q, const QMapboxGLSettings &settings, const QSize &size, qreal pixelRatio_)
     : QObject(q)
-    , q_ptr(q)
-    , fileSourceObj(sharedDefaultFileSource(
+    , m_fileSourceObj(sharedDefaultFileSource(
         settings.cacheDatabasePath().toStdString(),
         settings.assetPath().toStdString(),
         settings.cacheDatabaseMaximumSize()))
-    , threadPool(mbgl::sharedThreadPool())
-    , mode(settings.contextMode())
-    , pixelRatio(pixelRatio_)
+    , m_threadPool(mbgl::sharedThreadPool())
+    , m_mode(settings.contextMode())
+    , m_pixelRatio(pixelRatio_)
 {
     // Setup the FileSource
-    fileSourceObj->setAccessToken(settings.accessToken().toStdString());
-    fileSourceObj->setAPIBaseURL(settings.apiBaseUrl().toStdString());
+    m_fileSourceObj->setAccessToken(settings.accessToken().toStdString());
+    m_fileSourceObj->setAPIBaseURL(settings.apiBaseUrl().toStdString());
 
     if (settings.resourceTransform()) {
         m_resourceTransform = std::make_unique<mbgl::Actor<mbgl::ResourceTransform>>(*mbgl::Scheduler::GetCurrent(),
             [callback = settings.resourceTransform()] (mbgl::Resource::Kind, const std::string &&url_) -> std::string {
                 return callback(std::move(url_));
             });
-       fileSourceObj->setResourceTransform(m_resourceTransform->self());
+       m_fileSourceObj->setResourceTransform(m_resourceTransform->self());
     }
 
     // Setup MapObserver
-    mapObserver = std::make_unique<QMapboxGLMapObserver>(this);
+    m_mapObserver = std::make_unique<QMapboxGLMapObserver>(this);
 
     qRegisterMetaType<QMapboxGL::MapChange>("QMapboxGL::MapChange");
 
-    connect(mapObserver.get(), SIGNAL(mapChanged(QMapboxGL::MapChange)), q_ptr, SIGNAL(mapChanged(QMapboxGL::MapChange)));
-    connect(mapObserver.get(), SIGNAL(copyrightsChanged(QString)), q_ptr, SIGNAL(copyrightsChanged(QString)));
+    connect(m_mapObserver.get(), SIGNAL(mapChanged(QMapboxGL::MapChange)), q, SIGNAL(mapChanged(QMapboxGL::MapChange)));
+    connect(m_mapObserver.get(), SIGNAL(copyrightsChanged(QString)), q, SIGNAL(copyrightsChanged(QString)));
 
     // Setup the Map object
     mapObj = std::make_unique<mbgl::Map>(
             *this, // RendererFrontend
-            *mapObserver,
+            *m_mapObserver,
             sanitizedSize(size),
-            pixelRatio, *fileSourceObj, *threadPool,
+            m_pixelRatio, *m_fileSourceObj, *m_threadPool,
             mbgl::MapMode::Continuous,
             static_cast<mbgl::ConstrainMode>(settings.constrainMode()),
             static_cast<mbgl::ViewportMode>(settings.viewportMode()));
 
     // Needs to be Queued to give time to discard redundant draw calls via the `renderQueued` flag.
-    connect(this, SIGNAL(needsRendering()), q_ptr, SIGNAL(needsRendering()), Qt::QueuedConnection);
+    connect(this, SIGNAL(needsRendering()), q, SIGNAL(needsRendering()), Qt::QueuedConnection);
 }
 
 QMapboxGLPrivate::~QMapboxGLPrivate()
@@ -1596,23 +1571,83 @@ QMapboxGLPrivate::~QMapboxGLPrivate()
 
 void QMapboxGLPrivate::update(std::shared_ptr<mbgl::UpdateParameters> parameters)
 {
-    if (!mapRenderer) {
+    std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
+
+    if (!m_mapRenderer) {
         return;
     }
 
-    mapRenderer->updateParameters(std::move(parameters));
+    m_mapRenderer->updateParameters(std::move(parameters));
 
-    if (!renderQueued.test_and_set()) {
+    if (!m_renderQueued.test_and_set()) {
         emit needsRendering();
     }
 }
 
 void QMapboxGLPrivate::setObserver(mbgl::RendererObserver &observer)
 {
-    rendererObserver = std::make_shared<QMapboxGLRendererObserver>(
+    m_rendererObserver = std::make_shared<QMapboxGLRendererObserver>(
             *mbgl::util::RunLoop::Get(), observer);
 
-    if (mapRenderer) {
-        mapRenderer->setObserver(rendererObserver);
+    std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
+
+    if (m_mapRenderer) {
+        m_mapRenderer->setObserver(m_rendererObserver);
     }
 }
+
+void QMapboxGLPrivate::createRenderer()
+{
+    std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
+
+    if (m_mapRenderer) {
+        return;
+    }
+
+    m_mapRenderer = std::make_unique<QMapboxGLMapRenderer>(
+        m_pixelRatio,
+        *m_fileSourceObj,
+        *m_threadPool,
+        m_mode
+    );
+
+    m_mapRenderer->setObserver(m_rendererObserver);
+}
+
+void QMapboxGLPrivate::destroyRenderer()
+{
+    std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
+
+    m_mapRenderer.reset();
+}
+
+void QMapboxGLPrivate::render()
+{
+    std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
+
+    if (!m_mapRenderer) {
+        createRenderer();
+    }
+
+#if defined(__APPLE__) && QT_VERSION < 0x050000
+    // FIXME Qt 4.x provides an incomplete FBO at start.
+    // See https://bugreports.qt.io/browse/QTBUG-36802 for details.
+    if (glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE) {
+        return;
+    }
+#endif
+
+    m_renderQueued.clear();
+    m_mapRenderer->render();
+}
+
+void QMapboxGLPrivate::setFramebufferObject(quint32 fbo, const QSize& size)
+{
+    std::lock_guard<std::recursive_mutex> lock(m_mapRendererMutex);
+
+    if (!m_mapRenderer) {
+        createRenderer();
+    }
+
+    m_mapRenderer->updateFramebuffer(fbo, sanitizedSize(size));
+}
diff --git a/platform/qt/src/qmapboxgl_map_renderer.cpp b/platform/qt/src/qmapboxgl_map_renderer.cpp
index f9120379cb..af6823acb8 100644
--- a/platform/qt/src/qmapboxgl_map_renderer.cpp
+++ b/platform/qt/src/qmapboxgl_map_renderer.cpp
@@ -11,6 +11,7 @@ QMapboxGLMapRenderer::QMapboxGLMapRenderer(qreal pixelRatio,
 
 QMapboxGLMapRenderer::~QMapboxGLMapRenderer()
 {
+    MBGL_VERIFY_THREAD(tid);
 }
 
 void QMapboxGLMapRenderer::schedule(std::weak_ptr<mbgl::Mailbox> mailbox)
@@ -27,11 +28,15 @@ void QMapboxGLMapRenderer::updateParameters(std::shared_ptr<mbgl::UpdateParamete
 
 void QMapboxGLMapRenderer::updateFramebuffer(quint32 fbo, const mbgl::Size &size)
 {
+    MBGL_VERIFY_THREAD(tid);
+
     m_backend.updateFramebuffer(fbo, size);
 }
 
 void QMapboxGLMapRenderer::render()
 {
+    MBGL_VERIFY_THREAD(tid);
+
     std::shared_ptr<mbgl::UpdateParameters> params;
     {
         // Lock on the parameters
diff --git a/platform/qt/src/qmapboxgl_map_renderer.hpp b/platform/qt/src/qmapboxgl_map_renderer.hpp
index f7523604c7..aed6434cb0 100644
--- a/platform/qt/src/qmapboxgl_map_renderer.hpp
+++ b/platform/qt/src/qmapboxgl_map_renderer.hpp
@@ -8,6 +8,7 @@
 #include <mbgl/renderer/renderer_observer.hpp>
 #include <mbgl/storage/default_file_source.hpp>
 #include <mbgl/util/shared_thread_pool.hpp>
+#include <mbgl/util/util.hpp>
 
 #include <QtGlobal>
 
@@ -40,6 +41,8 @@ public:
     void updateParameters(std::shared_ptr<mbgl::UpdateParameters>);
 
 private:
+    MBGL_STORE_THREAD(tid)
+
     Q_DISABLE_COPY(QMapboxGLMapRenderer)
 
     std::mutex m_updateMutex;
diff --git a/platform/qt/src/qmapboxgl_p.hpp b/platform/qt/src/qmapboxgl_p.hpp
index b9a4b37ae5..31fb5138bf 100644
--- a/platform/qt/src/qmapboxgl_p.hpp
+++ b/platform/qt/src/qmapboxgl_p.hpp
@@ -31,27 +31,32 @@ public:
     void setObserver(mbgl::RendererObserver &) final;
     void update(std::shared_ptr<mbgl::UpdateParameters>) final;
 
-    mbgl::EdgeInsets margins;
-
-    QMapboxGL *q_ptr { nullptr };
+    // These need to be called on the same thread.
+    void createRenderer();
+    void destroyRenderer();
+    void render();
+    void setFramebufferObject(quint32 fbo, const QSize& size);
 
-    std::shared_ptr<mbgl::DefaultFileSource> fileSourceObj;
-    std::shared_ptr<mbgl::ThreadPool> threadPool;
+    mbgl::EdgeInsets margins;
     std::unique_ptr<mbgl::Map> mapObj;
 
-    std::unique_ptr<QMapboxGLMapObserver> mapObserver;
-    std::unique_ptr<QMapboxGLMapRenderer> mapRenderer;
-    std::shared_ptr<mbgl::RendererObserver> rendererObserver;
-
-    QMapboxGLSettings::GLContextMode mode;
-    qreal pixelRatio;
-
-    std::atomic_flag renderQueued = ATOMIC_FLAG_INIT;
 signals:
     void needsRendering();
 
 private:
     Q_DISABLE_COPY(QMapboxGLPrivate)
 
+    std::recursive_mutex m_mapRendererMutex;
+    std::shared_ptr<mbgl::RendererObserver> m_rendererObserver;
+
+    std::unique_ptr<QMapboxGLMapObserver> m_mapObserver;
+    std::shared_ptr<mbgl::DefaultFileSource> m_fileSourceObj;
+    std::shared_ptr<mbgl::ThreadPool> m_threadPool;
+    std::unique_ptr<QMapboxGLMapRenderer> m_mapRenderer;
     std::unique_ptr<mbgl::Actor<mbgl::ResourceTransform>> m_resourceTransform;
+
+    QMapboxGLSettings::GLContextMode m_mode;
+    qreal m_pixelRatio;
+
+    std::atomic_flag m_renderQueued = ATOMIC_FLAG_INIT;
 };