diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp index 73b25fd584..0bbc4005a0 100644 --- a/src/gui/image/qicon.cpp +++ b/src/gui/image/qicon.cpp @@ -180,9 +180,28 @@ void QPixmapIconEngine::paint(QPainter *painter, const QRect &rect, QIcon::Mode static inline int area(const QSize &s) { return s.width() * s.height(); } -// returns the smallest of the two that is still larger than or equal to size. -static QPixmapIconEngineEntry *bestSizeMatch( const QSize &size, QPixmapIconEngineEntry *pa, QPixmapIconEngineEntry *pb) +// Returns the smallest of the two that is still larger than or equal to size. +// Pixmaps at the correct scale are preferred, pixmaps at lower scale are +// used as fallbacks. We assume that the pixmap set is complete, in the sense +// that no 2x pixmap is going to be a better match than a 3x pixmap for the the +// target scale of 3 (It's OK if 3x pixmaps are missing - we'll fall back to +// the 2x pixmaps then.) +static QPixmapIconEngineEntry *bestSizeScaleMatch(const QSize &size, qreal scale, QPixmapIconEngineEntry *pa, QPixmapIconEngineEntry *pb) { + + // scale: we can only differentiate on scale if the scale differs + if (pa->scale != pb->scale) { + + // Score the pixmaps: 0 is an exact scale match, postive + // scores have more detail than requested, negative scores + // have less detail than rquested. + qreal ascore = pa->scale - scale; + qreal bscore = pb->scale - scale; + + // Take the one closest to 0 + return (qAbs(ascore) < qAbs(bscore)) ? pa : pb; + } + int s = area(size); if (pa->size == QSize() && pa->pixmap.isNull()) { pa->pixmap = QPixmap(pa->fileName); @@ -204,13 +223,13 @@ static QPixmapIconEngineEntry *bestSizeMatch( const QSize &size, QPixmapIconEngi return pb; } -QPixmapIconEngineEntry *QPixmapIconEngine::tryMatch(const QSize &size, QIcon::Mode mode, QIcon::State state) +QPixmapIconEngineEntry *QPixmapIconEngine::tryMatch(const QSize &size, qreal scale, QIcon::Mode mode, QIcon::State state) { QPixmapIconEngineEntry *pe = nullptr; for (int i = 0; i < pixmaps.count(); ++i) if (pixmaps.at(i).mode == mode && pixmaps.at(i).state == state) { if (pe) - pe = bestSizeMatch(size, &pixmaps[i], pe); + pe = bestSizeScaleMatch(size, scale, &pixmaps[i], pe); else pe = &pixmaps[i]; } @@ -218,42 +237,42 @@ QPixmapIconEngineEntry *QPixmapIconEngine::tryMatch(const QSize &size, QIcon::Mo } -QPixmapIconEngineEntry *QPixmapIconEngine::bestMatch(const QSize &size, QIcon::Mode mode, QIcon::State state, bool sizeOnly) +QPixmapIconEngineEntry *QPixmapIconEngine::bestMatch(const QSize &size, qreal scale, QIcon::Mode mode, QIcon::State state, bool sizeOnly) { - QPixmapIconEngineEntry *pe = tryMatch(size, mode, state); + QPixmapIconEngineEntry *pe = tryMatch(size, scale, mode, state); while (!pe){ QIcon::State oppositeState = (state == QIcon::On) ? QIcon::Off : QIcon::On; if (mode == QIcon::Disabled || mode == QIcon::Selected) { QIcon::Mode oppositeMode = (mode == QIcon::Disabled) ? QIcon::Selected : QIcon::Disabled; - if ((pe = tryMatch(size, QIcon::Normal, state))) + if ((pe = tryMatch(size, scale, QIcon::Normal, state))) break; - if ((pe = tryMatch(size, QIcon::Active, state))) + if ((pe = tryMatch(size, scale, QIcon::Active, state))) break; - if ((pe = tryMatch(size, mode, oppositeState))) + if ((pe = tryMatch(size, scale, mode, oppositeState))) break; - if ((pe = tryMatch(size, QIcon::Normal, oppositeState))) + if ((pe = tryMatch(size, scale, QIcon::Normal, oppositeState))) break; - if ((pe = tryMatch(size, QIcon::Active, oppositeState))) + if ((pe = tryMatch(size, scale, QIcon::Active, oppositeState))) break; - if ((pe = tryMatch(size, oppositeMode, state))) + if ((pe = tryMatch(size, scale, oppositeMode, state))) break; - if ((pe = tryMatch(size, oppositeMode, oppositeState))) + if ((pe = tryMatch(size, scale, oppositeMode, oppositeState))) break; } else { QIcon::Mode oppositeMode = (mode == QIcon::Normal) ? QIcon::Active : QIcon::Normal; - if ((pe = tryMatch(size, oppositeMode, state))) + if ((pe = tryMatch(size, scale, oppositeMode, state))) break; - if ((pe = tryMatch(size, mode, oppositeState))) + if ((pe = tryMatch(size, scale, mode, oppositeState))) break; - if ((pe = tryMatch(size, oppositeMode, oppositeState))) + if ((pe = tryMatch(size, scale, oppositeMode, oppositeState))) break; - if ((pe = tryMatch(size, QIcon::Disabled, state))) + if ((pe = tryMatch(size, scale, QIcon::Disabled, state))) break; - if ((pe = tryMatch(size, QIcon::Selected, state))) + if ((pe = tryMatch(size, scale, QIcon::Selected, state))) break; - if ((pe = tryMatch(size, QIcon::Disabled, oppositeState))) + if ((pe = tryMatch(size, scale, QIcon::Disabled, oppositeState))) break; - if ((pe = tryMatch(size, QIcon::Selected, oppositeState))) + if ((pe = tryMatch(size, scale, QIcon::Selected, oppositeState))) break; } @@ -272,8 +291,14 @@ QPixmapIconEngineEntry *QPixmapIconEngine::bestMatch(const QSize &size, QIcon::M QPixmap QPixmapIconEngine::pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) { + return scaledPixmap(size, mode, state, 1.0); +} + +QPixmap QPixmapIconEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) +{ + QPixmap pm; - QPixmapIconEngineEntry *pe = bestMatch(size, mode, state, false); + QPixmapIconEngineEntry *pe = bestMatch(size, scale, mode, state, false); if (pe) pm = pe->pixmap; @@ -332,7 +357,13 @@ QPixmap QPixmapIconEngine::pixmap(const QSize &size, QIcon::Mode mode, QIcon::St QSize QPixmapIconEngine::actualSize(const QSize &size, QIcon::Mode mode, QIcon::State state) { QSize actualSize; - if (QPixmapIconEngineEntry *pe = bestMatch(size, mode, state, true)) + + // The returned actiual size is the size in device independent pixels, + // so we limit the search to scale 1 and assume that e.g. @2x versions + // does not proviode extra actual sizes not also provided by the 1x versions. + qreal scale = 1; + + if (QPixmapIconEngineEntry *pe = bestMatch(size, scale, mode, state, true)) actualSize = pe->size; if (actualSize.isNull()) @@ -361,8 +392,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(), mode, state); - if(pe && pe->size == pixmap.size()) { + QPixmapIconEngineEntry *pe = tryMatch(pixmap.size(), pixmap.devicePixelRatio(), mode, state); + if (pe && pe->size == pixmap.size() && pe->scale == pixmap.devicePixelRatio()) { pe->pixmap = pixmap; pe->fileName.clear(); } else { diff --git a/src/gui/image/qicon_p.h b/src/gui/image/qicon_p.h index 49b0ac0172..40cb0c7efa 100644 --- a/src/gui/image/qicon_p.h +++ b/src/gui/image/qicon_p.h @@ -83,15 +83,16 @@ public: struct QPixmapIconEngineEntry { - QPixmapIconEngineEntry():mode(QIcon::Normal), state(QIcon::Off){} + QPixmapIconEngineEntry():scale(1), mode(QIcon::Normal), state(QIcon::Off){} QPixmapIconEngineEntry(const QPixmap &pm, QIcon::Mode m = QIcon::Normal, QIcon::State s = QIcon::Off) - :pixmap(pm), size(pm.size()), mode(m), state(s){} + :pixmap(pm), size(pm.size()), scale(pm.devicePixelRatio()), mode(m), state(s){} QPixmapIconEngineEntry(const QString &file, const QSize &sz = QSize(), QIcon::Mode m = QIcon::Normal, QIcon::State s = QIcon::Off) - :fileName(file), size(sz), mode(m), state(s){} + :fileName(file), size(sz), scale(1), mode(m), state(s){} QPixmapIconEngineEntry(const QString &file, const QImage &image, QIcon::Mode m = QIcon::Normal, QIcon::State s = QIcon::Off); QPixmap pixmap; QString fileName; QSize size; + qreal scale; QIcon::Mode mode; QIcon::State state; bool isNull() const {return (fileName.isEmpty() && pixmap.isNull()); } @@ -99,12 +100,9 @@ struct QPixmapIconEngineEntry Q_DECLARE_TYPEINFO(QPixmapIconEngineEntry, Q_MOVABLE_TYPE); inline QPixmapIconEngineEntry::QPixmapIconEngineEntry(const QString &file, const QImage &image, QIcon::Mode m, QIcon::State s) - : fileName(file), size(image.size()), mode(m), state(s) + : fileName(file), size(image.size()), scale(image.devicePixelRatio()), mode(m), state(s) { pixmap.convertFromImage(image); - // Reset the devicePixelRatio. The pixmap may be loaded from a @2x file, - // but be used as a 1x pixmap by QIcon. - pixmap.setDevicePixelRatio(1.0); } class Q_GUI_EXPORT QPixmapIconEngine : public QIconEngine { @@ -114,19 +112,21 @@ public: ~QPixmapIconEngine(); void paint(QPainter *painter, const QRect &rect, QIcon::Mode mode, QIcon::State state) override; QPixmap pixmap(const QSize &size, QIcon::Mode mode, QIcon::State state) override; - QPixmapIconEngineEntry *bestMatch(const QSize &size, QIcon::Mode mode, QIcon::State state, bool sizeOnly); + QPixmap scaledPixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) override; + QPixmapIconEngineEntry *bestMatch(const QSize &size, qreal scale, QIcon::Mode mode, QIcon::State state, bool sizeOnly); QSize actualSize(const QSize &size, QIcon::Mode mode, QIcon::State state) override; QList availableSizes(QIcon::Mode mode, QIcon::State state) override; void addPixmap(const QPixmap &pixmap, QIcon::Mode mode, QIcon::State state) override; void addFile(const QString &fileName, const QSize &size, QIcon::Mode mode, QIcon::State state) override; + QString key() const override; QIconEngine *clone() const override; bool read(QDataStream &in) override; bool write(QDataStream &out) const override; private: - QPixmapIconEngineEntry *tryMatch(const QSize &size, QIcon::Mode mode, QIcon::State state); + QPixmapIconEngineEntry *tryMatch(const QSize &size, qreal scale, QIcon::Mode mode, QIcon::State state); QList pixmaps; friend Q_GUI_EXPORT QDataStream &operator<<(QDataStream &s, const QIcon &icon); diff --git a/tests/auto/gui/image/qiconhighdpi/tst_qiconhighdpi.cpp b/tests/auto/gui/image/qiconhighdpi/tst_qiconhighdpi.cpp index 52f38419c7..62878926fa 100644 --- a/tests/auto/gui/image/qiconhighdpi/tst_qiconhighdpi.cpp +++ b/tests/auto/gui/image/qiconhighdpi/tst_qiconhighdpi.cpp @@ -39,6 +39,8 @@ private slots: void initTestCase(); void fromTheme_data(); void fromTheme(); + void addPixmap_data(); + void addPixmap(); void ninePatch(); }; @@ -173,6 +175,49 @@ void tst_QIconHighDpi::fromTheme() QCOMPARE(pixmap.devicePixelRatio(), expectedDpr); } +void tst_QIconHighDpi::addPixmap_data() +{ + // fromTheme() and addPixmap() test the QIconLoaderEngine and QPixmapIconEngine + // QIcon backend, repsectively. We want these to have identical behavior and + // re-use the same test data here. + fromTheme_data(); +} + +void tst_QIconHighDpi::addPixmap() +{ + QFETCH(int, requestedSize); + QFETCH(qreal, requestedDpr); + QFETCH(int, expectedSize); + QFETCH(qreal, expectedDpr); + + QIcon icon; + + // manual pixmap adder for full control of devicePixelRatio + auto addPixmapWithDpr = [&icon](const QString &path, qreal dpr) { + QImage image(path); + image.setDevicePixelRatio(dpr); + icon.addPixmap(QPixmap::fromImage(image)); + }; + + addPixmapWithDpr(":icons/testtheme/16x16/actions/appointment-new.png", 1); + addPixmapWithDpr(":icons/testtheme/22x22/actions/appointment-new.png", 1); + addPixmapWithDpr(":icons/testtheme/22x22@2/actions/appointment-new.png", 2); + + QVERIFY(icon.availableSizes().contains(QSize(16, 16))); + QVERIFY(icon.availableSizes().contains(QSize(22, 22))); + + if (qstrcmp(QTest::currentDataTag(), "16x16,dpr=2") == 0) + QSKIP("broken corner case"); // expect 22x22, get 32x32 + if (qstrcmp(QTest::currentDataTag(), "44x44,dpr=1") == 0) + QSKIP("broken corner case"); // expect 44x44, get 22x22 + if (qstrcmp(QTest::currentDataTag(), "8x8,dpr=3") == 0) + QSKIP("broken corner case");// expect 22x22, get 24x24 + + const QPixmap pixmap = icon.pixmap(QSize(requestedSize, requestedSize), requestedDpr); + QCOMPARE(pixmap.size(), QSize(expectedSize, expectedSize)); + QCOMPARE(pixmap.devicePixelRatio(), expectedDpr); +} + void tst_QIconHighDpi::ninePatch() { const QIcon icon(":/icons/misc/button.9.png");