Renderer/Scene3D: fixes to make it work without setData

Change-Id: Iec1d3bab2e3a5a25d1cbb5129addebd83b225580
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

5 files changed, 41 insertions, 16 deletions

diff --git a/src/quick3d/imports/scene3d/scene3ditem.cpp b/src/quick3d/imports/scene3d/scene3ditem.cpp
index 552204ffd..a69a63ca6 100644
--- a/src/quick3d/imports/scene3d/scene3ditem.cpp
+++ b/src/quick3d/imports/scene3d/scene3ditem.cpp
@@ -202,7 +202,10 @@ void Scene3DItem::setWindowSurface(QObject *rootObject)
         return;
     }
 
-    surfaceSelector->setWindow(this->window());
+    // Set the item's window surface if it appears
+    // the surface wasn't set on the surfaceSelector
+    if (!surfaceSelector->window())
+        surfaceSelector->setWindow(this->window());
 }
 
 void Scene3DItem::setCameraAspectModeHelper()
diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index 94760bda5..8aac0f6dd 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -569,6 +569,16 @@ void Renderer::doRender()
     // Render using current device state and renderer configuration
     const bool submissionSucceeded = submitRenderViews();
 
+    // Note: submitRenderViews returns false when
+    // * we cannot render because a shutdown has been scheduled
+    // * the renderview is incomplete (only when rendering with a Scene3D)
+    // Otherwise returns true even for cases like
+    // * No render view
+    // * No surface set
+    // * OpenGLContext failed to be set current
+    // This behavior is important as we need to
+    // call proceedToNextFrame despite rendering errors that aren't fatal
+
     // Only reset renderQueue and proceed to next frame if the submission
     // succeeded or it we are using a render thread.
 
@@ -641,12 +651,6 @@ bool Renderer::submitRenderViews()
     // we've been told to render before rendering
     if (m_renderThread) { // Prevent ouf of order execution
         m_submitRenderViewsSemaphore.acquire(1);
-
-        // Early return if we have been unlocked because of
-        // shutdown
-        if (!m_running.load())
-            return false;
-
         // When using Thread rendering, the semaphore should only
         // be released when the frame queue is complete and there's
         // something to render
@@ -671,6 +675,7 @@ bool Renderer::submitRenderViews()
 
     // Lock the mutex to protect access to m_surface and check if we are still set
     // to the running state and that we have a valid surface on which to draw
+    // TO DO: Is that still needed given the surface changes
     QMutexLocker locker(&m_mutex);
     const QVector<Render::RenderView *> renderViews = m_renderQueue->nextFrameQueue();
     if (!canRender()) {
@@ -732,7 +737,6 @@ bool Renderer::submitRenderViews()
             m_graphicsContext->setCurrentStateSet(m_defaultRenderStateSet);
         }
 
-
         // Set RenderView render state
         RenderStateSet *renderViewStateSet = renderView->stateSet();
         if (renderViewStateSet)
@@ -767,14 +771,15 @@ bool Renderer::submitRenderViews()
         frameElapsed = timer.elapsed();
     }
 
-    // Reset state to the default state if the last stateset is not the
-    // defaultRenderStateSet
-    if (m_graphicsContext->currentStateSet() != m_defaultRenderStateSet)
-        m_graphicsContext->setCurrentStateSet(m_defaultRenderStateSet);
-
-    // Finish up with last surface used in the list of RenderViews
-    m_graphicsContext->endDrawing(boundFboId == m_graphicsContext->defaultFBO());
+    if (surface) {
+        // Reset state to the default state if the last stateset is not the
+        // defaultRenderStateSet
+        if (m_graphicsContext->currentStateSet() != m_defaultRenderStateSet)
+            m_graphicsContext->setCurrentStateSet(m_defaultRenderStateSet);
 
+        // Finish up with last surface used in the list of RenderViews
+        m_graphicsContext->endDrawing(boundFboId == m_graphicsContext->defaultFBO());
+    }
     // Delete all the RenderViews which will clear the allocators
     // that were used for their allocation
     qDeleteAll(renderViews);
diff --git a/src/render/framegraph/qrendersurfaceselector.cpp b/src/render/framegraph/qrendersurfaceselector.cpp
index e218672e8..4f6533e5a 100644
--- a/src/render/framegraph/qrendersurfaceselector.cpp
+++ b/src/render/framegraph/qrendersurfaceselector.cpp
@@ -46,6 +46,7 @@ namespace Qt3DRender {
 
 QRenderSurfaceSelectorPrivate::QRenderSurfaceSelectorPrivate()
     : Qt3DRender::QFrameGraphNodePrivate()
+    , m_surface(Q_NULLPTR)
 {
 }
 
@@ -73,7 +74,7 @@ QSurface *QRenderSurfaceSelector::surface() const
 QWindow *QRenderSurfaceSelector::window() const
 {
     Q_D(const QRenderSurfaceSelector);
-    if (d->m_surface->surfaceClass() == QSurface::Window)
+    if (d->m_surface && d->m_surface->surfaceClass() == QSurface::Window)
         return static_cast<QWindow *>(d->m_surface);
     return Q_NULLPTR;
 }
diff --git a/src/render/graphicshelpers/graphicscontext.cpp b/src/render/graphicshelpers/graphicscontext.cpp
index 67d61deff..53dcaaef6 100644
--- a/src/render/graphicshelpers/graphicscontext.cpp
+++ b/src/render/graphicshelpers/graphicscontext.cpp
@@ -185,6 +185,10 @@ bool GraphicsContext::beginDrawing(QSurface *surface, const QColor &color)
     if (m_ownCurrent && !makeCurrent(m_surface))
         return false;
 
+    // Sets or Create the correct m_glHelper
+    // for the current surface
+    activateGLHelper();
+
     GLint err = m_gl->functions()->glGetError();
     if (err != 0) {
         qCWarning(Backend) << Q_FUNC_INFO << "glGetError:" << err;
@@ -309,6 +313,17 @@ void GraphicsContext::setOpenGLContext(QOpenGLContext* ctx, QSurface *surface)
     }
 }
 
+void GraphicsContext::activateGLHelper()
+{
+    // Sets the correct GL Helper depending on the surface
+    // If no helper exists, create one
+    m_glHelper = m_glHelpers.value(m_surface);
+    if (!m_glHelper) {
+        m_glHelper = resolveHighestOpenGLFunctions();
+        m_glHelpers.insert(m_surface, m_glHelper);
+    }
+}
+
 bool GraphicsContext::makeCurrent(QSurface *surface)
 {
     Q_ASSERT(m_gl);
diff --git a/src/render/graphicshelpers/graphicscontext_p.h b/src/render/graphicshelpers/graphicscontext_p.h
index 090f05937..cf7e36e6b 100644
--- a/src/render/graphicshelpers/graphicscontext_p.h
+++ b/src/render/graphicshelpers/graphicscontext_p.h
@@ -118,6 +118,7 @@ public:
     QOpenGLContext *openGLContext() { return m_gl; }
     bool makeCurrent(QSurface *surface);
     void doneCurrent();
+    void activateGLHelper();
 
     void activateShader(Shader* shader);
     QOpenGLShaderProgram *containsProgram(const ProgramDNA &dna);