Pixmap loader adds images to unreferenced list when cache: false
authorMartin Jones <martin.jones@nokia.com>
Fri, 27 Jul 2012 03:38:43 +0000 (13:38 +1000)
committerQt by Nokia <qt-info@nokia.com>
Fri, 27 Jul 2012 06:43:06 +0000 (08:43 +0200)
Don't add uncached images to the unreferenced list, since
they cannot be reused later as they are not in the cache.

Additionally, we currently search for images in the cache, even if we
set cache: false. Setting cache false should not put images in the cache
and should not use images that are in the cache.

Task-number: QTBUG-26676
Change-Id: Ib7eb42199ae6ae6154b696c83ad1dd959e0f208f
Reviewed-by: Andrew den Exter <andrew.den-exter@nokia.com>
Reviewed-by: Martin Jones <martin.jones@nokia.com>
src/quick/util/qquickpixmapcache.cpp
tests/auto/quick/qquickpixmapcache/tst_qquickpixmapcache.cpp

index 1f4da46..6ac5be5 100644 (file)
@@ -902,7 +902,10 @@ void QQuickPixmapData::release()
         }
 
         if (pixmapStatus == QQuickPixmap::Ready) {
-            pixmapStore()->unreferencePixmap(this);
+            if (inCache)
+                pixmapStore()->unreferencePixmap(this);
+            else
+                delete this;
         } else {
             removeFromCache();
             delete this;
@@ -1163,7 +1166,12 @@ void QQuickPixmap::load(QQmlEngine *engine, const QUrl &url, const QSize &reques
     QQuickPixmapKey key = { &url, &requestSize };
     QQuickPixmapStore *store = pixmapStore();
 
-    QHash<QQuickPixmapKey, QQuickPixmapData *>::Iterator iter = store->m_cache.find(key);
+    QHash<QQuickPixmapKey, QQuickPixmapData *>::Iterator iter = store->m_cache.end();
+
+    // If Cache is disabled, the pixmap will always be loaded, even if there is an existing
+    // cached version.
+    if (options & QQuickPixmap::Cache)
+        iter = store->m_cache.find(key);
 
     if (iter == store->m_cache.end()) {
         if (url.scheme() == QLatin1String("image")) {
index fb2405c..c238ed8 100644 (file)
@@ -74,6 +74,7 @@ private slots:
     void networkCrash();
 #endif
     void lockingCrash();
+    void uncached();
 #if PIXMAP_DATA_LEAK_TEST
     void dataLeak();
 #endif
@@ -352,11 +353,15 @@ public:
     virtual QPixmap requestPixmap(const QString &d, QSize *, const QSize &) {
         Q_UNUSED(d)
         QPixmap pix(800, 600);
-        pix.fill(Qt::red);
+        pix.fill(fillColor);
         return pix;
     }
+
+    static QRgb fillColor;
 };
 
+QRgb MyPixmapProvider::fillColor = qRgb(255, 0, 0);
+
 // QTBUG-13345
 void tst_qquickpixmapcache::shrinkcache()
 {
@@ -417,6 +422,56 @@ void tst_qquickpixmapcache::lockingCrash()
     }
 }
 
+void tst_qquickpixmapcache::uncached()
+{
+    QQmlEngine engine;
+    engine.addImageProvider(QLatin1String("mypixmaps"), new MyPixmapProvider);
+
+    QUrl url("image://mypixmaps/mypix");
+    {
+        QQuickPixmap p;
+        p.load(&engine, url, 0);
+        QImage img = p.image();
+        QCOMPARE(img.pixel(0,0), qRgb(255, 0, 0));
+    }
+
+    // uncached, so we will get a different colored image
+    MyPixmapProvider::fillColor = qRgb(0, 255, 0);
+    {
+        QQuickPixmap p;
+        p.load(&engine, url, 0);
+        QImage img = p.image();
+        QCOMPARE(img.pixel(0,0), qRgb(0, 255, 0));
+    }
+
+    // Load the image with cache enabled
+    MyPixmapProvider::fillColor = qRgb(0, 0, 255);
+    {
+        QQuickPixmap p;
+        p.load(&engine, url, QQuickPixmap::Cache);
+        QImage img = p.image();
+        QCOMPARE(img.pixel(0,0), qRgb(0, 0, 255));
+    }
+
+    // We should not get the cached version if we request uncached
+    MyPixmapProvider::fillColor = qRgb(255, 0, 255);
+    {
+        QQuickPixmap p;
+        p.load(&engine, url, 0);
+        QImage img = p.image();
+        QCOMPARE(img.pixel(0,0), qRgb(255, 0, 255));
+    }
+
+    // If we again load the image with cache enabled, we should get the previously cached version
+    MyPixmapProvider::fillColor = qRgb(0, 255, 255);
+    {
+        QQuickPixmap p;
+        p.load(&engine, url, QQuickPixmap::Cache);
+        QImage img = p.image();
+        QCOMPARE(img.pixel(0,0), qRgb(0, 0, 255));
+    }
+}
+
 
 #if PIXMAP_DATA_LEAK_TEST
 // This test should not be enabled by default as it