Ensure offscreen surface used during cleanup is created on gui thread

Windows uses a QWindow to back QOffscreenSurface so we must needs
create it on the gui thread. To complicate matters we don't know the
format used by Qt 3D until the Renderer is initialized. So we have to
defer creation of the offscreen surface until that time and do it in
the gui thread with the correct format.

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

10 files changed, 245 insertions, 8 deletions

diff --git a/src/render/backend/abstractrenderer_p.h b/src/render/backend/abstractrenderer_p.h
index 6edebf854..8b17cbf45 100644
--- a/src/render/backend/abstractrenderer_p.h
+++ b/src/render/backend/abstractrenderer_p.h
@@ -54,6 +54,7 @@
 #include <Qt3DRender/private/qt3drender_global_p.h>
 #include <Qt3DCore/qaspectjob.h>
 #include <Qt3DCore/qnodeid.h>
+#include <QtGui/qsurfaceformat.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -79,7 +80,7 @@ class Entity;
 class FrameGraphNode;
 class RenderSettings;
 class BackendNode;
-
+class OffscreenSurfaceHelper;
 
 class QT3DRENDERSHARED_PRIVATE_EXPORT AbstractRenderer
 {
@@ -149,6 +150,9 @@ public:
     virtual RenderSettings *settings() const = 0;
 
     virtual QVariant executeCommand(const QStringList &args) = 0;
+
+    virtual void setOffscreenSurfaceHelper(OffscreenSurfaceHelper *helper) = 0;
+    virtual QSurfaceFormat format() = 0;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(AbstractRenderer::BackendNodeDirtySet)
diff --git a/src/render/backend/offscreensurfacehelper.cpp b/src/render/backend/offscreensurfacehelper.cpp
new file mode 100644
index 000000000..89dc6211f
--- /dev/null
+++ b/src/render/backend/offscreensurfacehelper.cpp
@@ -0,0 +1,81 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "offscreensurfacehelper_p.h"
+
+#include <Qt3DRender/private/abstractrenderer_p.h>
+#include <QtGui/qoffscreensurface.h>
+#include <QtGui/qsurfaceformat.h>
+#include <QtCore/qcoreapplication.h>
+#include <QtCore/qthread.h>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3DRender {
+namespace Render {
+
+/*! \internal */
+OffscreenSurfaceHelper::OffscreenSurfaceHelper(AbstractRenderer *renderer,
+                                               QObject *parent)
+    : QObject(parent)
+    , m_renderer(renderer)
+    , m_offscreenSurface(nullptr)
+{
+    Q_ASSERT(renderer);
+}
+
+/*!
+ * \internal
+ * Called in context of main thread to create an offscreen surface
+ * which can later be made current with the Qt 3D OpenGL context to
+ * then allow graphics resources to be released cleanly.
+ */
+void OffscreenSurfaceHelper::createOffscreenSurface()
+{
+    Q_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread());
+    m_offscreenSurface = new QOffscreenSurface;
+    m_offscreenSurface->setParent(this);
+    m_offscreenSurface->setFormat(m_renderer->format());
+    m_offscreenSurface->create();
+}
+
+} // namespace Render
+} // namespace Qt3DRender
+
+QT_END_NAMESPACE
diff --git a/src/render/backend/offscreensurfacehelper_p.h b/src/render/backend/offscreensurfacehelper_p.h
new file mode 100644
index 000000000..a2c383162
--- /dev/null
+++ b/src/render/backend/offscreensurfacehelper_p.h
@@ -0,0 +1,86 @@
+/****************************************************************************
+**
+** Copyright (C) 2016 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt3D module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 3 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL3 included in the
+** packaging of this file. Please review the following information to
+** ensure the GNU Lesser General Public License version 3 requirements
+** will be met: https://www.gnu.org/licenses/lgpl-3.0.html.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 2.0 or (at your option) the GNU General
+** Public license version 3 or any later version approved by the KDE Free
+** Qt Foundation. The licenses are as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-2.0.html and
+** https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QT3DRENDER_RENDER_OFFSCREENSURFACEHELPER_H
+#define QT3DRENDER_RENDER_OFFSCREENSURFACEHELPER_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API.  It exists for the convenience
+// of other Qt classes.  This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <QObject>
+
+QT_BEGIN_NAMESPACE
+
+class QOffscreenSurface;
+
+namespace Qt3DRender {
+namespace Render {
+
+class AbstractRenderer;
+
+class OffscreenSurfaceHelper : public QObject
+{
+    Q_OBJECT
+public:
+    OffscreenSurfaceHelper(AbstractRenderer *renderer,
+                           QObject *parent = nullptr);
+    inline QOffscreenSurface *offscreenSurface() const { return m_offscreenSurface; }
+
+public slots:
+    void createOffscreenSurface();
+
+private:
+    Render::AbstractRenderer *m_renderer;
+    QOffscreenSurface *m_offscreenSurface;
+};
+
+} // namespace Render
+} // namespace Qt3DRender
+
+QT_END_NAMESPACE
+
+#endif // QT3DRENDER_RENDER_OFFSCREENSURFACEHELPER_H
diff --git a/src/render/backend/render-backend.pri b/src/render/backend/render-backend.pri
index 9dc208f8b..7196a25f4 100644
--- a/src/render/backend/render-backend.pri
+++ b/src/render/backend/render-backend.pri
@@ -38,7 +38,8 @@ HEADERS += \
     $$PWD/commandexecuter_p.h \
     $$PWD/uniform_p.h \
     $$PWD/shaderparameterpack_p.h \
-    $$PWD/renderviewbuilder_p.h
+    $$PWD/renderviewbuilder_p.h \
+    $$PWD/offscreensurfacehelper_p.h
 
 SOURCES += \
     $$PWD/renderthread.cpp \
@@ -70,5 +71,6 @@ SOURCES += \
     $$PWD/openglvertexarrayobject.cpp \
     $$PWD/uniform.cpp \
     $$PWD/shaderparameterpack.cpp \
-    $$PWD/renderviewbuilder.cpp
+    $$PWD/renderviewbuilder.cpp \
+    $$PWD/offscreensurfacehelper.cpp
 
diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index af87ecd86..2449880d4 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -82,6 +82,7 @@
 #include <Qt3DRender/private/platformsurfacefilter_p.h>
 #include <Qt3DRender/private/loadbufferjob_p.h>
 #include <Qt3DRender/private/rendercapture_p.h>
+#include <Qt3DRender/private/offscreensurfacehelper_p.h>
 
 #include <Qt3DRender/qcameralens.h>
 #include <Qt3DCore/private/qeventfilterservice_p.h>
@@ -169,6 +170,7 @@ Renderer::Renderer(QRenderAspect::RenderType type)
     , m_shaderGathererJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([this] { lookForDirtyShaders(); }, JobTypes::DirtyShaderGathering))
     , m_syncTextureLoadingJob(Render::GenericLambdaJobPtr<std::function<void ()>>::create([] {}, JobTypes::SyncTextureLoading))
     , m_ownedContext(false)
+    , m_offscreenHelper(nullptr)
     #ifdef QT3D_JOBS_RUN_STATS
     , m_commandExecuter(new Qt3DRender::Debug::CommandExecuter(this))
     #endif
@@ -291,6 +293,14 @@ void Renderer::initialize()
     // The context will be made current later on (at render time)
     m_graphicsContext->setOpenGLContext(ctx);
 
+    // Store the format used by the context and queue up creating an
+    // offscreen surface in the main thread so that it is available
+    // for use when we want to shutdown the renderer. We need to create
+    // the offscreen surface on the main thread because on some platforms
+    // (MS Windows), an offscreen surface is just a hidden QWindow.
+    m_format = ctx->format();
+    QMetaObject::invokeMethod(m_offscreenHelper, "createOffscreenSurface");
+
     // Awake setScenegraphRoot in case it was waiting
     m_waitForInitializationToBeCompleted.release(1);
     // Allow the aspect manager to proceed
@@ -340,13 +350,16 @@ void Renderer::releaseGraphicsResources()
         return;
 
     // Try to temporarily make the context current so we can free up any resources
+    QMutexLocker locker(&m_offscreenSurfaceMutex);
+    QOffscreenSurface *offscreenSurface = m_offscreenHelper->offscreenSurface();
+    if (!offscreenSurface) {
+        qWarning() << "Failed to make context current: OpenGL resources will not be destroyed";
+        return;
+    }
+
     QOpenGLContext *context = m_graphicsContext->openGLContext();
     Q_ASSERT(context);
-    QSurfaceFormat format = context->format();
-    QOffscreenSurface offscreenSurface;
-    offscreenSurface.setFormat(format);
-    offscreenSurface.create();
-    if (context->makeCurrent(&offscreenSurface)) {
+    if (context->makeCurrent(offscreenSurface)) {
 
         // Clean up the graphics context and any resources
         const QVector<GLTexture*> activeTextures = m_nodesManager->glTextureManager()->activeResources();
@@ -664,6 +677,21 @@ QVariant Renderer::executeCommand(const QStringList &args)
     return QVariant();
 }
 
+/*!
+    \internal
+    Called in the context of the aspect thread from QRenderAspect::onRegistered
+*/
+void Renderer::setOffscreenSurfaceHelper(OffscreenSurfaceHelper *helper)
+{
+    QMutexLocker locker(&m_offscreenSurfaceMutex);
+    m_offscreenHelper = helper;
+}
+
+QSurfaceFormat Renderer::format()
+{
+    return m_format;
+}
+
 // When this function is called, we must not be processing the commands for frame n+1
 void Renderer::prepareCommandsSubmission(const QVector<RenderView *> &renderViews)
 {
diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h
index ad86fc3ce..da5421d6c 100644
--- a/src/render/backend/renderer_p.h
+++ b/src/render/backend/renderer_p.h
@@ -232,6 +232,8 @@ public:
     bool isReadyToSubmit();
 
     QVariant executeCommand(const QStringList &args) Q_DECL_OVERRIDE;
+    void setOffscreenSurfaceHelper(OffscreenSurfaceHelper *helper) Q_DECL_OVERRIDE;
+    QSurfaceFormat format() Q_DECL_OVERRIDE;
 
     struct ViewSubmissionResultData
     {
@@ -270,6 +272,7 @@ private:
     ShaderParameterPack m_defaultUniformPack;
 
     QScopedPointer<GraphicsContext> m_graphicsContext;
+    QSurfaceFormat m_format;
 
     RenderQueue *m_renderQueue;
     QScopedPointer<RenderThread> m_renderThread;
@@ -328,6 +331,9 @@ private:
 
     bool m_ownedContext;
 
+    OffscreenSurfaceHelper *m_offscreenHelper;
+    QMutex m_offscreenSurfaceMutex;
+
 #ifdef QT3D_JOBS_RUN_STATS
     QScopedPointer<Qt3DRender::Debug::CommandExecuter> m_commandExecuter;
     friend class Qt3DRender::Debug::CommandExecuter;
diff --git a/src/render/frontend/qrenderaspect.cpp b/src/render/frontend/qrenderaspect.cpp
index 4ba5c208a..14555d15a 100644
--- a/src/render/frontend/qrenderaspect.cpp
+++ b/src/render/frontend/qrenderaspect.cpp
@@ -121,6 +121,7 @@
 #include <Qt3DRender/private/rendersettings_p.h>
 #include <Qt3DRender/private/backendnode_p.h>
 #include <Qt3DRender/private/rendercapture_p.h>
+#include <Qt3DRender/private/offscreensurfacehelper_p.h>
 
 #include <Qt3DCore/qentity.h>
 #include <Qt3DCore/qtransform.h>
@@ -442,6 +443,12 @@ void QRenderAspect::onRegistered()
     d->m_renderer = new Render::Renderer(d->m_renderType);
     d->m_renderer->setNodeManagers(d->m_nodeManagers);
 
+    // Create a helper for deferring creation of an offscreen surface used during cleanup
+    // to the main thread, after we knwo what the surface format in use is.
+    d->m_offscreenHelper = new Render::OffscreenSurfaceHelper(d->m_renderer);
+    d->m_offscreenHelper->moveToThread(QCoreApplication::instance()->thread());
+    d->m_renderer->setOffscreenSurfaceHelper(d->m_offscreenHelper);
+
     // Register backend types now that we have a renderer
     d->registerBackendTypes();
 
@@ -479,6 +486,11 @@ void QRenderAspect::onUnregistered()
     // Waits for the render thread to join (if using threaded renderer)
     delete d->m_renderer;
     d->m_renderer = nullptr;
+
+    // Queue the offscreen surface helper for deletion on the main thread.
+    // That will take care of deleting the offscreen surface itself.
+    d->m_offscreenHelper->deleteLater();
+    d->m_offscreenHelper = nullptr;
 }
 
 QVector<Qt3DCore::QAspectJobPtr> QRenderAspectPrivate::createGeometryRendererJobs()
diff --git a/src/render/frontend/qrenderaspect_p.h b/src/render/frontend/qrenderaspect_p.h
index f196c2ec2..d2c789ada 100644
--- a/src/render/frontend/qrenderaspect_p.h
+++ b/src/render/frontend/qrenderaspect_p.h
@@ -67,6 +67,10 @@ class AbstractRenderer;
 class NodeManagers;
 }
 
