Scene3DRenderer: ensure window pointer stays the same while renderingv5.9.4

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.

This also covers the case where the item is destroyed but the cross-thread
signals are not received before rendering happens.

Task-number: QTBUG-63897
Task-number: QTBUG-65407
Change-Id: I5f2797e2210b532f9086ed186959fce27ea9f514
Reviewed-by: Sean Harmer <sean.harmer@kdab.com>

2 files changed, 20 insertions, 9 deletions

diff --git a/src/quick3d/imports/scene3d/scene3drenderer.cpp b/src/quick3d/imports/scene3d/scene3drenderer.cpp
index 23aaf9ed0..37ae8e48b 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;
@@ -269,7 +277,7 @@ void Scene3DRenderer::render()
         // We are in the QSGRenderThread (doing a direct call would result in a race)
         static const QMetaMethod setItemAreaAndDevicePixelRatio = setItemAreaAndDevicePixelRatioMethod();
         setItemAreaAndDevicePixelRatio.invoke(m_item, Qt::QueuedConnection, Q_ARG(QSize, boundingRectSize),
-                                              Q_ARG(qreal, window->effectiveDevicePixelRatio()));
+                                              Q_ARG(qreal, m_window->effectiveDevicePixelRatio()));
     }
 
     // Rebuild FBO and textures if never created or a resize has occurred
@@ -283,7 +291,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());
     }
 
@@ -323,13 +331,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;