From c7cbf216999cd833cae667b8c6fbfa65d33e8efd Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Thu, 5 Apr 2018 21:12:52 +0200 Subject: [PATCH] QPixmapCache: make sure to not overflow cache limit The cost for the pixmap cache was calculated in bytes but setCacheLimit() takes the size in kilobytes. This lead to the situation that all values above 2097152 overflowed and disabled the caching completely. Fix it by calculating the cost in kilobytes as it is done in QGLContext. Task-number: QTBUG-45293 Change-Id: Ib8dc2360c8f3201ce0b615a04c38b5ccaa8fc6cf Reviewed-by: Friedemann Kleint Reviewed-by: Eirik Aavitsland Reviewed-by: Andy Shaw --- src/gui/image/qpixmapcache.cpp | 25 +++++++++++++------ .../image/qpixmapcache/tst_qpixmapcache.cpp | 3 +++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/gui/image/qpixmapcache.cpp b/src/gui/image/qpixmapcache.cpp index 4b8b1203d6..3d1652f68b 100644 --- a/src/gui/image/qpixmapcache.cpp +++ b/src/gui/image/qpixmapcache.cpp @@ -86,7 +86,17 @@ QT_BEGIN_NAMESPACE \sa QCache, QPixmap */ -static int cache_limit = 10240; // 10 MB cache limit +static const int cache_limit_default = 10240; // 10 MB cache limit + +static inline int cost(const QPixmap &pixmap) +{ + // make sure to do a 64bit calculation + const qint64 costKb = static_cast(pixmap.width()) * + pixmap.height() * pixmap.depth() / (8 * 1024); + const qint64 costMax = std::numeric_limits::max(); + // a small pixmap should have at least a cost of 1(kb) + return static_cast(qBound(1LL, costKb, costMax)); +} /*! \class QPixmapCache::Key @@ -237,7 +247,7 @@ uint qHash(const QPixmapCache::Key &k) QPMCache::QPMCache() : QObject(0), - QCache(cache_limit * 1024), + QCache(cache_limit_default), keyArray(0), theid(0), ps(0), keyArraySize(0), freeKey(0), t(false) { } @@ -553,7 +563,7 @@ bool QPixmapCache::find(const Key &key, QPixmap* pixmap) bool QPixmapCache::insert(const QString &key, const QPixmap &pixmap) { - return pm_cache()->insert(key, pixmap, pixmap.width() * pixmap.height() * pixmap.depth() / 8); + return pm_cache()->insert(key, pixmap, cost(pixmap)); } /*! @@ -573,7 +583,7 @@ bool QPixmapCache::insert(const QString &key, const QPixmap &pixmap) */ QPixmapCache::Key QPixmapCache::insert(const QPixmap &pixmap) { - return pm_cache()->insert(pixmap, pixmap.width() * pixmap.height() * pixmap.depth() / 8); + return pm_cache()->insert(pixmap, cost(pixmap)); } /*! @@ -590,7 +600,7 @@ bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap) //The key is not valid anymore, a flush happened before probably if (!key.d || !key.d->isValid) return false; - return pm_cache()->replace(key, pixmap, pixmap.width() * pixmap.height() * pixmap.depth() / 8); + return pm_cache()->replace(key, pixmap, cost(pixmap)); } /*! @@ -603,7 +613,7 @@ bool QPixmapCache::replace(const Key &key, const QPixmap &pixmap) int QPixmapCache::cacheLimit() { - return cache_limit; + return pm_cache()->maxCost(); } /*! @@ -616,8 +626,7 @@ int QPixmapCache::cacheLimit() void QPixmapCache::setCacheLimit(int n) { - cache_limit = n; - pm_cache()->setMaxCost(1024 * cache_limit); + pm_cache()->setMaxCost(n); } /*! diff --git a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp index 8a2a35f86c..158530428d 100644 --- a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp +++ b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp @@ -92,6 +92,9 @@ void tst_QPixmapCache::cacheLimit() // it was between 2048 and 10240 last time I looked at it QVERIFY(originalCacheLimit >= 1024 && originalCacheLimit <= 20480); + QPixmapCache::setCacheLimit(std::numeric_limits::max()); + QCOMPARE(QPixmapCache::cacheLimit(), std::numeric_limits::max()); + QPixmapCache::setCacheLimit(100); QCOMPARE(QPixmapCache::cacheLimit(), 100);