Cleanly exit the render thread

We have to mutex protect accesses to the m_surface variable in the
Renderer as the main thread may call destroy() on the corresponding
QWindow. This can happen in response to the window being closed or
changing QScreens.

Calling swapBuffers() on a QSurface that has been destroyed does not
fail gracefully but crashes. This change works around this by using
the new QPlatformSurfaceEvent in QtGui which is sent to the window
or offsceen surface when the platform surface has been created or
is about to be destroyed.

This event is used to inform the renderer to stop rendering to the
window surface in a safe manner, before the native surface gets
destroyed.

The Renderer::submitRenderViews() now checks at the start of each
frame that the m_surface is still valid.

There is still another crash at shutdown if applications are
sending a lot of updates to the backend.

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

13 files changed, 388 insertions, 50 deletions

diff --git a/src/core/aspects/qaspectengine.cpp b/src/core/aspects/qaspectengine.cpp
index c5574ff39..0d2a7f2db 100644
--- a/src/core/aspects/qaspectengine.cpp
+++ b/src/core/aspects/qaspectengine.cpp
@@ -150,6 +150,7 @@ void QAspectEngine::setData(const QVariantMap &data)
     Q_D(QAspectEngine);
     QMetaObject::invokeMethod(d->m_aspectThread->aspectManager(),
                               "setData",
+                              Qt::BlockingQueuedConnection,
                               Q_ARG(const QVariantMap &, data));
 }
 
diff --git a/src/core/aspects/qaspectengine.h b/src/core/aspects/qaspectengine.h
index d4fdffd77..1005e0ecc 100644
--- a/src/core/aspects/qaspectengine.h
+++ b/src/core/aspects/qaspectengine.h
@@ -48,8 +48,6 @@
 
 QT_BEGIN_NAMESPACE
 
