More work on GL threading.

Removed QObject inheritance from QGLEngineSharedShaders and
made it thread-local, so that paintengines in different threads
can use programs without clashing.

Lifted some restrictions on QPixmap so that they may be used in
threads when the GL2 engine is active. Made the QGLContextGroup
a part of the pixmap and image cache keys.

4 files changed, 90 insertions, 32 deletions

diff --git a/src/opengl/gl2paintengineex/qglengineshadermanager.cpp b/src/opengl/gl2paintengineex/qglengineshadermanager.cpp
index 40b3641..5330706 100644
--- a/src/opengl/gl2paintengineex/qglengineshadermanager.cpp
+++ b/src/opengl/gl2paintengineex/qglengineshadermanager.cpp
@@ -51,19 +51,61 @@ QT_BEGIN_NAMESPACE
 
 static void qt_shared_shaders_free(void *data)
 {
+    qDebug() << "qt_shared_shaders_free() for thread:" << hex << QThread::currentThread();
     delete reinterpret_cast<QGLEngineSharedShaders *>(data);
 }
 
-Q_GLOBAL_STATIC_WITH_ARGS(QGLContextResource, qt_shared_shaders, (qt_shared_shaders_free))
+
+class QGLThreadLocalShaders {
+public:
+    QGLThreadLocalShaders(QGLContextResource *resource) : m_resource(resource), m_shaders(0) {}
+    ~QGLThreadLocalShaders() {
+        if (m_shaders) {
+            qDebug() << "~QGLThreadLocalShaders() for thread:" << hex << QThread::currentThread();
+            m_shaders->cleanupBeforeDestruction();
+        }
+    }
+    QGLContextResource *m_resource;
+    QGLEngineSharedShaders *m_shaders;
+};
+
+class QGLThreadShaders
+{
+public:
+    QGLContextResource *shadersForThread() {
+        QGLThreadLocalShaders *shaders = m_storage.localData();
+        if (!shaders) {
+            // The QGLContextResource is needed because the shaders need to be cleaned up
+            // when the context is destroyed, not just when a thread exits
+            QGLContextResource *resource = new QGLContextResource(qt_shared_shaders_free);
+            qDebug() << "new local resource for thread:" << hex << QThread::currentThread() << resource;
+            shaders = new QGLThreadLocalShaders(resource);
+            m_storage.setLocalData(shaders);
+        }
+        return shaders->m_resource;
+    }
+
+    void setShadersForThread(const QGLContext *context, QGLEngineSharedShaders *sharedShaders) {
+        shadersForThread()->insert(context, sharedShaders);
+        m_storage.localData()->m_shaders = sharedShaders;
+        qDebug() << "  -> context:" << context;
+    }
+
+private:
+    QThreadStorage<QGLThreadLocalShaders *> m_storage;
+};
+
+Q_GLOBAL_STATIC(QGLThreadShaders, qt_shared_shaders);
 
 QGLEngineSharedShaders *QGLEngineSharedShaders::shadersForContext(const QGLContext *context)
 {
-    QGLEngineSharedShaders *p = reinterpret_cast<QGLEngineSharedShaders *>(qt_shared_shaders()->value(context));
-    if (!p) {
+    QGLEngineSharedShaders *shaders = reinterpret_cast<QGLEngineSharedShaders *>(qt_shared_shaders()->shadersForThread()->value(context));
+    if (!shaders) {
         QGLShareContextScope scope(context);
-        qt_shared_shaders()->insert(context, p = new QGLEngineSharedShaders(context));
+        shaders = new QGLEngineSharedShaders(context);
+        qt_shared_shaders()->setShadersForThread(context, shaders);
     }
-    return p;
+    return shaders;
 }
 
 const char* QGLEngineSharedShaders::qShaderSnippets[] = {
@@ -170,18 +212,20 @@ QGLEngineSharedShaders::QGLEngineSharedShaders(const QGLContext* context)
     source.clear();
     source.append(qShaderSnippets[MainVertexShader]);
     source.append(qShaderSnippets[PositionOnlyVertexShader]);
-    vertexShader = new QGLShader(QGLShader::Vertex, context, this);
+    vertexShader = new QGLShader(QGLShader::Vertex, context, 0);
+    shaders.append(vertexShader);
     if (!vertexShader->compileSourceCode(source))
         qWarning("Vertex shader for simpleShaderProg (MainVertexShader & PositionOnlyVertexShader) failed to compile");
 
     source.clear();
     source.append(qShaderSnippets[MainFragmentShader]);
     source.append(qShaderSnippets[ShockingPinkSrcFragmentShader]);
-    fragShader = new QGLShader(QGLShader::Fragment, context, this);
+    fragShader = new QGLShader(QGLShader::Fragment, context, 0);
+    shaders.append(fragShader);
     if (!fragShader->compileSourceCode(source))
         qWarning("Fragment shader for simpleShaderProg (MainFragmentShader & ShockingPinkSrcFragmentShader) failed to compile");
 
-    simpleShaderProg = new QGLShaderProgram(context, this);
+    simpleShaderProg = new QGLShaderProgram(context, 0);
     simpleShaderProg->addShader(vertexShader);
     simpleShaderProg->addShader(fragShader);
     simpleShaderProg->bindAttributeLocation("vertexCoordsArray", QT_VERTEX_COORDS_ATTR);
@@ -198,18 +242,20 @@ QGLEngineSharedShaders::QGLEngineSharedShaders(const QGLContext* context)
     source.clear();
     source.append(qShaderSnippets[MainWithTexCoordsVertexShader]);
     source.append(qShaderSnippets[UntransformedPositionVertexShader]);
-    vertexShader = new QGLShader(QGLShader::Vertex, context, this);
+    vertexShader = new QGLShader(QGLShader::Vertex, context, 0);
+    shaders.append(vertexShader);
     if (!vertexShader->compileSourceCode(source))
         qWarning("Vertex shader for blitShaderProg (MainWithTexCoordsVertexShader & UntransformedPositionVertexShader) failed to compile");
 
     source.clear();
     source.append(qShaderSnippets[MainFragmentShader]);
     source.append(qShaderSnippets[ImageSrcFragmentShader]);
-    fragShader = new QGLShader(QGLShader::Fragment, context, this);
+    fragShader = new QGLShader(QGLShader::Fragment, context, 0);
+    shaders.append(fragShader);
     if (!fragShader->compileSourceCode(source))
         qWarning("Fragment shader for blitShaderProg (MainFragmentShader & ImageSrcFragmentShader) failed to compile");
 
-    blitShaderProg = new QGLShaderProgram(context, this);
+    blitShaderProg = new QGLShaderProgram(context, 0);
     blitShaderProg->addShader(vertexShader);
     blitShaderProg->addShader(fragShader);
     blitShaderProg->bindAttributeLocation("textureCoordArray", QT_TEXTURE_COORDS_ATTR);
@@ -224,9 +270,20 @@ QGLEngineSharedShaders::QGLEngineSharedShaders(const QGLContext* context)
 
 QGLEngineSharedShaders::~QGLEngineSharedShaders()
 {
-    QList<QGLEngineShaderProg*>::iterator itr;
-    for (itr = cachedPrograms.begin(); itr != cachedPrograms.end(); ++itr)
-        delete *itr;
+    cleanupBeforeDestruction();
+}
+
+
+// This might be called both when a thread exits, or when the a
+// context is destroyed
+
+void QGLEngineSharedShaders::cleanupBeforeDestruction()
+{
+    qDeleteAll(shaders);
+    shaders.clear();
+
+    qDeleteAll(cachedPrograms);
+    cachedPrograms.clear();
 
     if (blitShaderProg) {
         delete blitShaderProg;
@@ -276,7 +333,8 @@ QGLEngineShaderProg *QGLEngineSharedShaders::findProgramInCache(const QGLEngineS
             source.append(qShaderSnippets[prog.compositionFragShader]);
         if (prog.maskFragShader)
             source.append(qShaderSnippets[prog.maskFragShader]);
-        fragShader = new QGLShader(QGLShader::Fragment, ctxGuard.context(), this);
+        fragShader = new QGLShader(QGLShader::Fragment, ctxGuard.context(), 0);
+        shaders.append(fragShader);
         QByteArray description;
 #if defined(QT_DEBUG)
         // Name the shader for easier debugging
@@ -302,7 +360,8 @@ QGLEngineShaderProg *QGLEngineSharedShaders::findProgramInCache(const QGLEngineS
         source.clear();
         source.append(qShaderSnippets[prog.mainVertexShader]);
         source.append(qShaderSnippets[prog.positionVertexShader]);
-        vertexShader = new QGLShader(QGLShader::Vertex, ctxGuard.context(), this);
+        vertexShader = new QGLShader(QGLShader::Vertex, ctxGuard.context(), 0);
+        shaders.append(vertexShader);
 #if defined(QT_DEBUG)
         // Name the shader for easier debugging
         description.clear();
@@ -320,7 +379,7 @@ QGLEngineShaderProg *QGLEngineSharedShaders::findProgramInCache(const QGLEngineS
         newProg = new QGLEngineShaderProg(prog);
 
         // If the shader program's not found in the cache, create it now.
-        newProg->program = new QGLShaderProgram(ctxGuard.context(), this);
+        newProg->program = new QGLShaderProgram(ctxGuard.context(), 0);
         newProg->program->addShader(vertexShader);
         newProg->program->addShader(fragShader);
 
@@ -413,7 +472,6 @@ QGLEngineShaderManager::QGLEngineShaderManager(QGLContext* context)
       currentShaderProg(0)
 {
     sharedShaders = QGLEngineSharedShaders::shadersForContext(context);
-    connect(sharedShaders, SIGNAL(shaderProgNeedsChanging()), this, SLOT(shaderProgNeedsChangingSlot()));
 }
 
 QGLEngineShaderManager::~QGLEngineShaderManager()
diff --git a/src/opengl/gl2paintengineex/qglengineshadermanager_p.h b/src/opengl/gl2paintengineex/qglengineshadermanager_p.h
index 06b96ae..8d3697b 100644
--- a/src/opengl/gl2paintengineex/qglengineshadermanager_p.h
+++ b/src/opengl/gl2paintengineex/qglengineshadermanager_p.h
@@ -259,9 +259,9 @@ static const GLuint QT_PMV_MATRIX_3_ATTR = 5;
 
 class QGLEngineShaderProg;
 
-class QGLEngineSharedShaders : public QObject
+class QGLEngineSharedShaders
 {
-    Q_OBJECT
+    Q_GADGET
 public:
 
     enum SnippetName {
@@ -364,14 +364,16 @@ public:
     // full.
     void cleanupCustomStage(QGLCustomShaderStage* stage);
 
-signals:
-    void shaderProgNeedsChanging();
+    // this is needed so that threads can get a foot in and have a chance to
+    // clean up the shaders they've created before the thread exits
+    void cleanupBeforeDestruction();
 
 private:
     QGLSharedResourceGuard ctxGuard;
     QGLShaderProgram *blitShaderProg;
     QGLShaderProgram *simpleShaderProg;
     QList<QGLEngineShaderProg*> cachedPrograms;
+    QList<QGLShader *> shaders;
 
     static const char* qShaderSnippets[TotalSnippetCount];
 };
@@ -492,9 +494,6 @@ public:
 
     QGLEngineSharedShaders* sharedShaders;
 
-private slots:
-    void shaderProgNeedsChangingSlot() { shaderProgNeedsChanging = true; }
-
 private:
     QGLContext*     ctx;
     bool            shaderProgNeedsChanging;
diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp
index b875fd1..fa6dd37 100644
--- a/src/opengl/qgl.cpp
+++ b/src/opengl/qgl.cpp
@@ -2236,7 +2236,7 @@ QImage QGLContextPrivate::convertToGLFormat(const QImage &image, bool force_prem
 QGLTexture *QGLContextPrivate::bindTexture(const QImage &image, GLenum target, GLint format,
                                            QGLContext::BindOptions options)
 {
-    const qint64 key = image.cacheKey();
+    const qint64 key = image.cacheKey() | (qint64) group;
     QGLTexture *texture = textureCacheLookup(key, target);
     if (texture) {
         glBindTexture(target, texture->id);
@@ -2509,7 +2509,7 @@ QGLTexture *QGLContextPrivate::bindTexture(const QPixmap &pixmap, GLenum target,
     Q_UNUSED(q);
 #endif
 
-    const qint64 key = pixmap.cacheKey();
+    const qint64 key = pixmap.cacheKey() | (qint64) group;
     QGLTexture *texture = textureCacheLookup(key, target);
     if (texture) {
         glBindTexture(target, texture->id);
@@ -2532,6 +2532,7 @@ QGLTexture *QGLContextPrivate::bindTexture(const QPixmap &pixmap, GLenum target,
     }
 #endif
 
+    printf("  -> bindTexture key: %llx\n", key);
     if (!texture) {
         QImage image = pixmap.toImage();
         // If the system depth is 16 and the pixmap doesn't have an alpha channel
@@ -4061,11 +4062,7 @@ bool QGLWidget::event(QEvent *e)
 #if defined(Q_WS_X11)
     // prevents X errors on some systems, where we get a flush to a
     // hidden widget
-    if (e->type() == QEvent::Hide) {
-        makeCurrent();
-        glFinish();
-        doneCurrent();
-    } else if (e->type() == QEvent::ParentChange) {
+    if (e->type() == QEvent::ParentChange) {
         // if we've reparented a window that has the current context
         // bound, we need to rebind that context to the new window id
         if (d->glcx == QGLContext::currentContext())
@@ -5373,6 +5370,7 @@ void *QGLContextResource::value(const QGLContext *key)
 
 void QGLContextResource::cleanup(const QGLContext *ctx, void *value)
 {
+    qDebug() << "QGLContextResource::cleanup() this:" << hex << this << "thread:" << QThread::currentThread();
     QGLShareContextScope scope(ctx);
     free(value);
     active.deref();
diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h
index 16c225f..9b862eb 100644
--- a/src/opengl/qgl_p.h
+++ b/src/opengl/qgl_p.h
@@ -473,6 +473,8 @@ private:
     QGLContext *m_ctx;
 };
 
+#include <qdebug.h>
+#include <qthread.h>
 class QGLTexture {
 public:
     QGLTexture(QGLContext *ctx = 0, GLuint tx_id = 0, GLenum tx_target = GL_TEXTURE_2D,
@@ -489,6 +491,7 @@ public:
     ~QGLTexture() {
         if (options & QGLContext::MemoryManagedBindOption) {
             Q_ASSERT(context);
+            qDebug()<< "~QGLTexture: thread:" << hex <<QThread::currentThread() << "(main thread:" << QApplication::instance()->thread() << ")" << "context:" << context << "current context:" << QGLContext::currentContext();
             QGLShareContextScope scope(context);
 #if defined(Q_WS_X11)
             // Although glXReleaseTexImage is a glX call, it must be called while there