From 22b9079040ae0d4f35781509fa6aea7e38ac47bb Mon Sep 17 00:00:00 2001
From: Tom Cooksey <thomas.cooksey@nokia.com>
Date: Mon, 12 Oct 2009 15:39:52 +0200
Subject: Separate modification & destruction pixmap cleanup hooks
Before the QExplicitlySharedDataPointer change, the ref-count was 0 when
calling the cleanup hooks from ~QPixmap. That enabled the hook to figure
out if the pixmap is being modified or deleted. As the ref count is now
1 when calling the cleanup hooks in ~QPixmap, we need to seperate the
hooks.
This change should make using textre-from-pixmap faster as the EGL/glX
surface wont get re-created everytime the pixmap is modified.
Reviewed-By: Gunnar
---
src/gui/image/qimagepixmapcleanuphooks.cpp | 35 ++++++++++++++++++++++++------
src/gui/image/qimagepixmapcleanuphooks_p.h | 17 +++++++++++----
src/gui/image/qpixmap.cpp | 7 +++---
src/opengl/qgl.cpp | 21 ++++++++++++++----
src/opengl/qgl_p.h | 6 ++++-
5 files changed, 67 insertions(+), 19 deletions(-)
diff --git a/src/gui/image/qimagepixmapcleanuphooks.cpp b/src/gui/image/qimagepixmapcleanuphooks.cpp
index d08d3ef..138eb0d 100644
--- a/src/gui/image/qimagepixmapcleanuphooks.cpp
+++ b/src/gui/image/qimagepixmapcleanuphooks.cpp
@@ -70,19 +70,30 @@ QImagePixmapCleanupHooks *QImagePixmapCleanupHooks::instance()
return qt_image_and_pixmap_cleanup_hooks;
}
-void QImagePixmapCleanupHooks::addPixmapHook(_qt_pixmap_cleanup_hook_pm hook)
+void QImagePixmapCleanupHooks::addPixmapModificationHook(_qt_pixmap_cleanup_hook_pm hook)
{
- pixmapHooks.append(hook);
+ pixmapModificationHooks.append(hook);
}
+void QImagePixmapCleanupHooks::addPixmapDestructionHook(_qt_pixmap_cleanup_hook_pm hook)
+{
+ pixmapDestructionHooks.append(hook);
+}
+
+
void QImagePixmapCleanupHooks::addImageHook(_qt_image_cleanup_hook_64 hook)
{
imageHooks.append(hook);
}
-void QImagePixmapCleanupHooks::removePixmapHook(_qt_pixmap_cleanup_hook_pm hook)
+void QImagePixmapCleanupHooks::removePixmapModificationHook(_qt_pixmap_cleanup_hook_pm hook)
+{
+ pixmapModificationHooks.removeAll(hook);
+}
+
+void QImagePixmapCleanupHooks::removePixmapDestructionHook(_qt_pixmap_cleanup_hook_pm hook)
{
- pixmapHooks.removeAll(hook);
+ pixmapDestructionHooks.removeAll(hook);
}
void QImagePixmapCleanupHooks::removeImageHook(_qt_image_cleanup_hook_64 hook)
@@ -91,15 +102,25 @@ void QImagePixmapCleanupHooks::removeImageHook(_qt_image_cleanup_hook_64 hook)
}
-void QImagePixmapCleanupHooks::executePixmapHooks(QPixmap* pm)
+void QImagePixmapCleanupHooks::executePixmapModificationHooks(QPixmap* pm)
{
- for (int i = 0; i < qt_image_and_pixmap_cleanup_hooks->pixmapHooks.count(); ++i)
- qt_image_and_pixmap_cleanup_hooks->pixmapHooks[i](pm);
+ Q_ASSERT(qt_image_and_pixmap_cleanup_hooks);
+ for (int i = 0; i < qt_image_and_pixmap_cleanup_hooks->pixmapModificationHooks.count(); ++i)
+ qt_image_and_pixmap_cleanup_hooks->pixmapModificationHooks[i](pm);
if (qt_pixmap_cleanup_hook_64)
qt_pixmap_cleanup_hook_64(pm->cacheKey());
}
+void QImagePixmapCleanupHooks::executePixmapDestructionHooks(QPixmap* pm)
+{
+ Q_ASSERT(qt_image_and_pixmap_cleanup_hooks);
+ for (int i = 0; i < qt_image_and_pixmap_cleanup_hooks->pixmapDestructionHooks.count(); ++i)
+ qt_image_and_pixmap_cleanup_hooks->pixmapDestructionHooks[i](pm);
+
+ if (qt_pixmap_cleanup_hook_64)
+ qt_pixmap_cleanup_hook_64(pm->cacheKey());
+}
void QImagePixmapCleanupHooks::executeImageHooks(qint64 key)
{
diff --git a/src/gui/image/qimagepixmapcleanuphooks_p.h b/src/gui/image/qimagepixmapcleanuphooks_p.h
index dd2d0f7..16c8974 100644
--- a/src/gui/image/qimagepixmapcleanuphooks_p.h
+++ b/src/gui/image/qimagepixmapcleanuphooks_p.h
@@ -70,18 +70,27 @@ public:
static QImagePixmapCleanupHooks *instance();
- void addPixmapHook(_qt_pixmap_cleanup_hook_pm);
+ // Gets called when a pixmap is about to be modified:
+ void addPixmapModificationHook(_qt_pixmap_cleanup_hook_pm);
+
+ // Gets called when a pixmap is about to be destroyed:
+ void addPixmapDestructionHook(_qt_pixmap_cleanup_hook_pm);
+
+ // Gets called when an image is about to be modified or destroyed:
void addImageHook(_qt_image_cleanup_hook_64);
- void removePixmapHook(_qt_pixmap_cleanup_hook_pm);
+ void removePixmapModificationHook(_qt_pixmap_cleanup_hook_pm);
+ void removePixmapDestructionHook(_qt_pixmap_cleanup_hook_pm);
void removeImageHook(_qt_image_cleanup_hook_64);
- static void executePixmapHooks(QPixmap*);
+ static void executePixmapModificationHooks(QPixmap*);
+ static void executePixmapDestructionHooks(QPixmap*);
static void executeImageHooks(qint64 key);
private:
QList<_qt_image_cleanup_hook_64> imageHooks;
- QList<_qt_pixmap_cleanup_hook_pm> pixmapHooks;
+ QList<_qt_pixmap_cleanup_hook_pm> pixmapModificationHooks;
+ QList<_qt_pixmap_cleanup_hook_pm> pixmapDestructionHooks;
};
QT_END_NAMESPACE
diff --git a/src/gui/image/qpixmap.cpp b/src/gui/image/qpixmap.cpp
index bf6c9ae..f94552d 100644
--- a/src/gui/image/qpixmap.cpp
+++ b/src/gui/image/qpixmap.cpp
@@ -322,8 +322,9 @@ QPixmap::QPixmap(const char * const xpm[])
QPixmap::~QPixmap()
{
- if (data->is_cached && data->ref == 1)
- QImagePixmapCleanupHooks::executePixmapHooks(this);
+ Q_ASSERT(data->ref >= 1); // Catch if ref-counting changes again
+ if (data->is_cached && data->ref == 1) // ref will be decrememnted after destructor returns
+ QImagePixmapCleanupHooks::executePixmapDestructionHooks(this);
}
/*!
@@ -1917,7 +1918,7 @@ void QPixmap::detach()
}
if (data->is_cached && data->ref == 1)
- QImagePixmapCleanupHooks::executePixmapHooks(this);
+ QImagePixmapCleanupHooks::executePixmapModificationHooks(this);
#if defined(Q_WS_MAC)
QMacPixmapData *macData = id == QPixmapData::MacClass ? static_cast<QMacPixmapData*>(data.data()) : 0;
diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp
index 39f04d4..97e3dad 100644
--- a/src/opengl/qgl.cpp
+++ b/src/opengl/qgl.cpp
@@ -1584,7 +1584,10 @@ QGLTextureCache::QGLTextureCache()
Q_ASSERT(qt_gl_texture_cache == 0);
qt_gl_texture_cache = this;
- QImagePixmapCleanupHooks::instance()->addPixmapHook(pixmapCleanupHook);
+ QImagePixmapCleanupHooks::instance()->addPixmapModificationHook(cleanupTextures);
+#ifdef Q_WS_X11
+ QImagePixmapCleanupHooks::instance()->addPixmapDestructionHook(cleanupPixmapSurfaces);
+#endif
QImagePixmapCleanupHooks::instance()->addImageHook(imageCleanupHook);
}
@@ -1592,7 +1595,10 @@ QGLTextureCache::~QGLTextureCache()
{
qt_gl_texture_cache = 0;
- QImagePixmapCleanupHooks::instance()->removePixmapHook(pixmapCleanupHook);
+ QImagePixmapCleanupHooks::instance()->removePixmapModificationHook(cleanupTextures);
+#ifdef Q_WS_X11
+ QImagePixmapCleanupHooks::instance()->removePixmapDestructionHook(cleanupPixmapSurfaces);
+#endif
QImagePixmapCleanupHooks::instance()->removeImageHook(imageCleanupHook);
}
@@ -1660,7 +1666,7 @@ void QGLTextureCache::imageCleanupHook(qint64 cacheKey)
}
-void QGLTextureCache::pixmapCleanupHook(QPixmap* pixmap)
+void QGLTextureCache::cleanupTextures(QPixmap* pixmap)
{
// ### remove when the GL texture cache becomes thread-safe
if (qApp->thread() == QThread::currentThread()) {
@@ -1669,14 +1675,21 @@ void QGLTextureCache::pixmapCleanupHook(QPixmap* pixmap)
if (texture && texture->options & QGLContext::MemoryManagedBindOption)
instance()->remove(cacheKey);
}
+}
+
#if defined(Q_WS_X11)
+void QGLTextureCache::cleanupPixmapSurfaces(QPixmap* pixmap)
+{
+ // Remove any bound textures first:
+ cleanupTextures(pixmap);
+
QPixmapData *pd = pixmap->data_ptr().data();
if (pd->classId() == QPixmapData::X11Class) {
Q_ASSERT(pd->ref == 1); // Make sure reference counting isn't broken
QGLContextPrivate::destroyGlSurfaceForPixmap(pd);
}
-#endif
}
+#endif
void QGLTextureCache::deleteIfEmpty()
{
diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h
index 129e7f7..9a17c67 100644
--- a/src/opengl/qgl_p.h
+++ b/src/opengl/qgl_p.h
@@ -498,7 +498,11 @@ public:
static QGLTextureCache *instance();
static void deleteIfEmpty();
static void imageCleanupHook(qint64 cacheKey);
- static void pixmapCleanupHook(QPixmap* pixmap);
+ static void cleanupTextures(QPixmap* pixmap);
+#ifdef Q_WS_X11
+ // X11 needs to catch pixmap data destruction to delete EGL/GLX pixmap surfaces
+ static void cleanupPixmapSurfaces(QPixmap* pixmap);
+#endif
private:
QCache<qint64, QGLTexture> m_cache;
--
cgit v0.12