From bf1d4023f36fa213b62cf947dbca1590f89b58a7 Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Sat, 25 May 2024 14:54:34 +0200 Subject: [PATCH] QIcon/QIconLoader: fix usage of scaledPixmap() QIconEngine::scaledPixmap() gets the unscaled size of the pixmap, not the scaled one. This is correctly handled by QSvgIconEngine, QAppleIconEngine, QAndroidPlatformIconEngine and QWindowsIconEngine but not internally. Therefore fix this here and also make sure the pixmap with the correct dpr is saved in the QPixmapCache to avoid a detach resulting in an increased memory usage when the pixmap dpr did not match the expected dpr. Fixes: QTBUG-124573 Change-Id: Ic75d7a89dae89da326c72cac326490f49d135fa7 Reviewed-by: Volker Hilsheimer (cherry picked from commit 245bdc8ec31755d6ab38b796014bdcab6a1d17ae) --- src/gui/image/qabstractfileiconengine.cpp | 15 ++++++------ src/gui/image/qicon.cpp | 14 ++++++++---- src/gui/image/qiconengine.cpp | 9 +++++--- src/gui/image/qiconloader.cpp | 28 +++++++++-------------- src/gui/image/qiconloader_p.h | 7 +++--- 5 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/gui/image/qabstractfileiconengine.cpp b/src/gui/image/qabstractfileiconengine.cpp index ffbe088d206..b9b9abc1f55 100644 --- a/src/gui/image/qabstractfileiconengine.cpp +++ b/src/gui/image/qabstractfileiconengine.cpp @@ -28,6 +28,11 @@ using namespace Qt::StringLiterals; */ QPixmap QAbstractFileIconEngine::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) +{ + return scaledPixmap(size, mode, state, 1.0); +} + +QPixmap QAbstractFileIconEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) { Q_UNUSED(mode); Q_UNUSED(state); @@ -37,13 +42,13 @@ QPixmap QAbstractFileIconEngine::pixmap(const QSize &size, QIcon::Mode mode, QString key = cacheKey(); if (key.isEmpty()) - return filePixmap(size, mode, state); + return filePixmap(size * scale, mode, state); key += u'_' + QString::number(size.width()); QPixmap result; if (!QPixmapCache::find(key, &result)) { - result = filePixmap(size, mode, state); + result = filePixmap(size * scale, mode, state); if (!result.isNull()) QPixmapCache::insert(key, result); } @@ -51,12 +56,6 @@ QPixmap QAbstractFileIconEngine::pixmap(const QSize &size, QIcon::Mode mode, return result; } -QPixmap QAbstractFileIconEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) -{ - Q_UNUSED(scale); // (size is pre-multiplied by scale) - return pixmap(size, mode, state); -} - QSize QAbstractFileIconEngine::actualSize(const QSize &size, QIcon::Mode mode, QIcon::State state) { diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp index 8d3e93bbc1a..65c016c27f1 100644 --- a/src/gui/image/qicon.cpp +++ b/src/gui/image/qicon.cpp @@ -209,7 +209,7 @@ static QPixmapIconEngineEntry *bestSizeScaleMatch(const QSize &size, qreal scale return (qAbs(ascore) < qAbs(bscore)) ? pa : pb; } - qint64 s = area(size); + qint64 s = area(size * scale); if (pa->size == QSize() && pa->pixmap.isNull()) { pa->pixmap = QPixmap(pa->fileName); pa->size = pa->pixmap.size(); @@ -346,13 +346,15 @@ QPixmap QPixmapIconEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIc return scaledPixmap(size, mode, state, scale); } - const auto actualSize = adjustSize(size, pm.size()); + const auto actualSize = adjustSize(size * scale, pm.size()); + const auto calculatedDpr = QIconPrivate::pixmapDevicePixelRatio(scale, size, actualSize); QString key = "qt_"_L1 % HexString(pm.cacheKey()) % HexString(pe->mode) % HexString(QGuiApplication::palette().cacheKey()) % HexString(actualSize.width()) - % HexString(actualSize.height()); + % HexString(actualSize.height()) + % HexString(qRound(calculatedDpr * 1000)); if (mode == QIcon::Active) { if (QPixmapCache::find(key % HexString(mode), &pm)) @@ -376,6 +378,7 @@ QPixmap QPixmapIconEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIc if (!generated.isNull()) pm = generated; } + pm.setDevicePixelRatio(calculatedDpr); QPixmapCache::insert(key % HexString(mode), pm); } return pm; @@ -415,7 +418,8 @@ QList QPixmapIconEngine::availableSizes(QIcon::Mode mode, QIcon::State st void QPixmapIconEngine::addPixmap(const QPixmap &pixmap, QIcon::Mode mode, QIcon::State state) { if (!pixmap.isNull()) { - QPixmapIconEngineEntry *pe = tryMatch(pixmap.size(), pixmap.devicePixelRatio(), mode, state); + QPixmapIconEngineEntry *pe = tryMatch(pixmap.size() / pixmap.devicePixelRatio(), + pixmap.devicePixelRatio(), mode, state); if (pe && pe->size == pixmap.size() && pe->pixmap.devicePixelRatio() == pixmap.devicePixelRatio()) { pe->pixmap = pixmap; pe->fileName.clear(); @@ -905,7 +909,7 @@ QPixmap QIcon::pixmap(const QSize &size, qreal devicePixelRatio, Mode mode, Stat } // Try get a pixmap that is big enough to be displayed at device pixel resolution. - QPixmap pixmap = d->engine->scaledPixmap(size * devicePixelRatio, mode, state, devicePixelRatio); + QPixmap pixmap = d->engine->scaledPixmap(size, mode, state, devicePixelRatio); pixmap.setDevicePixelRatio(d->pixmapDevicePixelRatio(devicePixelRatio, size, pixmap.size())); return pixmap; } diff --git a/src/gui/image/qiconengine.cpp b/src/gui/image/qiconengine.cpp index efcc3824ba7..999da649e44 100644 --- a/src/gui/image/qiconengine.cpp +++ b/src/gui/image/qiconengine.cpp @@ -42,6 +42,9 @@ QT_BEGIN_NAMESPACE /*! Returns the actual size of the icon the engine provides for the requested \a size, \a mode and \a state. The default implementation returns the given \a size. + + The returned size is in device-independent pixels (This + is relevant for high-dpi pixmaps). */ QSize QIconEngine::actualSize(const QSize &size, QIcon::Mode /*mode*/, QIcon::State /*state*/) { @@ -234,10 +237,10 @@ void QIconEngine::virtual_hook(int id, void *data) { switch (id) { case QIconEngine::ScaledPixmapHook: { - // We don't have any notion of scale besides "@nx", so just call pixmap() here. QIconEngine::ScaledPixmapArgument &arg = *reinterpret_cast(data); - arg.pixmap = pixmap(arg.size, arg.mode, arg.state); + // try to get a pixmap with the correct size, the dpr is adjusted later on + arg.pixmap = pixmap(arg.size * arg.scale, arg.mode, arg.state); break; } default: @@ -282,7 +285,7 @@ bool QIconEngine::isNull() Returns a pixmap for the given \a size, \a mode, \a state and \a scale. The \a scale argument is typically equal to the \l {High DPI} - {device pixel ratio} of the display. + {device pixel ratio} of the display. The size is given in device-independent pixels. \include qiconengine-virtualhookhelper.qdocinc diff --git a/src/gui/image/qiconloader.cpp b/src/gui/image/qiconloader.cpp index 97735e4af63..bc7cafc53c0 100644 --- a/src/gui/image/qiconloader.cpp +++ b/src/gui/image/qiconloader.cpp @@ -900,7 +900,7 @@ QSize QIconLoaderEngine::actualSize(const QSize &size, QIcon::Mode mode, return QSize(0, 0); } -QPixmap PixmapEntry::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) +QPixmap PixmapEntry::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) { Q_UNUSED(state); @@ -911,13 +911,15 @@ QPixmap PixmapEntry::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State st // If the size of the best match we have (basePixmap) is larger than the // requested size, we downscale it to match. - const auto actualSize = QPixmapIconEngine::adjustSize(size, basePixmap.size()); + const auto actualSize = QPixmapIconEngine::adjustSize(size * scale, basePixmap.size()); + const auto calculatedDpr = QIconPrivate::pixmapDevicePixelRatio(scale, size, actualSize); QString key = "$qt_theme_"_L1 % HexString(basePixmap.cacheKey()) % HexString(mode) % HexString(QGuiApplication::palette().cacheKey()) % HexString(actualSize.width()) - % HexString(actualSize.height()); + % HexString(actualSize.height()) + % HexString(qRound(calculatedDpr * 1000)); QPixmap cachedPixmap; if (QPixmapCache::find(key, &cachedPixmap)) { @@ -929,32 +931,24 @@ QPixmap PixmapEntry::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State st cachedPixmap = basePixmap; if (QGuiApplication *guiApp = qobject_cast(qApp)) cachedPixmap = static_cast(QObjectPrivate::get(guiApp))->applyQIconStyleHelper(mode, cachedPixmap); + cachedPixmap.setDevicePixelRatio(calculatedDpr); QPixmapCache::insert(key, cachedPixmap); } return cachedPixmap; } -QPixmap ScalableEntry::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) +QPixmap ScalableEntry::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) { if (svgIcon.isNull()) svgIcon = QIcon(filename); - // Bypass QIcon API, as that will scale by device pixel ratio of the - // highest DPR screen since we're not passing on any QWindow. - if (QIconEngine *engine = svgIcon.data_ptr() ? svgIcon.data_ptr()->engine : nullptr) - return engine->pixmap(size, mode, state); - - return QPixmap(); + return svgIcon.pixmap(size, scale, mode, state); } QPixmap QIconLoaderEngine::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) { - QIconLoaderEngineEntry *entry = entryForSize(m_info, size); - if (entry) - return entry->pixmap(size, mode, state); - - return QPixmap(); + return scaledPixmap(size, mode, state, 1.0); } QString QIconLoaderEngine::key() const @@ -975,8 +969,8 @@ bool QIconLoaderEngine::isNull() QPixmap QIconLoaderEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) { const int integerScale = qCeil(scale); - QIconLoaderEngineEntry *entry = entryForSize(m_info, size / integerScale, integerScale); - return entry ? entry->pixmap(size, mode, state) : QPixmap(); + QIconLoaderEngineEntry *entry = entryForSize(m_info, size, integerScale); + return entry ? entry->pixmap(size, mode, state, scale) : QPixmap(); } QList QIconLoaderEngine::availableSizes(QIcon::Mode mode, QIcon::State state) diff --git a/src/gui/image/qiconloader_p.h b/src/gui/image/qiconloader_p.h index 4a8079a3e98..5d0a65e09d5 100644 --- a/src/gui/image/qiconloader_p.h +++ b/src/gui/image/qiconloader_p.h @@ -66,20 +66,21 @@ public: virtual ~QIconLoaderEngineEntry() {} virtual QPixmap pixmap(const QSize &size, QIcon::Mode mode, - QIcon::State state) = 0; + QIcon::State state, + qreal scale) = 0; QString filename; QIconDirInfo dir; }; struct ScalableEntry : public QIconLoaderEngineEntry { - QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) override; + QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) override; QIcon svgIcon; }; struct PixmapEntry : public QIconLoaderEngineEntry { - QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) override; + QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) override; QPixmap basePixmap; };