From 38922774119817f4bf1595b9651f914e5c3d9f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Trond=20Kjern=C3=A5sen?= Date: Tue, 22 Jun 2010 12:51:00 +0200 Subject: 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. --- src/gui/image/qpixmap.cpp | 1 + src/gui/painting/qpainter.cpp | 11 ++- .../gl2paintengineex/qglengineshadermanager.cpp | 94 +++++++++++++++++----- .../gl2paintengineex/qglengineshadermanager_p.h | 13 ++- src/opengl/qgl.cpp | 12 ++- src/opengl/qgl_p.h | 3 + 6 files changed, 98 insertions(+), 36 deletions(-) diff --git a/src/gui/image/qpixmap.cpp b/src/gui/image/qpixmap.cpp index 20e4b50..e630e80 100644 --- a/src/gui/image/qpixmap.cpp +++ b/src/gui/image/qpixmap.cpp @@ -94,6 +94,7 @@ Q_GUI_EXPORT qint64 qt_pixmap_id(const QPixmap &pixmap) static bool qt_pixmap_thread_test() { + return true; if (!qApp) { qFatal("QPixmap: Must construct a QApplication before a QPaintDevice"); return false; diff --git a/src/gui/painting/qpainter.cpp b/src/gui/painting/qpainter.cpp index 312cc95..ec2aca4 100644 --- a/src/gui/painting/qpainter.cpp +++ b/src/gui/painting/qpainter.cpp @@ -5252,7 +5252,8 @@ void QPainter::drawPixmap(const QPointF &p, const QPixmap &pm) return; #ifndef QT_NO_DEBUG - qt_painter_thread_test(d->device->devType(), "drawPixmap()"); + if (d->engine->type() != QPaintEngine::OpenGL2) + qt_painter_thread_test(d->device->devType(), "drawPixmap()"); #endif if (d->extended) { @@ -5322,7 +5323,8 @@ void QPainter::drawPixmap(const QRectF &r, const QPixmap &pm, const QRectF &sr) if (!d->engine || pm.isNull()) return; #ifndef QT_NO_DEBUG - qt_painter_thread_test(d->device->devType(), "drawPixmap()"); + if (d->engine->type() != QPaintEngine::OpenGL2) + qt_painter_thread_test(d->device->devType(), "drawPixmap()"); #endif qreal x = r.x(); @@ -5926,7 +5928,7 @@ void QPainter::drawStaticText(const QPointF &topLeftPosition, const QStaticText // Recreate the layout of the static text because the matrix or font has changed if (staticTextNeedsReinit) - staticText_d->init(); + staticText_d->init(); if (transformedPosition != staticText_d->position) { // Translate to actual position QFixed fx = QFixed::fromReal(transformedPosition.x()); @@ -6666,7 +6668,8 @@ void QPainter::drawTiledPixmap(const QRectF &r, const QPixmap &pixmap, const QPo return; #ifndef QT_NO_DEBUG - qt_painter_thread_test(d->device->devType(), "drawTiledPixmap()"); + if (d->engine->type() != QPaintEngine::OpenGL2) + qt_painter_thread_test(d->device->devType(), "drawTiledPixmap()"); #endif qreal sw = pixmap.width(); 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(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 m_storage; +}; + +Q_GLOBAL_STATIC(QGLThreadShaders, qt_shared_shaders); QGLEngineSharedShaders *QGLEngineSharedShaders::shadersForContext(const QGLContext *context) { - QGLEngineSharedShaders *p = reinterpret_cast(qt_shared_shaders()->value(context)); - if (!p) { + QGLEngineSharedShaders *shaders = reinterpret_cast(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::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 cachedPrograms; + QList 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 +#include 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 <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 -- cgit v0.12