Fix several bugs with GL texture cache

Reviewed-By: Trond
Autotest: tst_QGL::qglContextDefaultBindTexture

4 files changed, 43 insertions, 29 deletions

diff --git a/src/gui/image/qimagepixmapcleanuphooks.cpp b/src/gui/image/qimagepixmapcleanuphooks.cpp
index ace4bb6..517fcb0 100644
--- a/src/gui/image/qimagepixmapcleanuphooks.cpp
+++ b/src/gui/image/qimagepixmapcleanuphooks.cpp
@@ -122,19 +122,32 @@ void QImagePixmapCleanupHooks::executeImageHooks(qint64 key)
         qt_image_cleanup_hook_64(key);
 }
 
-void QImagePixmapCleanupHooks::enableCleanupHooks(const QPixmap &pixmap)
-{
-    enableCleanupHooks(const_cast<QPixmap &>(pixmap).data_ptr().data());
-}
 
 void QImagePixmapCleanupHooks::enableCleanupHooks(QPixmapData *pixmapData)
 {
     pixmapData->is_cached = true;
 }
 
+void QImagePixmapCleanupHooks::enableCleanupHooks(const QPixmap &pixmap)
+{
+    enableCleanupHooks(const_cast<QPixmap &>(pixmap).data_ptr().data());
+}
+
 void QImagePixmapCleanupHooks::enableCleanupHooks(const QImage &image)
 {
     const_cast<QImage &>(image).data_ptr()->is_cached = true;
 }
 
+bool QImagePixmapCleanupHooks::isImageCached(const QImage &image)
+{
+    return const_cast<QImage &>(image).data_ptr()->is_cached;
+}
+
+bool QImagePixmapCleanupHooks::isPixmapCached(const QPixmap &pixmap)
+{
+    return const_cast<QPixmap&>(pixmap).data_ptr().data()->is_cached;
+}
+
+
+
 QT_END_NAMESPACE
diff --git a/src/gui/image/qimagepixmapcleanuphooks_p.h b/src/gui/image/qimagepixmapcleanuphooks_p.h
index 88dd3a6..eae11f4 100644
--- a/src/gui/image/qimagepixmapcleanuphooks_p.h
+++ b/src/gui/image/qimagepixmapcleanuphooks_p.h
@@ -72,6 +72,9 @@ public:
     static void enableCleanupHooks(const QPixmap &pixmap);
     static void enableCleanupHooks(QPixmapData *pixmapData);
 
+    static bool isImageCached(const QImage &image);
+    static bool isPixmapCached(const QPixmap &pixmap);
+
     // Gets called when a pixmap data is about to be modified:
     void addPixmapDataModificationHook(_qt_pixmap_cleanup_hook_pmd);
 
diff --git a/src/opengl/qgl.cpp b/src/opengl/qgl.cpp
index 0a89412..22b0602 100644
--- a/src/opengl/qgl.cpp
+++ b/src/opengl/qgl.cpp
@@ -1594,18 +1594,18 @@ QGLTextureCache::QGLTextureCache()
     Q_ASSERT(qt_gl_texture_cache == 0);
     qt_gl_texture_cache = this;
 
-    QImagePixmapCleanupHooks::instance()->addPixmapDataModificationHook(cleanupTextures);
+    QImagePixmapCleanupHooks::instance()->addPixmapDataModificationHook(cleanupTexturesForPixampData);
     QImagePixmapCleanupHooks::instance()->addPixmapDataDestructionHook(cleanupBeforePixmapDestruction);
-    QImagePixmapCleanupHooks::instance()->addImageHook(imageCleanupHook);
+    QImagePixmapCleanupHooks::instance()->addImageHook(cleanupTexturesForCacheKey);
 }
 
 QGLTextureCache::~QGLTextureCache()
 {
     qt_gl_texture_cache = 0;
 
-    QImagePixmapCleanupHooks::instance()->removePixmapDataModificationHook(cleanupTextures);
+    QImagePixmapCleanupHooks::instance()->removePixmapDataModificationHook(cleanupTexturesForPixampData);
     QImagePixmapCleanupHooks::instance()->removePixmapDataDestructionHook(cleanupBeforePixmapDestruction);
-    QImagePixmapCleanupHooks::instance()->removeImageHook(imageCleanupHook);
+    QImagePixmapCleanupHooks::instance()->removeImageHook(cleanupTexturesForCacheKey);
 }
 
 void QGLTextureCache::insert(QGLContext* ctx, qint64 key, QGLTexture* texture, int cost)
@@ -1661,32 +1661,25 @@ QGLTextureCache* QGLTextureCache::instance()
   a hook that removes textures from the cache when a pixmap/image
   is deref'ed
 */
