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.9 6.8
Fixes: QTBUG-133412
Change-Id: I66f2ac76ebf240a625649171b4553a3b95d7c3a1
Reviewed-by: Volker Hilsheimer <[email protected]>

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;
 }

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}
 )

tests/auto/gui/image/qicon/tst_qicon.cpp

@@ -10,7 +10,7 @@
 #include <QProcess>
 #endif
 #include <qicon.h>
-#include <qiconengine.h>
+#include <private/qabstractfileiconengine_p.h>
 
 #include <algorithm>
 
@@ -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<int>("engineSize");
+    QTest::addColumn<int>("requestedSize");
+    QTest::addColumn<qreal>("requestedDpr");
+    QTest::addColumn<int>("expectedSize");
+    QTest::addColumn<qreal>("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);