+namespace Render {
+class OffscreenSurfaceHelper;
+}
+
 class QT3DRENDERSHARED_PRIVATE_EXPORT QRenderAspectPrivate : public Qt3DCore::QAbstractAspectPrivate
 {
 public:
@@ -89,6 +93,7 @@ public:
     bool m_initialized;
     QList<QSceneIOHandler *> m_sceneIOHandler;
     QRenderAspect::RenderType m_renderType;
+    Render::OffscreenSurfaceHelper *m_offscreenHelper;
 };
 
 }
diff --git a/tests/auto/render/commons/testrenderer.cpp b/tests/auto/render/commons/testrenderer.cpp
index cda741936..87e60a263 100644
--- a/tests/auto/render/commons/testrenderer.cpp
+++ b/tests/auto/render/commons/testrenderer.cpp
@@ -66,4 +66,14 @@ QVariant TestRenderer::executeCommand(const QStringList &args)
     return QVariant();
 }
 
+void TestRenderer::setOffscreenSurfaceHelper(Qt3DRender::Render::OffscreenSurfaceHelper *helper)
+{
+    Q_UNUSED(helper);
+}
+
+QSurfaceFormat TestRenderer::format()
+{
+    return QSurfaceFormat();
+}
+
 QT_END_NAMESPACE
diff --git a/tests/auto/render/commons/testrenderer.h b/tests/auto/render/commons/testrenderer.h
index 05aafbbd8..2e572582e 100644
--- a/tests/auto/render/commons/testrenderer.h
+++ b/tests/auto/render/commons/testrenderer.h
@@ -75,6 +75,9 @@ public:
     void resetDirty();
     QVariant executeCommand(const QStringList &args) Q_DECL_OVERRIDE;
 
+    void setOffscreenSurfaceHelper(Qt3DRender::Render::OffscreenSurfaceHelper *helper) Q_DECL_OVERRIDE;
+    QSurfaceFormat format() Q_DECL_OVERRIDE;
+
 protected:
     Qt3DRender::Render::AbstractRenderer::BackendNodeDirtySet m_changes;
 };