From 80a2ebd93346f885d904e4e1020e112cc635dbaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Trond=20Kjern=C3=A5sen?= Date: Tue, 22 Jun 2010 13:02:54 +0200 Subject: Redesigned how GL resource management works. The usage of QGLContextResource has changed: You're now have to subclass QGLContextResource and reimplement the freeResource() function and add your cleanup code there instead of using a plain callback function. It's now also possible to delete a QGLContextResource *before* the QGLContextGroup it refers to is destroyed, as the resource will remove itself from the context groups it's a member of. The QGLTextureGlyphCache is no longer a QObject, and it no longer depends on the aboutToDestroyContext() signal. That concept doesn't work in a threaded environment, as it relies on an eventloop to dispatch the signal to the thread. It's common to *not* have an eventloop running in a thread, which means the signal might never be delivered. QGLTextureGlyphCache now inherits from QGLContextResource, and gets cleaned up correctly when the group context is destroyed. Note that up until now the glyph cache has never been shared among sharing contexts for the GL 2 engine. Made the gradient and pixmap blur caches use the new QGLContextResource scheme. Added a template that wraps the common init code for paintEngine() function implementations for QGLWidget, QGLPixelBuffer and QGLFramebufferObject. Fixed a bug in QFontCache where the font caches weren't cleared when a thread other than the main thread exited (backported to 4.6.3), which caused resource leaks. --- .../gl2paintengineex/qglengineshadermanager.cpp | 80 ++++++++-------------- .../gl2paintengineex/qglengineshadermanager_p.h | 4 -- src/opengl/gl2paintengineex/qglgradientcache.cpp | 12 ++-- .../gl2paintengineex/qpaintengineex_opengl2.cpp | 1 + .../gl2paintengineex/qtextureglyphcache_gl.cpp | 47 +++---------- .../gl2paintengineex/qtextureglyphcache_gl_p.h | 31 +++------ src/opengl/qgl.cpp | 61 +++++------------ src/opengl/qgl_p.h | 32 +++++++-- src/opengl/qglframebufferobject.cpp | 8 +-- src/opengl/qglpixelbuffer.cpp | 13 ++-- src/opengl/qglpixmapfilter.cpp | 12 ++-- 11 files changed, 116 insertions(+), 185 deletions(-) diff --git a/src/opengl/gl2paintengineex/qglengineshadermanager.cpp b/src/opengl/gl2paintengineex/qglengineshadermanager.cpp index 0ba324d..888bac9 100644 --- a/src/opengl/gl2paintengineex/qglengineshadermanager.cpp +++ b/src/opengl/gl2paintengineex/qglengineshadermanager.cpp @@ -48,65 +48,54 @@ QT_BEGIN_NAMESPACE +class QGLSharedShaderResource : public QGLContextResource +{ +public: + ~QGLSharedShaderResource() { + qDebug() << "~QGLSharedShaderResource cleaned up" << hex << this; + } -// Handling the cleanup and creation of the thread-local shader cache -// differes from other QGLContextResources. The cache needs to be -// cleaned up when the thread exits, or when its group context is -// destroyed. - -static void qt_gl_free_shared_shaders(void *value); + void freeResource(void *value) + { + qDebug() << "Context destroyed:"; + qDebug() << " -> deleting shader cache" << hex << value; + delete reinterpret_cast(value); + } +}; class QGLThreadLocalShaders { public: - QGLThreadLocalShaders(const QGLContext *context, QGLEngineSharedShaders *shaders) - : m_context(context), m_shaders(shaders) - { - m_resource = new QGLContextResource(qt_gl_free_shared_shaders); - m_resource->insert(context, this); + QGLEngineSharedShaders *shadersForContext(const QGLContext *context) { + QGLEngineSharedShaders *shaders = reinterpret_cast(m_resource.value(context)); + if (!shaders) { + QGLShareContextScope scope(context); + shaders = new QGLEngineSharedShaders(context); + qDebug() << " -> new shader cache for context:" << hex << context << "cache:" << shaders; + m_resource.insert(context, shaders); + } + return shaders; } + ~QGLThreadLocalShaders() { qDebug() << "Thread exit:"; qDebug() << "~QGLThreadLocalShaders() for thread:" << hex << QThread::currentThread(); - qDebug() << " -> deleting shader cache:" << hex << m_shaders; - delete m_shaders; - // remove the resource from the group's resource list, so that - // the cleanup function is not called when the context is destroyed - m_resource->remove(m_context); - delete m_resource; } - const QGLContext *m_context; - QGLEngineSharedShaders *m_shaders; - QGLContextResource *m_resource; -}; -static void qt_gl_free_shared_shaders(void *value) -{ - QGLThreadLocalShaders *local = reinterpret_cast(value); - qDebug() << "Context destroyed:"; - qDebug() << " -> deleting shader cache" << hex << local->m_shaders; - delete local->m_shaders; - local->m_shaders = 0; - - // this function is called when the context group for the context - // we have ref'ed is deleted - that means our context ptr is no - // longer valid - local->m_context = 0; -} +private: + QGLSharedShaderResource m_resource; +}; class QGLShaderStorage { public: QGLEngineSharedShaders *shadersForThread(const QGLContext *context) { - QGLThreadLocalShaders *shaders = m_storage.localData(); + QGLThreadLocalShaders *&shaders = m_storage.localData(); if (!shaders) { - qDebug() << "New shader cache for thread:" << hex << QThread::currentThread(); - QGLShareContextScope scope(context); - shaders = new QGLThreadLocalShaders(context, new QGLEngineSharedShaders(context)); - qDebug() << " -> context:" << context; - m_storage.setLocalData(shaders); + qDebug() << "New thread storage for:" << hex << QThread::currentThread(); + shaders = new QGLThreadLocalShaders; } - return shaders->m_shaders; + return shaders->shadersForContext(context); } private: @@ -282,15 +271,6 @@ QGLEngineSharedShaders::QGLEngineSharedShaders(const QGLContext* context) QGLEngineSharedShaders::~QGLEngineSharedShaders() { - cleanupBeforeDestruction(); -} - - -// This might be called both when a thread exits, or when the a -// context is destroyed - -void QGLEngineSharedShaders::cleanupBeforeDestruction() -{ qDeleteAll(shaders); shaders.clear(); diff --git a/src/opengl/gl2paintengineex/qglengineshadermanager_p.h b/src/opengl/gl2paintengineex/qglengineshadermanager_p.h index 8d3697b..e5ababf 100644 --- a/src/opengl/gl2paintengineex/qglengineshadermanager_p.h +++ b/src/opengl/gl2paintengineex/qglengineshadermanager_p.h @@ -364,10 +364,6 @@ public: // full. void cleanupCustomStage(QGLCustomShaderStage* stage); - // 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; diff --git a/src/opengl/gl2paintengineex/qglgradientcache.cpp b/src/opengl/gl2paintengineex/qglgradientcache.cpp index a1495dd..a0a3f8f 100644 --- a/src/opengl/gl2paintengineex/qglgradientcache.cpp +++ b/src/opengl/gl2paintengineex/qglgradientcache.cpp @@ -46,12 +46,16 @@ QT_BEGIN_NAMESPACE -static void QGL2GradientCache_free(void *ptr) +class QGLGradientCacheResource : public QGLContextResource { - delete reinterpret_cast(ptr); -} +public: + void freeResource(void *value) + { + delete reinterpret_cast(value); + } +}; -Q_GLOBAL_STATIC_WITH_ARGS(QGLContextResource, qt_gradient_caches, (QGL2GradientCache_free)) +Q_GLOBAL_STATIC(QGLGradientCacheResource, qt_gradient_caches) QGL2GradientCache *QGL2GradientCache::cacheForContext(const QGLContext *context) { diff --git a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp index 634b315..260e797 100644 --- a/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp +++ b/src/opengl/gl2paintengineex/qpaintengineex_opengl2.cpp @@ -1457,6 +1457,7 @@ void QGL2PaintEngineExPrivate::drawCachedGlyphs(QFontEngineGlyphCache::Type glyp (QGLTextureGlyphCache *) staticTextItem->fontEngine->glyphCache(ctx, glyphType, QTransform()); if (!cache || cache->cacheType() != glyphType) { cache = new QGLTextureGlyphCache(ctx, glyphType, QTransform()); + cache->insert(ctx, cache); staticTextItem->fontEngine->setGlyphCache(ctx, cache); } diff --git a/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp b/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp index faf4563..001a09e 100644 --- a/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp +++ b/src/opengl/gl2paintengineex/qtextureglyphcache_gl.cpp @@ -54,45 +54,10 @@ extern Q_GUI_EXPORT bool qt_cleartype_enabled; QGLTextureGlyphCache::QGLTextureGlyphCache(const QGLContext *context, QFontEngineGlyphCache::Type type, const QTransform &matrix) : QImageTextureGlyphCache(type, matrix) - , ctx(0) - , pex(0) + , ctx(context) , m_width(0) , m_height(0) { - if (context != 0) - setContext(context); -} - -QGLTextureGlyphCache::~QGLTextureGlyphCache() -{ - cleanUpContext(); -} - -void QGLTextureGlyphCache::cleanUpContext() -{ - if (ctx) { - QGLShareContextScope scope(ctx); - - if (!ctx->d_ptr->workaround_brokenFBOReadBack && pex != 0) - glDeleteFramebuffers(1, &m_fbo); - - if (m_width || m_height) { - glDeleteTextures(1, &m_texture); - m_width = 0; - m_height = 0; - m_h = 0; - } - - ctx = 0; - } -} - -void QGLTextureGlyphCache::setContext(const QGLContext *context) -{ - cleanUpContext(); - - ctx = context; - // broken FBO readback is a bug in the SGX 1.3 and 1.4 drivers for the N900 where // copying between FBO's is broken if the texture is either GL_ALPHA or POT. The // workaround is to use a system-memory copy of the glyph cache for this device. @@ -101,8 +66,12 @@ void QGLTextureGlyphCache::setContext(const QGLContext *context) if (!ctx->d_ptr->workaround_brokenFBOReadBack && pex != 0) glGenFramebuffers(1, &m_fbo); - connect(QGLSignalProxy::instance(), SIGNAL(aboutToDestroyContext(const QGLContext*)), - SLOT(contextDestroyed(const QGLContext*))); + fprintf(stderr, "## QGLTextureGlyphCache(): ctx: %p - this: %p\n", ctx, this); +} + +QGLTextureGlyphCache::~QGLTextureGlyphCache() +{ + fprintf(stderr, "## ~QGLTextureGlyphCache(): context: %p - this: %p\n", ctx, this); } void QGLTextureGlyphCache::createTextureData(int width, int height) @@ -161,7 +130,7 @@ void QGLTextureGlyphCache::resizeTextureData(int width, int height) GLuint oldTexture = m_texture; createTextureData(width, height); - + if (pex == 0 || ctx->d_ptr->workaround_brokenFBOReadBack) { QImageTextureGlyphCache::resizeTextureData(width, height); Q_ASSERT(image().depth() == 8); diff --git a/src/opengl/gl2paintengineex/qtextureglyphcache_gl_p.h b/src/opengl/gl2paintengineex/qtextureglyphcache_gl_p.h index d291ac3..710a12d 100644 --- a/src/opengl/gl2paintengineex/qtextureglyphcache_gl_p.h +++ b/src/opengl/gl2paintengineex/qtextureglyphcache_gl_p.h @@ -62,9 +62,8 @@ QT_BEGIN_NAMESPACE class QGL2PaintEngineExPrivate; -class Q_OPENGL_EXPORT QGLTextureGlyphCache : public QObject, public QImageTextureGlyphCache +class Q_OPENGL_EXPORT QGLTextureGlyphCache : public QGLContextResource, public QImageTextureGlyphCache { - Q_OBJECT public: QGLTextureGlyphCache(const QGLContext *context, QFontEngineGlyphCache::Type type, const QTransform &matrix); ~QGLTextureGlyphCache(); @@ -82,33 +81,21 @@ public: inline void setPaintEnginePrivate(QGL2PaintEngineExPrivate *p) { pex = p; } - void setContext(const QGLContext *context); inline const QGLContext *context() const { return ctx; } + void freeResource(void *) { + qDebug() << "QGLTextureGlyphCache::freeResource():" << this << "ctx:" << ctx; + // At this point, the context group is made current, so it's safe to + // release resources without a makeCurrent() call + if (!ctx->d_ptr->workaround_brokenFBOReadBack) + glDeleteFramebuffers(1, &m_fbo); -public Q_SLOTS: - void contextDestroyed(const QGLContext *context) { - if (context == ctx) { - const QGLContext *nextCtx = qt_gl_transfer_context(ctx); - if (!nextCtx) { - // the context may not be current, so we cannot directly - // destroy the fbo and texture here, but since the context - // is about to be destroyed, the GL server will do the - // clean up for us anyway - m_fbo = 0; - m_texture = 0; - ctx = 0; - } else { - // since the context holding the texture is shared, and - // about to be destroyed, we have to transfer ownership - // of the texture to one of the share contexts - ctx = const_cast(nextCtx); - } - } + if (m_width || m_height) + glDeleteTextures(1, &m_texture); } private: diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp index 1223552..753a984 100644 --- a/src/opengl/qgl.cpp +++ b/src/opengl/qgl.cpp @@ -2028,6 +2028,7 @@ QGLContext::~QGLContext() // remove any textures cached in this context QGLTextureCache::instance()->removeContextTextures(this); + qDebug() << "~QGLContext(): about to clean up" << hex << this; d_ptr->group->cleanupResources(this); QGLSignalProxy::instance()->emitAboutToDestroyContext(this); @@ -2533,7 +2534,6 @@ 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 @@ -5029,41 +5029,12 @@ void QGLWidget::drawTexture(const QPointF &point, QMacCompatGLuint textureId, QM } #endif -#if !defined(QT_OPENGL_ES_1) -class QtGL2EngineStorage -{ -public: - QPaintEngine *engine() { - QPaintEngine *localEngine = storage.localData(); - if (!localEngine) { - localEngine = new QGL2PaintEngineEx; - storage.setLocalData(localEngine); - } - return localEngine; - } - -private: - QThreadStorage storage; -}; -Q_GLOBAL_STATIC(QtGL2EngineStorage, qt_gl_2_engine) +#ifndef QT_OPENGL_ES_1 +Q_GLOBAL_STATIC(QGLEngineThreadStorage, qt_gl_2_engine) #endif #ifndef QT_OPENGL_ES_2 -class QtGL1EngineStorage -{ -public: - QPaintEngine *engine() { - QPaintEngine *localEngine = storage.localData(); - if (!localEngine) { - localEngine = new QOpenGLPaintEngine; - storage.setLocalData(localEngine); - } - return localEngine; - } -private: - QThreadStorage storage; -}; -Q_GLOBAL_STATIC(QtGL1EngineStorage, qt_gl_engine) +Q_GLOBAL_STATIC(QGLEngineThreadStorage, qt_gl_engine) #endif Q_OPENGL_EXPORT QPaintEngine* qt_qgl_paint_engine() @@ -5339,13 +5310,17 @@ void QGLContextGroup::removeShare(const QGLContext *context) { group->m_shares.clear(); } -QGLContextResource::QGLContextResource(FreeFunc f) - : free(f), active(0) +QGLContextResource::QGLContextResource() + : active(0) { } QGLContextResource::~QGLContextResource() { + for (int i = 0; i < m_groups.size(); ++i) { + m_groups.at(i)->m_resources.remove(this); + active.deref(); + } #ifndef QT_NO_DEBUG if (active != 0) { qWarning("QtOpenGL: Resources are still available at program shutdown.\n" @@ -5360,6 +5335,7 @@ void QGLContextResource::insert(const QGLContext *key, void *value) QGLContextGroup *group = QGLContextPrivate::contextGroup(key); Q_ASSERT(!group->m_resources.contains(this)); group->m_resources.insert(this, value); + m_groups.append(group); active.ref(); } @@ -5371,23 +5347,18 @@ void *QGLContextResource::value(const QGLContext *key) void QGLContextResource::cleanup(const QGLContext *ctx, void *value) { - qDebug() << "QGLContextResource::cleanup() this:" << hex << this << "thread:" << QThread::currentThread(); + qDebug() << "QGLContextResource::cleanup() this:" << hex << this << "ctx:" << ctx << "thread:" << (void *)QThread::currentThread(); QGLShareContextScope scope(ctx); - free(value); + freeResource(value); active.deref(); -} -void QGLContextResource::remove(const QGLContext *ctx) -{ - if (ctx) { - QGLContextGroup *group = QGLContextPrivate::contextGroup(ctx); - group->m_resources.remove(this); - active.deref(); - } + QGLContextGroup *group = QGLContextPrivate::contextGroup(ctx); + m_groups.removeOne(group); } void QGLContextGroup::cleanupResources(const QGLContext *ctx) { + qDebug() << "QGLContextGroup::cleanupResources() for ctx:" << hex << ctx << "shares:" << m_shares.size() << "res:" << m_resources.size(); // If there are still shares, then no cleanup to be done yet. if (m_shares.size() > 1) return; diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h index d5083e9..1399359 100644 --- a/src/opengl/qgl_p.h +++ b/src/opengl/qgl_p.h @@ -229,6 +229,7 @@ public: static void addShare(const QGLContext *context, const QGLContext *share); static void removeShare(const QGLContext *context); + private: QGLContextGroup(const QGLContext *context); @@ -491,7 +492,6 @@ 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 @@ -615,19 +615,17 @@ inline GLenum qt_gl_preferredTextureTarget() class Q_OPENGL_EXPORT QGLContextResource { public: - typedef void (*FreeFunc)(void *); - QGLContextResource(FreeFunc f); - ~QGLContextResource(); + QGLContextResource(); + virtual ~QGLContextResource(); // Set resource 'value' for 'key' and all its shared contexts. void insert(const QGLContext *key, void *value); // Return resource for 'key' or a shared context. void *value(const QGLContext *key); // Cleanup 'value' in response to a context group being destroyed. void cleanup(const QGLContext *ctx, void *value); - // Remove this resource from the group's resource list. - void remove(const QGLContext *ctx); + virtual void freeResource(void *value) = 0; private: - FreeFunc free; + QList m_groups; QAtomicInt active; }; @@ -720,6 +718,26 @@ private: int gl_extensions_length; }; + +// this is a class that wraps a QThreadStorage object for storing +// thread local instances of the GL 1 and GL 2 paint engines + +template +class QGLEngineThreadStorage +{ +public: + QPaintEngine *engine() { + QPaintEngine *localEngine = storage.localData(); + if (!localEngine) { + localEngine = new T; + storage.setLocalData(localEngine); + } + return localEngine; + } + +private: + QThreadStorage storage; +}; QT_END_NAMESPACE #endif // QGL_P_H diff --git a/src/opengl/qglframebufferobject.cpp b/src/opengl/qglframebufferobject.cpp index deffc20..fe60e83 100644 --- a/src/opengl/qglframebufferobject.cpp +++ b/src/opengl/qglframebufferobject.cpp @@ -984,11 +984,11 @@ QImage QGLFramebufferObject::toImage() const } #if !defined(QT_OPENGL_ES_1) -Q_GLOBAL_STATIC(QGL2PaintEngineEx, qt_buffer_2_engine) +Q_GLOBAL_STATIC(QGLEngineThreadStorage, qt_buffer_2_engine) #endif #ifndef QT_OPENGL_ES_2 -Q_GLOBAL_STATIC(QOpenGLPaintEngine, qt_buffer_engine) +Q_GLOBAL_STATIC(QGLEngineThreadStorage, qt_buffer_engine) #endif /*! \reimp */ @@ -1002,7 +1002,7 @@ QPaintEngine *QGLFramebufferObject::paintEngine() const #if !defined (QT_OPENGL_ES_2) if (qt_gl_preferGL2Engine()) { #endif - QPaintEngine *engine = qt_buffer_2_engine(); + QPaintEngine *engine = qt_buffer_2_engine()->engine(); if (engine->isActive() && engine->paintDevice() != this) { d->engine = new QGL2PaintEngineEx; return d->engine; @@ -1014,7 +1014,7 @@ QPaintEngine *QGLFramebufferObject::paintEngine() const #endif #if !defined(QT_OPENGL_ES_2) - QPaintEngine *engine = qt_buffer_engine(); + QPaintEngine *engine = qt_buffer_engine()->engine(); if (engine->isActive() && engine->paintDevice() != this) { d->engine = new QOpenGLPaintEngine; return d->engine; diff --git a/src/opengl/qglpixelbuffer.cpp b/src/opengl/qglpixelbuffer.cpp index 9a8b243..e5aed44 100644 --- a/src/opengl/qglpixelbuffer.cpp +++ b/src/opengl/qglpixelbuffer.cpp @@ -388,25 +388,26 @@ bool QGLPixelBuffer::isValid() const } #if !defined(QT_OPENGL_ES_1) -Q_GLOBAL_STATIC(QGL2PaintEngineEx, qt_buffer_2_engine) +Q_GLOBAL_STATIC(QGLEngineThreadStorage, qt_buffer_2_engine) #endif #ifndef QT_OPENGL_ES_2 -Q_GLOBAL_STATIC(QOpenGLPaintEngine, qt_buffer_engine) +Q_GLOBAL_STATIC(QGLEngineThreadStorage, qt_buffer_engine) #endif /*! \reimp */ QPaintEngine *QGLPixelBuffer::paintEngine() const { + return qt_qgl_paint_engine(); #if defined(QT_OPENGL_ES_1) - return qt_buffer_engine(); + return qt_buffer_engine()->engine(); #elif defined(QT_OPENGL_ES_2) - return qt_buffer_2_engine(); + return qt_buffer_2_engine()->engine(); #else if (qt_gl_preferGL2Engine()) - return qt_buffer_2_engine(); + return qt_buffer_2_engine()->engine(); else - return qt_buffer_engine(); + return qt_buffer_engine()->engine(); #endif } diff --git a/src/opengl/qglpixmapfilter.cpp b/src/opengl/qglpixmapfilter.cpp index bfa5ef1..011869e 100644 --- a/src/opengl/qglpixmapfilter.cpp +++ b/src/opengl/qglpixmapfilter.cpp @@ -337,12 +337,16 @@ private: QList QGLBlurTextureCache::blurTextureCaches; -static void QGLBlurTextureCache_free(void *ptr) +class QGLBlurCacheResource : public QGLContextResource { - delete reinterpret_cast(ptr); -} +public: + void freeResource(void *value) + { + delete reinterpret_cast(value); + } +}; -Q_GLOBAL_STATIC_WITH_ARGS(QGLContextResource, qt_blur_texture_caches, (QGLBlurTextureCache_free)) +Q_GLOBAL_STATIC(QGLBlurCacheResource, qt_blur_texture_caches) QGLBlurTextureCache::QGLBlurTextureCache() : timerId(0) -- cgit v0.12