From 660a63d73944c198d8c2e65a0e35fa63e13784b7 Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Thu, 6 Feb 2025 21:28:24 +0100 Subject: [PATCH] QIcon::pixmap() make sure to always return a correctly sized pixmap Some icon engines might not be able to return a properly sized pixmap. Therefore we must make sure within QIcon::pixmap() to return a pixmap with the requested size. This is done by simply adjusting the device pixel ratio instead scaling to avoid the loosy scaling until the icon is drawn later on. The dpr adjustment was already done for dpr == 1.0 so the function returned different results for different device pixel ratios ... Pick-to: 6.8 Fixes: QTBUG-133412 Change-Id: I66f2ac76ebf240a625649171b4553a3b95d7c3a1 Reviewed-by: Volker Hilsheimer (cherry picked from commit f142bd121c5be67a5701c849cea25e7abe4cd720) Reviewed-by: Qt Cherry-pick Bot --- src/gui/image/qicon.cpp | 13 ++--- tests/auto/gui/image/qicon/CMakeLists.txt | 1 + tests/auto/gui/image/qicon/tst_qicon.cpp | 65 ++++++++++++++++++++++- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp index acb059d7aba..854e936d6ea 100644 --- a/src/gui/image/qicon.cpp +++ b/src/gui/image/qicon.cpp @@ -917,6 +917,8 @@ QPixmap QIcon::pixmap(const QSize &size, Mode mode, State state) const might be smaller than requested, but never larger, unless the device-pixel ratio of the returned pixmap is larger than 1. + \note The requested devicePixelRatio might not match the returned one. This delays the + scaling of the QPixmap until it is drawn later on. \note Prior to Qt 6.8 this function wronlgy passed the device dependent pixmap size to QIconEngine::scaledPixmap(), since Qt 6.8 it's the device independent size (not scaled with the \a devicePixelRatio). @@ -932,15 +934,10 @@ QPixmap QIcon::pixmap(const QSize &size, qreal devicePixelRatio, Mode mode, Stat if (devicePixelRatio == -1) devicePixelRatio = qApp->devicePixelRatio(); - // Handle the simple normal-dpi case - if (!(devicePixelRatio > 1.0)) { - QPixmap pixmap = d->engine->pixmap(size, mode, state); - pixmap.setDevicePixelRatio(1.0); - return pixmap; - } - - // Try get a pixmap that is big enough to be displayed at device pixel resolution. QPixmap pixmap = d->engine->scaledPixmap(size, mode, state, devicePixelRatio); + // even though scaledPixmap() should return a pixmap with an appropriate size, we + // can not rely on it. Therefore we simply set the dpr to a correct size to make + // sure the pixmap is not larger than requested. pixmap.setDevicePixelRatio(d->pixmapDevicePixelRatio(devicePixelRatio, size, pixmap.size())); return pixmap; } diff --git a/tests/auto/gui/image/qicon/CMakeLists.txt b/tests/auto/gui/image/qicon/CMakeLists.txt index 93f75741c0e..5ef1cf2d8f1 100644 --- a/tests/auto/gui/image/qicon/CMakeLists.txt +++ b/tests/auto/gui/image/qicon/CMakeLists.txt @@ -44,6 +44,7 @@ qt_internal_add_test(tst_qicon tst_qicon.cpp LIBRARIES Qt::Gui + Qt::GuiPrivate TestIconPlugin TESTDATA ${test_data} ) diff --git a/tests/auto/gui/image/qicon/tst_qicon.cpp b/tests/auto/gui/image/qicon/tst_qicon.cpp index d11c87d2284..695596b181d 100644 --- a/tests/auto/gui/image/qicon/tst_qicon.cpp +++ b/tests/auto/gui/image/qicon/tst_qicon.cpp @@ -10,7 +10,7 @@ #include #endif #include -#include +#include #include @@ -34,6 +34,8 @@ private slots: void detach(); void addFile(); void pixmap(); + void pixmapByDprFromEngine_data(); + void pixmapByDprFromEngine(); void paint(); void availableSizes(); void name(); @@ -447,6 +449,67 @@ void tst_QIcon::pixmap() QVERIFY(icon.pixmap(QSize(16, 16), -1).size().width() >= 16); } +void tst_QIcon::pixmapByDprFromEngine_data() +{ + QTest::addColumn("engineSize"); + QTest::addColumn("requestedSize"); + QTest::addColumn("requestedDpr"); + QTest::addColumn("expectedSize"); + QTest::addColumn("expectedDpr"); + + QTest::newRow("engine 16x16, request 32x32, dpr = 1") + << 16 << 32 << 1.0 << 16 << 1.0; // no upscaling is done + QTest::newRow("engine 16x16, request 32x32, dpr = 2") + << 16 << 32 << 2.0 << 16 << 1.0; // no upscaling is done + QTest::newRow("engine 32x32, request 32x32, dpr = 1") + << 32 << 32 << 1.0 << 32 << 1.0; + QTest::newRow("engine 32x32, request 32x32, dpr = 2") + << 32 << 32 << 2.0 << 32 << 1.0; // no upscaling is done + QTest::newRow("engine 32x32, request 16x16, dpr = 1") + << 32 << 16 << 1.0 << 32 << 2.0; // downscaling done by increasing dpr + QTest::newRow("engine 32x32, request 16x16, dpr = 2") + << 32 << 16 << 2.0 << 32 << 2.0; + QTest::newRow("engine 32x32, request 8x8, dpr = 1") + << 32 << 8 << 1.0 << 32 << 4.0; // downscaling done by increasing dpr + QTest::newRow("engine 32x32, request 8x8, dpr = 2") + << 32 << 8 << 2.0 << 32 << 4.0; // downscaling done by increasing dpr +} + +void tst_QIcon::pixmapByDprFromEngine() +{ + QFETCH(int, engineSize); + QFETCH(int, requestedSize); + QFETCH(qreal, requestedDpr); + QFETCH(int, expectedSize); + QFETCH(qreal, expectedDpr); + + class TestEngine : public QPixmapIconEngine + { + public: + using QPixmapIconEngine::QPixmapIconEngine; + QSize size; + + QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) override + { + return scaledPixmap(size, mode, state, 1.0f); + } + QPixmap scaledPixmap(const QSize &, QIcon::Mode, QIcon::State, qreal) override + { + // simulate an icon engine which does no scaling (= only has fixed size icons) + QPixmap pm(size); + pm.fill(Qt::red); + return pm; + } + }; + + auto testEngine = new TestEngine; + QIcon ico(testEngine); + testEngine->size = QSize(engineSize, engineSize); + auto pm = ico.pixmap(QSize(requestedSize, requestedSize), requestedDpr); + QCOMPARE(pm.size(), QSize(expectedSize, expectedSize)); + QCOMPARE(pm.devicePixelRatio(), expectedDpr); +} + void tst_QIcon::paint() { QImage img16_1x(16, 16, QImage::Format_ARGB32);