Fix cleanup of non-threaded render loops.

They would unconditionally call cleanupNodesOnShutdown on hide(), but
QQuickWindow::sceneGraphInvalidated would only be emitted if this was
the last window being hidden, leading to an inconsistent state in the
application.

Since the non-threaded render loops do not support releasing resources
(there is one OpenGL context and one QSGRenderContext shared between
all windows) we delay cleanup until the window is destroyed.

This change also make the render loops track the windows until they
are destroyed, similar to what the threaded one does. The purpose of
this is to, in the case of dangling windows, only trigger invalidation
of the scene graph when the last QQuickWindow is destroyed through
QSGRenderLoop::cleanup().

Task-number: QTBUG-41210
Change-Id: I7e12a4f726ebb3e7935c822b6046abb3590c583a
Reviewed-by: Ulf Hermann <ulf.hermann@digia.com>
Reviewed-by: Laszlo Agocs <laszlo.agocs@digia.com>

3 files changed, 42 insertions, 61 deletions

diff --git a/src/quick/scenegraph/qsgrenderloop.cpp b/src/quick/scenegraph/qsgrenderloop.cpp
index 720d9a6fd1..cd92d12d43 100644
--- a/src/quick/scenegraph/qsgrenderloop.cpp
+++ b/src/quick/scenegraph/qsgrenderloop.cpp
@@ -265,37 +265,24 @@ void QSGGuiThreadRenderLoop::show(QQuickWindow *window)
 
 void QSGGuiThreadRenderLoop::hide(QQuickWindow *window)
 {
-    if (!m_windows.contains(window))
-        return;
-
-    m_windows.remove(window);
     QQuickWindowPrivate *cd = QQuickWindowPrivate::get(window);
-    if (gl)
-        gl->makeCurrent(window);
     cd->fireAboutToStop();
-    cd->cleanupNodesOnShutdown();
-
-    if (m_windows.size() == 0) {
-        if (!cd->persistentSceneGraph) {
-            rc->invalidate();
-            QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
-            if (!cd->persistentGLContext) {
-                delete gl;
-                gl = 0;
-            }
-        }
-    }
 }
 
 void QSGGuiThreadRenderLoop::windowDestroyed(QQuickWindow *window)
 {
+    m_windows.remove(window);
     hide(window);
+    QQuickWindowPrivate *d = QQuickWindowPrivate::get(window);
+    if (gl)
+        gl->makeCurrent(window);
+    d->cleanupNodesOnShutdown();
     if (m_windows.size() == 0) {
         rc->invalidate();
         QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
         delete gl;
         gl = 0;
-    } else if (window == gl->surface()) {
+    } else if (gl && window == gl->surface()) {
         gl->doneCurrent();
     }
 }
diff --git a/src/quick/scenegraph/qsgwindowsrenderloop.cpp b/src/quick/scenegraph/qsgwindowsrenderloop.cpp
index de1160064d..10eba74607 100644
--- a/src/quick/scenegraph/qsgwindowsrenderloop.cpp
+++ b/src/quick/scenegraph/qsgwindowsrenderloop.cpp
@@ -185,14 +185,6 @@ void QSGWindowsRenderLoop::show(QQuickWindow *window)
 void QSGWindowsRenderLoop::hide(QQuickWindow *window)
 {
     RLDEBUG("hide");
-
-    for (int i=0; i<m_windows.size(); ++i) {
-        if (m_windows.at(i).window == window) {
-            m_windows.removeAt(i);
-            break;
-        }
-    }
-
     // The expose event is queued while hide is sent synchronously, so
     // the value might not be updated yet. (plus that the windows plugin
     // sends exposed=true when it goes to hidden, so it is doubly broken)
@@ -200,47 +192,41 @@ void QSGWindowsRenderLoop::hide(QQuickWindow *window)
     // anyoneShowing will report the right value.
     if (window->isExposed())
         handleObscurity();
-
     if (!m_gl)
         return;
-
-    QQuickWindowPrivate *cd = QQuickWindowPrivate::get(window);
-    m_gl->makeCurrent(window);
-    cd->fireAboutToStop();
-    cd->cleanupNodesOnShutdown();
-
-    // If this is the last tracked window, check for persistent SG and GL and
-    // potentially clean up.
-    if (m_windows.size() == 0) {
-        if (!cd->persistentSceneGraph) {
-            QQuickWindowPrivate::get(window)->context->invalidate();
-            QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
-            if (!cd->persistentGLContext) {
-                delete m_gl;
-                m_gl = 0;
-            }
-        }
-    }
+    QQuickWindowPrivate::get(window)->fireAboutToStop();
 }
 
 void QSGWindowsRenderLoop::windowDestroyed(QQuickWindow *window)
 {
     RLDEBUG("windowDestroyed");
+    for (int i=0; i<m_windows.size(); ++i) {
+        if (m_windows.at(i).window == window) {
+            m_windows.removeAt(i);
+            break;
+        }
+    }
+
     hide(window);
 
-    // If this is the last tracked window, clean up SG and GL.
+    QQuickWindowPrivate *d = QQuickWindowPrivate::get(window);
+    if (m_gl)
+        m_gl->makeCurrent(window);
+    d->cleanupNodesOnShutdown();
     if (m_windows.size() == 0) {
-        QQuickWindowPrivate::get(window)->context->invalidate();
+        d->context->invalidate();
         QCoreApplication::sendPostedEvents(0, QEvent::DeferredDelete);
         delete m_gl;
         m_gl = 0;
+    } else if (m_gl) {
+        m_gl->doneCurrent();
     }
 }
 
 bool QSGWindowsRenderLoop::anyoneShowing() const
 {
     foreach (const WindowData &wd, m_windows)
-        if (wd.window->isExposed() && wd.window->size().isValid())
+        if (wd.window->isVisible() && wd.window->isExposed() && wd.window->size().isValid())
             return true;
     return false;
 }