-void QGLTextureCache::imageCleanupHook(qint64 cacheKey)
+void QGLTextureCache::cleanupTexturesForCacheKey(qint64 cacheKey)
 {
     // ### remove when the GL texture cache becomes thread-safe
-    if (qApp->thread() != QThread::currentThread())
-        return;
-    QGLTexture *texture = instance()->getTexture(cacheKey);
-    if (texture && texture->options & QGLContext::MemoryManagedBindOption)
+    if (qApp->thread() == QThread::currentThread()) {
         instance()->remove(cacheKey);
+        Q_ASSERT(instance()->getTexture(cacheKey) == 0);
+    }
 }
 
 
-void QGLTextureCache::cleanupTextures(QPixmapData* pmd)
+void QGLTextureCache::cleanupTexturesForPixampData(QPixmapData* pmd)
 {
-    // ### remove when the GL texture cache becomes thread-safe
-    if (qApp->thread() == QThread::currentThread()) {
-        const qint64 cacheKey = pmd->cacheKey();
-        QGLTexture *texture = instance()->getTexture(cacheKey);
-        if (texture && texture->options & QGLContext::MemoryManagedBindOption)
-            instance()->remove(cacheKey);
-    }
+    cleanupTexturesForCacheKey(pmd->cacheKey());
 }
 
 void QGLTextureCache::cleanupBeforePixmapDestruction(QPixmapData* pmd)
 {
     // Remove any bound textures first:
-    cleanupTextures(pmd);
+    cleanupTexturesForPixampData(pmd);
 
 #if defined(Q_WS_X11)
     if (pmd->classId() == QPixmapData::X11Class) {
@@ -2087,8 +2080,9 @@ QGLTexture *QGLContextPrivate::bindTexture(const QImage &image, GLenum target, G
     // NOTE: bindTexture(const QImage&, GLenum, GLint, const qint64, bool) should never return null
     Q_ASSERT(texture);
 
-    if (texture->id > 0)
-        QImagePixmapCleanupHooks::enableCleanupHooks(image);
+    // Enable the cleanup hooks for this image so that the texture cache entry is removed when the
+    // image gets deleted:
+    QImagePixmapCleanupHooks::enableCleanupHooks(image);
 
     return texture;
 }
@@ -2142,6 +2136,7 @@ QGLTexture* QGLContextPrivate::bindTexture(const QImage &image, GLenum target, G
     int tx_h = qt_next_power_of_two(image.height());
 
     QImage img = image;
+
     if (!(QGLExtensions::glExtensions() & QGLExtensions::NPOTTextures)
         && !(QGLFormat::openGLVersionFlags() & QGLFormat::OpenGL_ES_Version_2_0)
         && (target == GL_TEXTURE_2D && (tx_w != image.width() || tx_h != image.height())))
@@ -2310,6 +2305,7 @@ QGLTexture* QGLContextPrivate::bindTexture(const QImage &image, GLenum target, G
     int cost = img.width()*img.height()*4/1024;
     QGLTexture *texture = new QGLTexture(q, tx_id, target, options);
     QGLTextureCache::instance()->insert(q, key, texture, cost);
+
     return texture;
 }
 
@@ -2426,7 +2422,7 @@ GLuint QGLContext::bindTexture(const QImage &image, GLenum target, GLint format)
         return 0;
 
     Q_D(QGLContext);
-    QGLTexture *texture = d->bindTexture(image, target, format, false, DefaultBindOption);
+    QGLTexture *texture = d->bindTexture(image, target, format, DefaultBindOption);
     return texture->id;
 }
 
@@ -2566,11 +2562,13 @@ void QGLContext::deleteTexture(GLuint id)
     for (int i = 0; i < ddsKeys.size(); ++i) {
         GLuint texture = dds_cache->value(ddsKeys.at(i));
         if (id == texture) {
-            glDeleteTextures(1, &texture);
             dds_cache->remove(ddsKeys.at(i));
-            return;
+            break;
         }
     }
+
+    // Finally, actually delete the texture ID
+    glDeleteTextures(1, &id);
 }
 
 #ifdef Q_MAC_COMPAT_GL_FUNCTIONS
diff --git a/src/opengl/qgl_p.h b/src/opengl/qgl_p.h
index da0887e..0d5a54a 100644
--- a/src/opengl/qgl_p.h
+++ b/src/opengl/qgl_p.h
@@ -540,8 +540,8 @@ public:
 
     static QGLTextureCache *instance();
     static void deleteIfEmpty();
-    static void imageCleanupHook(qint64 cacheKey);
-    static void cleanupTextures(QPixmapData* pixmap);
+    static void cleanupTexturesForCacheKey(qint64 cacheKey);
+    static void cleanupTexturesForPixampData(QPixmapData* pixmap);
     static void cleanupBeforePixmapDestruction(QPixmapData* pixmap);
 
 private: