Scene3DRenderer: ensure window pointer stays the same while rendering

It appears that the pointer value for the window could in some cases be
updated meanwhile we were rendering, which resulted in random crashes.

We now save the window pointer to a member variable on startup or whenever the
windowChanged signal is fired and use a mutex to ensure the pointer won't
change while we are rendering.

Change-Id: I93ade6bd549886b75140484c83fe59187d47ed9f
Task-number: QTBUG-63897
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

2 files changed, 19 insertions, 8 deletions

diff --git a/src/quick3d/imports/scene3d/scene3drenderer.cpp b/src/quick3d/imports/scene3d/scene3drenderer.cpp
index 26e966f76..01977f72b 100644
--- a/src/quick3d/imports/scene3d/scene3drenderer.cpp
+++ b/src/quick3d/imports/scene3d/scene3drenderer.cpp
@@ -131,6 +131,7 @@ Scene3DRenderer::Scene3DRenderer(Scene3DItem *item, Qt3DCore::QAspectEngine *asp
     , m_texture(nullptr)
     , m_node(nullptr)
     , m_cleaner(nullptr)
+    , m_window(nullptr)
     , m_multisample(false) // this value is not used, will be synced from the Scene3DItem instead
     , m_lastMultisample(false)
     , m_needsShutdown(true)
@@ -138,9 +139,16 @@ Scene3DRenderer::Scene3DRenderer(Scene3DItem *item, Qt3DCore::QAspectEngine *asp
     Q_CHECK_PTR(m_item);
     Q_CHECK_PTR(m_item->window());
 
+    m_window = m_item->window();
     QObject::connect(m_item->window(), &QQuickWindow::beforeRendering, this, &Scene3DRenderer::render, Qt::DirectConnection);
     QObject::connect(m_item->window(), &QQuickWindow::sceneGraphInvalidated, this, &Scene3DRenderer::onSceneGraphInvalidated, Qt::DirectConnection);
+    // So that we can schedule the cleanup
     QObject::connect(m_item, &QQuickItem::windowChanged, this, &Scene3DRenderer::onWindowChanged, Qt::QueuedConnection);
+    // Main thread -> updates the rendering window
+    QObject::connect(m_item, &QQuickItem::windowChanged, [this] (QQuickWindow *w) {
+        QMutexLocker l(&m_windowMutex);
+        m_window = w;
+    });
 
     Q_ASSERT(QOpenGLContext::currentContext());
     ContextSaver saver;
@@ -245,11 +253,11 @@ void Scene3DRenderer::setSGNode(Scene3DSGNode *node)
 
 void Scene3DRenderer::render()
 {
-    if (!m_item || !m_item->window())
+    QMutexLocker l(&m_windowMutex);
+    // Lock to ensure the window doesn't change while we are rendering
+    if (!m_item || !m_window)
         return;
 
-    QQuickWindow *window = m_item->window();
-
     if (m_aspectEngine->rootEntity() != m_item->entity())
         scheduleRootEntityChange();
 
@@ -257,10 +265,10 @@ void Scene3DRenderer::render()
 
     // The OpenGL state may be dirty from the previous QtQuick nodes, so reset
     // it here to give Qt3D the clean state it expects
-    window->resetOpenGLState();
+    m_window->resetOpenGLState();
 
     const QSize boundingRectSize = m_item->boundingRect().size().toSize();
-    const QSize currentSize = boundingRectSize * window->effectiveDevicePixelRatio();
+    const QSize currentSize = boundingRectSize * m_window->effectiveDevicePixelRatio();
     const bool sizeHasChanged = currentSize != m_lastSize;
     const bool multisampleHasChanged = m_multisample != m_lastMultisample;
     const bool forceRecreate = sizeHasChanged || multisampleHasChanged;
@@ -282,7 +290,7 @@ void Scene3DRenderer::render()
 
     if (m_finalFBO.isNull() || forceRecreate) {
         m_finalFBO.reset(createFramebufferObject(currentSize));
-        m_texture.reset(window->createTextureFromId(m_finalFBO->texture(), m_finalFBO->size(), QQuickWindow::TextureHasAlphaChannel));
+        m_texture.reset(m_window->createTextureFromId(m_finalFBO->texture(), m_finalFBO->size(), QQuickWindow::TextureHasAlphaChannel));
         m_node->setTexture(m_texture.data());
     }
 
@@ -322,13 +330,13 @@ void Scene3DRenderer::render()
 
     // Reset the state used by the Qt Quick scenegraph to avoid any
     // interference when rendering the rest of the UI.
-    window->resetOpenGLState();
+    m_window->resetOpenGLState();
 
     // Mark material as dirty to request a new frame
     m_node->markDirty(QSGNode::DirtyMaterial);
 
     // Request next frame
-    window->update();
+    m_window->update();
 }
 
 } // namespace Qt3DRender
diff --git a/src/quick3d/imports/scene3d/scene3drenderer_p.h b/src/quick3d/imports/scene3d/scene3drenderer_p.h
index ab1db1010..7a85bc774 100644
--- a/src/quick3d/imports/scene3d/scene3drenderer_p.h
+++ b/src/quick3d/imports/scene3d/scene3drenderer_p.h
@@ -53,6 +53,7 @@
 
 #include <QtCore/QObject>
 #include <QtCore/qsize.h>
+#include <QtCore/QMutex>
 
 QT_BEGIN_NAMESPACE
 
@@ -102,6 +103,8 @@ private:
     QScopedPointer<QSGTexture> m_texture;
     Scene3DSGNode *m_node; // Will be released by the QtQuick SceneGraph
     Scene3DCleaner *m_cleaner;
+    QQuickWindow *m_window;
+    QMutex m_windowMutex;
     QSize m_lastSize;
     bool m_multisample;
     bool m_lastMultisample;