-class QSurface;
-
 namespace Qt3D {
 
 class QAbstractAspect;
diff --git a/src/core/aspects/qaspectengine_p.h b/src/core/aspects/qaspectengine_p.h
index 68d0af221..5770937c2 100644
--- a/src/core/aspects/qaspectengine_p.h
+++ b/src/core/aspects/qaspectengine_p.h
@@ -43,6 +43,7 @@
 #define QT3D_QASPECTENGINE_P_H
 
 #include <private/qobject_p.h>
+#include <Qt3DCore/qaspectengine.h>
 #include <QtCore/qsharedpointer.h>
 
 QT_BEGIN_NAMESPACE
diff --git a/src/render/backend/platformsurfacefilter.cpp b/src/render/backend/platformsurfacefilter.cpp
new file mode 100644
index 000000000..90bc3fc3a
--- /dev/null
+++ b/src/render/backend/platformsurfacefilter.cpp
@@ -0,0 +1,122 @@
+/****************************************************************************
+**
+** Copyright (C) 2014 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: http://www.qt-project.org/legal
+**
+** 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 Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/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 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include "platformsurfacefilter_p.h"
+
+#include <Qt3DRenderer/private/renderer_p.h>
+
+#include <QMetaObject>
+#include <QPlatformSurfaceEvent>
+#include <QOffscreenSurface>
+#include <QWindow>
+
+QT_BEGIN_NAMESPACE
+
+namespace Qt3D {
+namespace Render {
+
+PlatformSurfaceFilter::PlatformSurfaceFilter(Renderer *renderer,
+                                             QObject *parent)
+    : QObject(parent)
+    , m_obj(Q_NULLPTR)
+    , m_surface(Q_NULLPTR)
+    , m_renderer(renderer)
+{
+    Q_ASSERT(m_renderer);
+    qRegisterMetaType<QSurface *>("QSurface*");
+}
+
+PlatformSurfaceFilter::~PlatformSurfaceFilter()
+{
+    if (m_obj)
+        m_obj->removeEventFilter(this);
+}
+
+void PlatformSurfaceFilter::setWindow(QWindow *window)
+{
+    setSurface(window);
+}
+
+void PlatformSurfaceFilter::setOffscreenSurface(QOffscreenSurface *offscreen)
+{
+    setSurface(offscreen);
+}
+
+bool PlatformSurfaceFilter::eventFilter(QObject *obj, QEvent *e)
+{
+    if (obj == m_obj && e->type() == QEvent::PlatformSurface) {
+        QPlatformSurfaceEvent *ev = static_cast<QPlatformSurfaceEvent *>(e);
+
+        switch (ev->surfaceEventType()) {
+        case QPlatformSurfaceEvent::SurfaceCreated:
+            setRendererSurface(m_surface);
+            break;
+
+        case QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed:
+            setRendererSurface(Q_NULLPTR);
+            break;
+
+        default:
+            qCritical("Unknown surface type");
+            Q_UNREACHABLE();
+        }
+    }
+    return false;
+}
+
+void PlatformSurfaceFilter::setRendererSurface(QSurface *surface)
+{
+    // Tell the renderer about the surface on which to render. This function
+    // is called in the context of the main thread and internally
+    // the renderer uses a private thread to submit OpenGL calls. The surface
+    // pointer within the renderer is protected by a mutex that is locked for
+    // the duration of a frame. In this way, the renderer can be sure to have
+    // a valid surface for the duration of the frame for which it is submitting
+    // draw calls. Only when the frame finishes and the mutex is unlocked does
+    // this call to Renderer::setSurface continue. Thereby blocking the main
+    // thread from destroying the platform surface before we are ready.
+    m_renderer->setSurface(surface);
+}
+
+} // namespace Render
+} // namespace Qt3D
+
+QT_END_NAMESPACE
diff --git a/src/render/backend/platformsurfacefilter_p.h b/src/render/backend/platformsurfacefilter_p.h
new file mode 100644
index 000000000..5ba3efddd
--- /dev/null
+++ b/src/render/backend/platformsurfacefilter_p.h
@@ -0,0 +1,101 @@
+/****************************************************************************
+**
+** Copyright (C) 2014 Klaralvdalens Datakonsult AB (KDAB).
+** Contact: http://www.qt-project.org/legal
+**
+** 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 Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/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 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QT3D_RENDER_PLATFORMSURFACEFILTER_H
+#define QT3D_RENDER_PLATFORMSURFACEFILTER_H
+
+#include <QtCore/qobject.h>
+#include <QtGui/qsurface.h>
+
+QT_BEGIN_NAMESPACE
+
+class QOffscreenSurface;
+class QWindow;
+
+namespace Qt3D {
+namespace Render {
+
+class Renderer;
+
+class PlatformSurfaceFilter : public QObject
+{
+    Q_OBJECT
+
+public:
+    explicit PlatformSurfaceFilter(Renderer *renderAspect,
+                                   QObject *parent = 0);
+    ~PlatformSurfaceFilter();
+
+    void setWindow(QWindow *window);
+    void setOffscreenSurface(QOffscreenSurface *offscreen);
+
+    bool eventFilter(QObject *obj, QEvent *e) Q_DECL_OVERRIDE;
+
+private:
+    void setRendererSurface(QSurface *surface);
+
+    template<class T>
+    void setSurface(T *surface)
+    {
+        if (m_obj == surface)
+            return;
+
+        if (m_obj)
+            m_obj->removeEventFilter(this);
+
+        m_surface = surface;
+        m_obj = surface;
+
+        if (m_obj)
+            m_obj->installEventFilter(this);
+    }
+
+    QObject *m_obj;
+    QSurface *m_surface;
+    Renderer *m_renderer;
+};
+
+} // namespace Render
+} // namespace Qt3D
+
+QT_END_NAMESPACE
+
+#endif // QT3D_RENDER_PLATFORMSURFACEFILTER_H
diff --git a/src/render/backend/qgraphicscontext.cpp b/src/render/backend/qgraphicscontext.cpp
index c7a514493..707ad20c1 100644
--- a/src/render/backend/qgraphicscontext.cpp
+++ b/src/render/backend/qgraphicscontext.cpp
@@ -110,12 +110,6 @@ QGraphicsContext::~QGraphicsContext()
     static_contexts.remove(m_id);
 }
 
-void QGraphicsContext::setSurface(QSurface *s)
-{
-    qCDebug(Backend) << Q_FUNC_INFO;
-    m_surface = s;
-}
-
 void QGraphicsContext::initialize()
 {
     m_initialized = true;
@@ -131,12 +125,12 @@ void QGraphicsContext::initialize()
     m_textureScopes.resize(numTexUnits);
 }
 
-bool QGraphicsContext::beginDrawing(const QColor &clearColor)
+bool QGraphicsContext::beginDrawing(QSurface *surface, const QColor &clearColor)
 {
-    if (!m_gl || !m_surface) {
-        qCWarning(Backend) << Q_FUNC_INFO << "no content or surface provided";
-        return false;
-    }
+    Q_ASSERT(surface);
+    Q_ASSERT(m_gl);
+
+    m_surface = surface;
 
     bool ok = m_gl->makeCurrent(m_surface);
     if (!ok) {
@@ -209,18 +203,17 @@ void QGraphicsContext::releaseOpenGL()
     m_bufferHash.clear();
 }
 
-void QGraphicsContext::setOpenGLContext(QOpenGLContext* ctx)
+void QGraphicsContext::setOpenGLContext(QOpenGLContext* ctx, QSurface *surface)
 {
+    Q_ASSERT(surface);
+    Q_ASSERT(ctx);
+
     releaseOpenGL();
     m_gl = ctx;
-    //    m_gl->setParent(this);
 
-    // The Context should be made current to the surface
-    // otherwise gl functions initialization fails
-
-    Q_ASSERT(m_surface);
-    m_gl->makeCurrent(m_surface);
+    m_gl->makeCurrent(surface);
     resolveHighestOpenGLFunctions();
+    m_gl->doneCurrent();
 }
 
 // That assumes that the shaderProgram in RenderShader stays the same
diff --git a/src/render/backend/qgraphicscontext_p.h b/src/render/backend/qgraphicscontext_p.h
index ad6535cb8..ed9b09207 100644
--- a/src/render/backend/qgraphicscontext_p.h
+++ b/src/render/backend/qgraphicscontext_p.h
@@ -93,11 +93,9 @@ public:
     QGraphicsContext();
     ~QGraphicsContext();
 
-    void setSurface(QSurface* s);
-
     int id() const; // unique, small integer ID of this context
 
-    bool beginDrawing(const QColor &clearColor);
+    bool beginDrawing(QSurface *surface, const QColor &clearColor);
     void clearBackBuffer(QClearBuffer::BufferType buffers);
     void endDrawing();
 
@@ -109,7 +107,7 @@ public:
      * this context
      */
     void releaseOpenGL();
-    void setOpenGLContext(QOpenGLContext* ctx);
+    void setOpenGLContext(QOpenGLContext* ctx, QSurface *surface);
     QOpenGLContext *openGLContext() { return m_gl; }
 
     void activateShader(RenderShader* shader);
diff --git a/src/render/backend/qrenderaspect.cpp b/src/render/backend/qrenderaspect.cpp
index af2961578..2b4418b67 100644
--- a/src/render/backend/qrenderaspect.cpp
+++ b/src/render/backend/qrenderaspect.cpp
@@ -98,6 +98,7 @@
 #include <Qt3DCore/private/qaspectmanager_p.h>
 
 #include <QDebug>
+#include <QOffscreenSurface>
 #include <QThread>
 #include <QWindow>
 
@@ -108,11 +109,46 @@ namespace Qt3D {
 QRenderAspectPrivate::QRenderAspectPrivate(QRenderAspect *qq)
     : QAbstractAspectPrivate(qq)
     , m_renderer(new Render::Renderer)
+    , m_surfaceEventFilter(new Render::PlatformSurfaceFilter(m_renderer))
     , m_initialized(false)
 {
     m_aspectType = QAbstractAspect::AspectRenderer;
 }
 
+void QRenderAspectPrivate::setSurface(QSurface *surface)
+{
+    if (m_surface == surface)
+        return;
+
+    m_surface = surface;
+
+    // If we have a new surface, install the platform surface event filter onto it
+    // so that we get informed when the underlying platform surface is about to be
+    // deleted and we can tell the renderer about it before it's too late.
+    if (m_surface) {
+        bool hasPlatformSurface = false;
+        switch (m_surface->surfaceClass()) {
+        case QSurface::Window: {
+            QWindow *window = static_cast<QWindow *>(m_surface);
+            m_surfaceEventFilter->setWindow(window);
+            hasPlatformSurface = (window->handle() != Q_NULLPTR);
+            break;
+        }
+
+        case QSurface::Offscreen: {
+            QOffscreenSurface *offscreen = static_cast<QOffscreenSurface *>(m_surface);
+            m_surfaceEventFilter->setOffscreenSurface(offscreen);
+            hasPlatformSurface = (offscreen->handle() != Q_NULLPTR);
+            break;
+        }
+        }
+
+        // If the window/offscreen surface has a native surface, tell the renderer
+        if (hasPlatformSurface)
+            m_renderer->setSurface(surface);
+    }
+}
+
 QRenderAspect::QRenderAspect(QObject *parent)
     : QAbstractAspect(*new QRenderAspectPrivate(this), parent)
 {
@@ -252,7 +288,7 @@ void QRenderAspect::onInitialize(const QVariantMap &data)
         surface = v.value<QSurface *>();
 
     if (surface)
-        d->m_renderer->setSurface(surface);
+        d->setSurface(surface);
 }
 
 void QRenderAspect::onCleanup()
diff --git a/src/render/backend/qrenderaspect_p.h b/src/render/backend/qrenderaspect_p.h
index ab6f6d51a..e6f2de54e 100644
--- a/src/render/backend/qrenderaspect_p.h
+++ b/src/render/backend/qrenderaspect_p.h
@@ -45,8 +45,12 @@
 #include <Qt3DCore/private/qabstractaspect_p.h>
 #include <Qt3DRenderer/qrenderaspect.h>
 
+#include <Qt3DRenderer/private/platformsurfacefilter_p.h>
+
 QT_BEGIN_NAMESPACE
 
+class QSurface;
+
 namespace Qt3D {
 
 namespace Render {
@@ -59,7 +63,15 @@ class QRenderAspectPrivate : public QAbstractAspectPrivate
 
     Q_DECLARE_PUBLIC(QRenderAspect)
 
+    void setSurface(QSurface *surface);
+
     Render::Renderer *m_renderer;
+
+    // The filter has affinity with the main thread so we have to delete it there
+    // via QScopedPointerDeleteLater
+    QScopedPointer<Render::PlatformSurfaceFilter, QScopedPointerDeleteLater> m_surfaceEventFilter;
+    QSurface *m_surface;
+
     bool m_initialized;
 };
 
diff --git a/src/render/backend/render-backend.pri b/src/render/backend/render-backend.pri
index 2dbdee26a..5ed51d0dc 100644
--- a/src/render/backend/render-backend.pri
+++ b/src/render/backend/render-backend.pri
@@ -49,7 +49,8 @@ HEADERS += \
     $$PWD/managers_p.h \
     $$PWD/handle_types_p.h \
     $$PWD/renderannotation_p.h \
-    $$PWD/renderparameter_p.h
+    $$PWD/renderparameter_p.h \
+    $$PWD/platformsurfacefilter_p.h
 
 SOURCES += \
     $$PWD/qrenderaspect.cpp \
@@ -89,4 +90,5 @@ SOURCES += \
     $$PWD/scenemanager.cpp \
     $$PWD/attachmentpack.cpp \
     $$PWD/renderannotation.cpp \
-    $$PWD/renderparameter.cpp
+    $$PWD/renderparameter.cpp \
+    $$PWD/platformsurfacefilter.cpp
diff --git a/src/render/backend/renderer.cpp b/src/render/backend/renderer.cpp
index ed748a972..8075ddc3f 100644
--- a/src/render/backend/renderer.cpp
+++ b/src/render/backend/renderer.cpp
@@ -92,6 +92,8 @@
 #include <QPluginLoader>
 #include <QDir>
 #include <QUrl>
+#include <QOffscreenSurface>
+#include <QWindow>
 
 // For Debug purposes only
 #include <QThread>
@@ -144,6 +146,9 @@ Renderer::Renderer(int cachedFrames)
     , m_debugLogger(Q_NULLPTR)
 {
     m_currentPreprocessingFrameIndex = 0;
+
+    // Set renderer as running - it will wait in the context of the
+    // RenderThread for RenderViews to be submitted
     m_running.fetchAndStoreOrdered(1);
     m_renderThread->waitForStart();
 
@@ -305,7 +310,6 @@ void Renderer::initialize()
     bool enableDebugLogging = !debugLoggingMode.isEmpty();
 
     m_graphicsContext.reset(new QGraphicsContext);
-    m_graphicsContext->setSurface(m_surface);
     m_graphicsContext->setRenderer(this);
 
     QSurfaceFormat sf = m_surface->format();
@@ -316,7 +320,7 @@ void Renderer::initialize()
     ctx->setFormat(sf);
     if (!ctx->create())
         qCWarning(Backend) << Q_FUNC_INFO << "OpenGL context creation failed";
-    m_graphicsContext->setOpenGLContext(ctx);
+    m_graphicsContext->setOpenGLContext(ctx, m_surface);
 
     if (enableDebugLogging) {
         bool supported = ctx->hasExtension("GL_KHR_debug");
@@ -344,6 +348,24 @@ void Renderer::initialize()
 }
 
 /*!
+    \internal
+
+    Called in the context of the RenderThread to do any shutdown and cleanup
+    that needs to be performed in the thread where the OpenGL context lives
+*/
+void Renderer::shutdown()
+{
+    // Stop and destroy the OpenGL logger
+    if (m_debugLogger) {
+        m_debugLogger->stopLogging();
+        m_debugLogger.reset(Q_NULLPTR);
+    }
+
+    // Clean up the graphics context
+    m_graphicsContext.reset(Q_NULLPTR);
+}
+
+/*!
  * Returns the a FrameAllocator for the frame identified by \a frameIndex in the context
  * of the caller thread. This also clears the FrameAllocator before usage.
  */
@@ -421,11 +443,24 @@ void Renderer::setSceneGraphRoot(RenderEntity *sgRoot)
 
 // Called in RenderAspect Thread context
 // Cannot do OpenGLContext initialization here
-void Renderer::setSurface(QSurface* s)
+void Renderer::setSurface(QSurface* surface)
 {
     qCDebug(Backend) << Q_FUNC_INFO << QThread::currentThread();
-    QMutexLocker locker(&m_mutex); // The will wait until initialize has been called by RenderThread::run
-    m_surface = s;
+    // Locking this mutex will wait until initialize() has been called by
+    // RenderThread::run() and the RenderThread is waiting on the
+    // m_waitForWindowToBeSetCondition condition.
+    //
+    // The first time this is called Renderer::setSurface will cause the
+    // Renderer::initialize() function to continue execution in the context
+    // of the Render Thread. On subsequent calls, just the surface will be
+    // updated.
+
+    // TODO: Remove the need for a valid surface from the renderer initialization
+    // We can use an offscreen surface to create and assess the OpenGL context.
+    // This should allow us to get rid of the "swapBuffers called on a non-exposed
+    // window" warning that we sometimes see.
+    QMutexLocker locker(&m_mutex);
+    m_surface = surface;
     m_waitForWindowToBeSetCondition.wakeOne();
 }
 
@@ -480,30 +515,47 @@ void Renderer::enqueueRenderView(Render::RenderView *renderView, int submitOrder
     }
 }
 
-// Happens in RenderThread context when all RenderViewJobs are done
-void Renderer::submitRenderViews()
+bool Renderer::canRender() const
 {
-    QMutexLocker locker(&m_mutex);
-    m_submitRenderViewsCondition.wait(locker.mutex());
-    // Allow RenderViewJobs to be processed for the next frame
-    // Without having to wait for the current RenderViews to be submitted
-    // as long as there is available space in the renderQueues.
-    // Otherwise it waits for submission to be done so as to never miss
-    // Any important state change that could be in a RenderView
-    locker.unlock();
-
     // Make sure that we've not been told to terminate whilst waiting on
     // the above wait condition
     if (!m_running.load()) {
         qCDebug(Rendering) << "RenderThread termination requested whilst waiting";
-        return;
+        return false;
     }
 
+    // Make sure that the surface we are rendering too has not been unset
+    // (probably due to the window being destroyed or changing QScreens).
+    if (!m_surface) {
+        qCDebug(Rendering) << "QSurface has been removed";
+        return false;
+    }
+
+    return true;
+}
+
+// Happens in RenderThread context when all RenderViewJobs are done
+void Renderer::submitRenderViews()
+{
+    QMutexLocker locker(&m_mutex);
+    m_submitRenderViewsCondition.wait(locker.mutex());
+    locker.unlock();
+
     QElapsedTimer timer;
     quint64 queueElapsed = 0;
     timer.start();
     while (m_renderQueues->queuedFrames() > 0)
     {
+        // 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
+        locker.relock();
+        if (!canRender()) {
+            QVector<Render::RenderView *> renderViews = m_renderQueues->nextFrameQueue();
+            qDeleteAll(renderViews);
+            m_renderQueues->popFrameQueue();
+            return;
+        }
+
         QVector<Render::RenderView *> renderViews = m_renderQueues->nextFrameQueue();
         int renderViewsCount = renderViews.size();
         quint64 frameElapsed = queueElapsed;
@@ -512,7 +564,7 @@ void Renderer::submitRenderViews()
             continue;
 
         // Bail out if we cannot make the OpenGL context current (e.g. if the window has been destroyed)
-        if (!m_graphicsContext->beginDrawing(renderViews.first()->clearColor()))
+        if (!m_graphicsContext->beginDrawing(m_surface, renderViews.first()->clearColor()))
             break;
 
         qCDebug(Memory) << Q_FUNC_INFO << "rendering frame " << renderViews.last()->frameIndex() << " Queue " << m_renderQueues->queuedFrames();
@@ -533,7 +585,12 @@ void Renderer::submitRenderViews()
             qCDebug(Rendering) << Q_FUNC_INFO << "Submitted Renderview " << i + 1 << "/" << renderViewsCount  << "in " << frameElapsed << "ms";
             frameElapsed = timer.elapsed();
         }
+
         m_graphicsContext->endDrawing();
+
+        // Let the Aspect Thread get a look in if it needs to change the surface
+        locker.unlock();
+
         qDeleteAll(renderViews);
         m_renderQueues->popFrameQueue();
         queueElapsed = timer.elapsed() - queueElapsed;
diff --git a/src/render/backend/renderer_p.h b/src/render/backend/renderer_p.h
index a8a031807..d64e1c7fe 100644
--- a/src/render/backend/renderer_p.h
+++ b/src/render/backend/renderer_p.h
@@ -195,12 +195,15 @@ public:
     void submitRenderViews();
 
     void initialize();
+    void shutdown();
 
     QFrameAllocator *currentFrameAllocator(int frameIndex);
 
     QMutex* mutex() { return &m_mutex; }
 
 private:
+    bool canRender() const;
+
     QRenderAspect *m_rendererAspect;
 
     // Frame graph root
diff --git a/src/render/backend/renderthread.cpp b/src/render/backend/renderthread.cpp
index bfad05fcf..7e71e8e63 100644
--- a/src/render/backend/renderthread.cpp
+++ b/src/render/backend/renderthread.cpp
@@ -60,7 +60,7 @@ RenderThread::RenderThread(Renderer *renderer)
 {
 }
 
-// Called by Renderer in the AspectThread
+// Called by Renderer in the context of the Aspect Thread
 void RenderThread::waitForStart( Priority priority )
 {
     qCDebug(Render::Backend) << "Starting Render thread and then going to sleep until it is ready for us...";
@@ -74,18 +74,32 @@ void RenderThread::waitForStart( Priority priority )
 void RenderThread::run()
 {
     m_mutex.lock();
-    // We lock the renderer's mutex before unlocking the render thread mutex
-    // To ensure that the Renderer's initialize waitCondition is setup while other threads
-    // are still waiting for this RenderThread's waitCondition to be satisfied
+
+    // We lock the renderer's mutex here before unlocking the render thread mutex
+    // and returning control to the calling thread (the Aspect Thread). This is
+    // to ensure that the Renderer's initialize() waitCondition is reached before
+    // other threads try to wake it up. This is guaranteed by having the
+    // Renderer::setSurface() function try to lock the renderer's mutex too.
+    // That function will block until the mutex is unlocked by the wait condition
+    // in the initialize() call below.
     QMutexLocker locker(m_renderer->mutex());
+
+    // Now we have ensured we will reach the wait condition as described above,
+    // return control to the aspect thread that created us.
     m_waitCondition.wakeOne();
     m_mutex.unlock();
-    // Renderer waits for a surface to be set
+
+    // This call to Renderer::initialize() waits for a surface to be set on the
+    // renderer in the context of the Aspect Thread
     m_renderer->initialize();
     locker.unlock();
 
+    // Enter the main OpenGL submission loop.
     m_renderer->render();
 
+    // Clean up any OpenGL resources
+    m_renderer->shutdown();
+
     qCDebug(Render::Backend) << "Exiting RenderThread";
 }