Clean up shared resources immediately as the last context is destroyed.

By not waiting until deleteLater() kicks in it's easier to auto-test. We
can now add a test case for what happens when a shared resource is still
valid while the last context is destroyed.

Change-Id: I72963928e6a921e49ed59a79e2579b497ba37ccf
Reviewed-on: http://codereview.qt.nokia.com/3732
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Gunnar Sletta <gunnar.sletta@nokia.com>

diff --git a/src/gui/kernel/qopenglcontext.cpp b/src/gui/kernel/qopenglcontext.cpp
index e61c117..d5b34c2 100644 (file)
--- a/src/gui/kernel/qopenglcontext.cpp
+++ b/src/gui/kernel/qopenglcontext.cpp
@@ -360,17 +360,7 @@ QOpenGLContextGroup::QOpenGLContextGroup()
 QOpenGLContextGroup::~QOpenGLContextGroup()
 {
     Q_D(QOpenGLContextGroup);
-
-    QList<QOpenGLSharedResource *>::iterator it = d->m_sharedResources.begin();
-    QList<QOpenGLSharedResource *>::iterator end = d->m_sharedResources.end();
-
-    while (it != end) {
-        (*it)->invalidateResource();
-        (*it)->m_group = 0;
-        ++it;
-    }
-
-    qDeleteAll(d->m_pendingDeletion.begin(), d->m_pendingDeletion.end());
+    d->cleanup();
 }
 
 QList<QOpenGLContext *> QOpenGLContextGroup::shares() const
@@ -402,8 +392,27 @@ void QOpenGLContextGroupPrivate::removeContext(QOpenGLContext *ctx)
     if (ctx == m_context && !m_shares.isEmpty())
         m_context = m_shares.first();
 
-    if (!m_refs.deref())
+    if (!m_refs.deref()) {
+        cleanup();
         q->deleteLater();
+    }
+}
+
+void QOpenGLContextGroupPrivate::cleanup()
+{
+    QList<QOpenGLSharedResource *>::iterator it = m_sharedResources.begin();
+    QList<QOpenGLSharedResource *>::iterator end = m_sharedResources.end();
+
+    while (it != end) {
+        (*it)->invalidateResource();
+        (*it)->m_group = 0;
+        ++it;
+    }
+
+    m_sharedResources.clear();
+
+    qDeleteAll(m_pendingDeletion.begin(), m_pendingDeletion.end());
+    m_pendingDeletion.clear();
 }
 
 void QOpenGLContextGroupPrivate::deletePendingResources(QOpenGLContext *ctx)

diff --git a/src/gui/kernel/qopenglcontext_p.h b/src/gui/kernel/qopenglcontext_p.h
index 88738bc..bfe0f9d 100644 (file)
--- a/src/gui/kernel/qopenglcontext_p.h
+++ b/src/gui/kernel/qopenglcontext_p.h
@@ -124,6 +124,8 @@ public:
     void addContext(QOpenGLContext *ctx);
     void removeContext(QOpenGLContext *ctx);
 
+    void cleanup();
+
     void deletePendingResources(QOpenGLContext *ctx);
 
     QOpenGLContext *m_context;

diff --git a/tests/auto/qopengl/tst_qopengl.cpp b/tests/auto/qopengl/tst_qopengl.cpp
index 1df7985..173d1e4 100644 (file)
--- a/tests/auto/qopengl/tst_qopengl.cpp
+++ b/tests/auto/qopengl/tst_qopengl.cpp
@@ -146,7 +146,24 @@ void tst_QOpenGL::sharedResourceCleanup()
     QCOMPARE(tracker.freeResourceCalls, 1);
     QCOMPARE(tracker.destructorCalls, 1);
 
+    tracker.reset();
+
+    resource = new SharedResource(&tracker);
+
+    // this should cause invalidateResource() to be called
     delete ctx2;
+
+    QCOMPARE(tracker.invalidateResourceCalls, 1);
+    QCOMPARE(tracker.freeResourceCalls, 0);
+    QCOMPARE(tracker.destructorCalls, 0);
+
+    // should have no effect other than destroying the resource,
+    // as it has already been invalidated
+    resource->free();
+
+    QCOMPARE(tracker.invalidateResourceCalls, 1);
+    QCOMPARE(tracker.freeResourceCalls, 0);
+    QCOMPARE(tracker.destructorCalls, 1);
 }
 
 static bool fuzzyComparePixels(const QRgb testPixel, const QRgb refPixel, const char* file, int line, int x = -1, int y = -1)