Make sure QSGRenderLoop is cleaned up cleanly.

e13547c595913c58e6bd6a5ed80fdc729fae7d47 used a global static to clean
up QSGRenderLoop which is triggered very late, potentially after
SG backend API plugins have been unloaded. This results in crashes
when used in combination with scenegraph-playgrounds's customcontext.

Partially revert the change and instead clean up at the time of
QApp::aboutToQuit and make sure we also disconnect cleanly from
all QQuickWindows.

This change also ensures that QSGRenderLoop::windowDestroyed() gets
called for all QQuickWindows registered with the render loop. This
ensures that rendering stops and that scene graph nodes and resources
will be cleaned up regardless of whether the application has
remembered to delete the window or not. This is a good thing as it
makes the scene graph shutdown a bit cleaner.

Change-Id: I9cb9093979f8eac05542f118a6ff9cfe5c84f745
Reviewed-by: Paul Olav Tvete <paul.tvete@digia.com>

6 files changed, 56 insertions, 11 deletions

diff --git a/src/quick/scenegraph/qsgrenderloop.cpp b/src/quick/scenegraph/qsgrenderloop.cpp
index 6647ec83d0..37df425e55 100644
--- a/src/quick/scenegraph/qsgrenderloop.cpp
+++ b/src/quick/scenegraph/qsgrenderloop.cpp
@@ -45,7 +45,6 @@
 
 #include <QtCore/QCoreApplication>
 #include <QtCore/QTime>
-#include <QtCore/QScopedPointer>
 #include <QtCore/QLibraryInfo>
 #include <QtCore/private/qabstractanimation_p.h>
 
