Scene3D: Revise render loop and synchronization

Before, the Scene3DRenderer marked the sg-node dirty in render, which
would then already mark it dirty for the next frame. This only works as
long as we always render, which is undesireble in some cases.
fa12f14b2 changed rendering to not always happen anymore. Thus, that
commit broke rendering under certain circumstances.

Now, Scene3DRenderer listens on a signal from the QChangeArbiter about
new pending changes. In reaction to this signal, we set an internal
dirty-flag in Scene3DRenderer. Only if this flag is set, synchronization
and rendering will happen on the Qt3D side.

Change-Id: I3b33faa5d60c270bd9b903b0e34c8fa24e2e29fd
Task-number: QTBUG-69985
Task-number: QTBUG-72923
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>
 

5 files changed, 57 insertions, 9 deletions

diff --git a/src/core/qchangearbiter.cpp b/src/core/qchangearbiter.cpp
index 2e32d6722..e0ee389c4 100644
--- a/src/core/qchangearbiter.cpp
+++ b/src/core/qchangearbiter.cpp
@@ -244,6 +244,8 @@ void QChangeArbiter::sceneChangeEvent(const QSceneChangePtr &e)
     QChangeQueue *localChangeQueue = m_tlsChangeQueue.localData();
     localChangeQueue->push_back(e);
 
+    emit receivedChange();
+
     //    qCDebug(ChangeArbiter) << "Change queue for thread" << QThread::currentThread() << "now contains" << localChangeQueue->count() << "items";
 }
 
@@ -259,6 +261,8 @@ void QChangeArbiter::sceneChangeEventWithLock(const QSceneChangeList &e)
     QChangeQueue *localChangeQueue = m_tlsChangeQueue.localData();
     qCDebug(ChangeArbiter) << Q_FUNC_INFO << "Handles " << e.size() << " changes at once";
     localChangeQueue->insert(localChangeQueue->end(), e.begin(), e.end());
+
+    emit receivedChange();
 }
 
 // Either we have the postman or we could make the QChangeArbiter agnostic to the postman
diff --git a/src/core/qchangearbiter_p.h b/src/core/qchangearbiter_p.h
index 4a82061ed..ac52273ea 100644
--- a/src/core/qchangearbiter_p.h
+++ b/src/core/qchangearbiter_p.h
@@ -120,6 +120,9 @@ public:
     static void createThreadLocalChangeQueue(void *changeArbiter);
     static void destroyThreadLocalChangeQueue(void *changeArbiter);
 
+Q_SIGNALS:
+    void receivedChange();
+
 protected:
     typedef std::vector<QSceneChangePtr> QChangeQueue;
     typedef QPair<ChangeFlags, QObserverInterface *> QObserverPair;
@@ -134,7 +137,7 @@ protected:
     void removeLockingChangeQueue(QChangeQueue *queue);
 
 private:
-    QMutex m_mutex;
+    mutable QMutex m_mutex;
     QAbstractAspectJobManager *m_jobManager;
 
     // The lists of observers indexed by observable (QNodeId).
diff --git a/src/quick3d/imports/scene3d/scene3drenderer.cpp b/src/quick3d/imports/scene3d/scene3drenderer.cpp
index b96fc516d..ca637f830 100644
--- a/src/quick3d/imports/scene3d/scene3drenderer.cpp
+++ b/src/quick3d/imports/scene3d/scene3drenderer.cpp
@@ -47,7 +47,13 @@
 #include <QtQuick/qquickwindow.h>
 
 #include <Qt3DRender/private/qrenderaspect_p.h>
+#include <Qt3DRender/private/abstractrenderer_p.h>
+#include <Qt3DRender/private/rendersettings_p.h>
 #include <Qt3DCore/private/qaspectengine_p.h>
+#include <Qt3DCore/private/qaspectmanager_p.h>
+#include <Qt3DCore/private/qchangearbiter_p.h>
+#include <Qt3DCore/private/qservicelocator_p.h>
+
 #include <scene3dcleaner_p.h>
 #include <scene3ditem_p.h>
 #include <scene3dlogging_p.h>
@@ -101,6 +107,21 @@ private:
     The Scene3DRenderer class renders a Qt3D scene as provided by a Scene3DItem.
     It owns the aspectEngine even though it doesn't instantiate it.
 
+    The render loop goes as follows:
+    \list
+    \li The main thread runs, drives Animations, etc. and causes changes to be
+    reported to the Qt3D change arbiter. The first change reported will cause
+    the scene3drenderer to be marked dirty.
+    \li The QtQuick render thread starts a new frame, synchronizes the scene
+    graph and emits afterSynchronizing. This will trigger some preparational
+    steps for rendering and mark the QSGNode dirty if the Scene3DRenderer is
+    dirty.
+    \li The QtQuick render loop emits beforeRendering. If we're marked dirty or
+    if the renderPolicy is set to Always, we'll ask the Qt3D renderer aspect to
+    render. That call is blocking. If the aspect jobs are not done, yet, the
+    renderer will exit early and we skip a frame.
+    \endlist
+
     The shutdown procedure is a two steps process that goes as follow:
 
     \list
