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

3 files changed, 45 insertions, 14 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;