@@ -82,12 +81,25 @@ extern Q_GUI_EXPORT QImage qt_gl_read_framebuffer(const QSize &size, bool alpha_
 DEFINE_BOOL_CONFIG_OPTION(qmlNoThreadedRenderer, QML_BAD_GUI_RENDER_LOOP);
 DEFINE_BOOL_CONFIG_OPTION(qmlForceThreadedRenderer, QML_FORCE_THREADED_RENDERER); // Might trigger graphics driver threading bugs, use at own risk
 
-Q_GLOBAL_STATIC(QScopedPointer<QSGRenderLoop>, s_renderLoopInstance);
+QSGRenderLoop *QSGRenderLoop::s_instance = 0;
 
 QSGRenderLoop::~QSGRenderLoop()
 {
 }
 
+void QSGRenderLoop::cleanup()
+{
+    foreach (QQuickWindow *w, windows()) {
+        QQuickWindowPrivate *wd = QQuickWindowPrivate::get(w);
+        if (wd->windowManager == this) {
+           windowDestroyed(w);
+           wd->windowManager = 0;
+        }
+    }
+    s_instance = 0;
+    delete this;
+}
+
 class QSGGuiThreadRenderLoop : public QSGRenderLoop
 {
     Q_OBJECT
@@ -114,6 +126,8 @@ public:
     QSGContext *sceneGraphContext() const;
     QSGRenderContext *createRenderContext(QSGContext *) const { return rc; }
 
+    QList<QQuickWindow *> windows() const { return m_windows.keys(); }
+
     bool event(QEvent *);
 
     struct WindowData {
@@ -148,8 +162,8 @@ bool QSGRenderLoop::useConsistentTiming()
 
 QSGRenderLoop *QSGRenderLoop::instance()
 {
-    if (s_renderLoopInstance->isNull()) {
-        s_renderLoopInstance->reset(QSGContext::createWindowManager());
+    if (!s_instance) {
+        s_instance = QSGContext::createWindowManager();
 
         bool info = qEnvironmentVariableIsSet("QSG_INFO");
 
@@ -159,7 +173,7 @@ QSGRenderLoop *QSGRenderLoop::instance()
                 qDebug() << "QSG: using fixed animation steps";
         }
 
-        if (s_renderLoopInstance->isNull()) {
+        if (!s_instance) {
 
             enum RenderLoopType {
                 BasicRenderLoop,
@@ -191,26 +205,28 @@ QSGRenderLoop *QSGRenderLoop::instance()
             switch (loopType) {
             case ThreadedRenderLoop:
                 if (info) qDebug() << "QSG: threaded render loop";
-                s_renderLoopInstance->reset(new QSGThreadedRenderLoop());
+                s_instance = new QSGThreadedRenderLoop();
                 break;
             case WindowsRenderLoop:
                 if (info) qDebug() << "QSG: windows render loop";
-                s_renderLoopInstance->reset(new QSGWindowsRenderLoop());
+                s_instance = new QSGWindowsRenderLoop();
                 break;
             default:
                 if (info) qDebug() << "QSG: basic render loop";
-                s_renderLoopInstance->reset(new QSGGuiThreadRenderLoop());
+                s_instance = new QSGGuiThreadRenderLoop();
                 break;
             }
         }
+
+        QObject::connect(QCoreApplication::instance(), SIGNAL(aboutToQuit()), s_instance, SLOT(cleanup()));
     }
-    return s_renderLoopInstance->data();
+    return s_instance;
 }
 
 void QSGRenderLoop::setInstance(QSGRenderLoop *instance)
 {
-    Q_ASSERT(s_renderLoopInstance->isNull());
-    s_renderLoopInstance->reset(instance);
+    Q_ASSERT(!s_instance);
+    s_instance = instance;
 }
 
 void QSGRenderLoop::handleContextCreationFailure(QQuickWindow *window,
diff --git a/src/quick/scenegraph/qsgrenderloop_p.h b/src/quick/scenegraph/qsgrenderloop_p.h
index 2418af4157..47b59992e2 100644
--- a/src/quick/scenegraph/qsgrenderloop_p.h
+++ b/src/quick/scenegraph/qsgrenderloop_p.h
@@ -78,6 +78,8 @@ public:
 
     virtual void releaseResources(QQuickWindow *window) = 0;
 
+    virtual QList<QQuickWindow *> windows() const = 0;
+
     // ### make this less of a singleton
     static QSGRenderLoop *instance();
     static void setInstance(QSGRenderLoop *instance);
@@ -91,6 +93,12 @@ Q_SIGNALS:
 
 protected:
     void handleContextCreationFailure(QQuickWindow *window, bool isEs);
+
+public Q_SLOTS:
+    void cleanup();
+
+private:
+    static QSGRenderLoop *s_instance;
 };
 
 QT_END_NAMESPACE
diff --git a/src/quick/scenegraph/qsgthreadedrenderloop.cpp b/src/quick/scenegraph/qsgthreadedrenderloop.cpp
index ba8c3a7fce..3dc2a50542 100644
--- a/src/quick/scenegraph/qsgthreadedrenderloop.cpp
+++ b/src/quick/scenegraph/qsgthreadedrenderloop.cpp
@@ -738,6 +738,14 @@ QSGThreadedRenderLoop::QSGThreadedRenderLoop()
     QSG_GUI_DEBUG((void *) 0, "QSGThreadedRenderLoop() created");
 }
 
+QList<QQuickWindow *> QSGThreadedRenderLoop::windows() const
+{
+    QList<QQuickWindow *> list;
+    foreach (const Window &w, m_windows)
+        list << w.window;
+    return list;
+}
+
 QSGRenderContext *QSGThreadedRenderLoop::createRenderContext(QSGContext *sg) const
 {
     return sg->createRenderContext();
diff --git a/src/quick/scenegraph/qsgthreadedrenderloop_p.h b/src/quick/scenegraph/qsgthreadedrenderloop_p.h
index e142f7f2c8..3997ef92b7 100644
--- a/src/quick/scenegraph/qsgthreadedrenderloop_p.h
+++ b/src/quick/scenegraph/qsgthreadedrenderloop_p.h
@@ -79,6 +79,8 @@ public:
 
     bool interleaveIncubation() const;
 
+    QList<QQuickWindow *> windows() const;
+
 public Q_SLOTS:
     void animationStarted();
     void animationStopped();
diff --git a/src/quick/scenegraph/qsgwindowsrenderloop.cpp b/src/quick/scenegraph/qsgwindowsrenderloop.cpp
index 913b737798..cbe9ea384b 100644
--- a/src/quick/scenegraph/qsgwindowsrenderloop.cpp
+++ b/src/quick/scenegraph/qsgwindowsrenderloop.cpp
@@ -111,6 +111,15 @@ QSGWindowsRenderLoop::~QSGWindowsRenderLoop()
     delete m_sg;
 }
 
+QList<QQuickWindow *> QSGWindowsRenderLoop::windows() const
+{
+    QList<QQuickWindow *> list;
+    foreach (const WindowData &w, m_windows)
+        list << w.window;
+    return list;
+}
+
+
 bool QSGWindowsRenderLoop::interleaveIncubation() const
 {
     return m_animationDriver->isRunning() && anyoneShowing();
diff --git a/src/quick/scenegraph/qsgwindowsrenderloop_p.h b/src/quick/scenegraph/qsgwindowsrenderloop_p.h
index e4ee688c9f..6c26e728da 100644
--- a/src/quick/scenegraph/qsgwindowsrenderloop_p.h
+++ b/src/quick/scenegraph/qsgwindowsrenderloop_p.h
@@ -86,6 +86,8 @@ public:
 
     bool interleaveIncubation() const;
 
+    QList<QQuickWindow *> windows() const;
+
 public Q_SLOTS:
     void started();
     void stopped();