@@ -139,6 +160,7 @@ Scene3DRenderer::Scene3DRenderer(Scene3DItem *item, Qt3DCore::QAspectEngine *asp
     , m_needsShutdown(true)
     , m_blocking(false)
     , m_forceRecreate(false)
+    , m_dirty(true) // we want to render at least once
 {
     Q_CHECK_PTR(m_item);
     Q_CHECK_PTR(m_item->window());
@@ -155,6 +177,12 @@ Scene3DRenderer::Scene3DRenderer(Scene3DItem *item, Qt3DCore::QAspectEngine *asp
         m_window = w;
     });
 
+    auto renderAspectPriv = static_cast<QRenderAspectPrivate*>(QRenderAspectPrivate::get(m_renderAspect));
+    QObject::connect(renderAspectPriv->m_aspectManager->changeArbiter(), &Qt3DCore::QChangeArbiter::receivedChange,
+                     this, [this] { m_dirty = true; }, Qt::DirectConnection);
+    QObject::connect(renderAspectPriv->m_aspectManager->changeArbiter(), &Qt3DCore::QChangeArbiter::receivedChange,
+                     m_item, &QQuickItem::update, Qt::AutoConnection);
+
     Q_ASSERT(QOpenGLContext::currentContext());
     ContextSaver saver;
     static_cast<QRenderAspectPrivate*>(QRenderAspectPrivate::get(m_renderAspect))->renderInitialize(saver.context());
@@ -169,6 +197,17 @@ Scene3DRenderer::~Scene3DRenderer()
     qCDebug(Scene3D) << Q_FUNC_INFO << QThread::currentThread();
 }
 
+bool Scene3DRenderer::shouldRender() const
+{
+    auto renderAspectPriv = static_cast<QRenderAspectPrivate*>(QRenderAspectPrivate::get(m_renderAspect));
+    return m_dirty
+        || (renderAspectPriv
+            && renderAspectPriv->m_renderer
+            && renderAspectPriv->m_renderer->settings()
+            && renderAspectPriv->m_renderer->settings()->renderPolicy() == QRenderSettings::Always);
+}
+
+
 QOpenGLFramebufferObject *Scene3DRenderer::createMultisampledFramebufferObject(const QSize &size)
 {
     QOpenGLFramebufferObjectFormat format;
@@ -249,7 +288,7 @@ void Scene3DRenderer::onWindowChanged(QQuickWindow *w)
 
 void Scene3DRenderer::synchronize()
 {
-    if (m_item && m_window) {
+    if (shouldRender() && m_item && m_window) {
         m_multisample = m_item->multisample();
 
         if (m_aspectEngine->rootEntity() != m_item->entity()) {
@@ -272,6 +311,8 @@ void Scene3DRenderer::synchronize()
         // point for the next frame
         m_lastSize = currentSize;
         m_lastMultisample = m_multisample;
+
+        m_node->markDirty(QSGNode::DirtyMaterial);
     }
 }
 
@@ -286,9 +327,11 @@ void Scene3DRenderer::render()
 {
     QMutexLocker l(&m_windowMutex);
     // Lock to ensure the window doesn't change while we are rendering
-    if (!m_window)
+    if (!m_window || !shouldRender())
         return;
 
+    m_dirty = false;
+
     ContextSaver saver;
 
     // The OpenGL state may be dirty from the previous QtQuick nodes, so reset
@@ -342,12 +385,6 @@ void Scene3DRenderer::render()
     // Reset the state used by the Qt Quick scenegraph to avoid any
     // interference when rendering the rest of the UI.
     m_window->resetOpenGLState();
-
-    // Mark material as dirty to request a new frame
-    m_node->markDirty(QSGNode::DirtyMaterial);
-
-    // Request next frame
-    m_window->update();
 }
 
 } // namespace Qt3DRender
diff --git a/src/quick3d/imports/scene3d/scene3drenderer_p.h b/src/quick3d/imports/scene3d/scene3drenderer_p.h
index e28ecbe6e..cbf11b12e 100644
--- a/src/quick3d/imports/scene3d/scene3drenderer_p.h
+++ b/src/quick3d/imports/scene3d/scene3drenderer_p.h
@@ -95,6 +95,8 @@ public Q_SLOTS:
     void onWindowChanged(QQuickWindow *w);
 
 private:
+    bool shouldRender() const;
+
     Scene3DItem *m_item; // Will be released by the QQuickWindow/QML Engine
     Qt3DCore::QAspectEngine *m_aspectEngine; // Will be released by the Scene3DRendererCleaner
     QRenderAspect *m_renderAspect; // Will be released by the aspectEngine
@@ -111,6 +113,7 @@ private:
     bool m_needsShutdown;
     bool m_blocking;
     bool m_forceRecreate;
+    bool m_dirty;
 
     friend class Scene3DCleaner;
 };
diff --git a/src/render/renderers/opengl/renderer/renderer.cpp b/src/render/renderers/opengl/renderer/renderer.cpp
index 4e40906a6..614439f0b 100644
--- a/src/render/renderers/opengl/renderer/renderer.cpp
+++ b/src/render/renderers/opengl/renderer/renderer.cpp
@@ -1688,6 +1688,7 @@ bool Renderer::shouldRender()
     // Only render if something changed during the last frame, or the last frame
     // was not rendered successfully (or render-on-demand is disabled)
     return (m_settings->renderPolicy() == QRenderSettings::Always
+            || m_renderThread == nullptr // <==> we use Scene3D
             || m_dirtyBits.marked != 0
             || m_dirtyBits.remaining != 0
             || !m_lastFrameCorrect.load());