@@ -251,7 +237,7 @@ void QSGWindowsRenderLoop::exposureChanged(QQuickWindow *window)
     if (windowData(window) == 0)
         return;
 
-    if (window->isExposed()) {
+    if (window->isExposed() && window->isVisible()) {
 
         // Stop non-visual animation timer as we now have a window rendering
         if (m_animationTimer && anyoneShowing()) {
diff --git a/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp b/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp
index aecd28f44b..dfb1ac5bfe 100644
--- a/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp
+++ b/tests/auto/quick/qquickwindow/tst_qquickwindow.cpp
@@ -1188,6 +1188,7 @@ void tst_qquickwindow::headless()
 
     QVERIFY(QTest::qWaitForWindowExposed(window));
     QVERIFY(window->isVisible());
+    bool threaded = window->openglContext()->thread() != QThread::currentThread();
 
     QSignalSpy initialized(window, SIGNAL(sceneGraphInitialized()));
     QSignalSpy invalidated(window, SIGNAL(sceneGraphInvalidated()));
@@ -1202,8 +1203,10 @@ void tst_qquickwindow::headless()
     window->hide();
     window->releaseResources();
 
-    QTRY_COMPARE(invalidated.size(), 1);
-    QVERIFY(window->openglContext() == 0);
+    if (threaded) {
+        QTRY_COMPARE(invalidated.size(), 1);
+        QVERIFY(window->openglContext() == 0);
+    }
 
     // Destroy the native windowing system buffers
     window->destroy();
@@ -1213,7 +1216,8 @@ void tst_qquickwindow::headless()
     window->show();
     QVERIFY(QTest::qWaitForWindowExposed(window));
 
-    QTRY_COMPARE(initialized.size(), 1);
+    if (threaded)
+        QTRY_COMPARE(initialized.size(), 1);
     QVERIFY(window->openglContext() != 0);
 
     // Verify that the visual output is the same
@@ -1536,6 +1540,7 @@ void tst_qquickwindow::hideThenDelete()
 
     QSignalSpy *openglDestroyed = 0;
     QSignalSpy *sgInvalidated = 0;
+    bool threaded = false;
 
     {
         QQuickWindow window;
@@ -1548,6 +1553,7 @@ void tst_qquickwindow::hideThenDelete()
         window.show();
 
         QTest::qWaitForWindowExposed(&window);
+        threaded = window.openglContext()->thread() != QThread::currentThread();
 
         openglDestroyed = new QSignalSpy(window.openglContext(), SIGNAL(aboutToBeDestroyed()));
         sgInvalidated = new QSignalSpy(&window, SIGNAL(sceneGraphInvalidated()));
@@ -1556,15 +1562,17 @@ void tst_qquickwindow::hideThenDelete()
 
         QTRY_VERIFY(!window.isExposed());
 
-        if (!persistentSG) {
-            QVERIFY(sgInvalidated->size() > 0);
-            if (!persistentGL)
-                QVERIFY(openglDestroyed->size() > 0);
-            else
+        if (threaded) {
+            if (!persistentSG) {
+                QVERIFY(sgInvalidated->size() > 0);
+                if (!persistentGL)
+                    QVERIFY(openglDestroyed->size() > 0);
+                else
+                    QVERIFY(openglDestroyed->size() == 0);
+            } else {
+                QVERIFY(sgInvalidated->size() == 0);
                 QVERIFY(openglDestroyed->size() == 0);
-        } else {
-            QVERIFY(sgInvalidated->size() == 0);
-            QVERIFY(openglDestroyed->size() == 0);
+            }
         }
     }