Teach QPixmapIconEngine how to handle @Nx pixmaps

Bring QPixmapIconEngine on par with QIconLoaderEngine
when it comes to @Nx pixmap handling: Make the scale
factor a test parameter during icon lookup.

This allows storing e.g 16x16@1, 16x16@2, 16x16@3 versions
of a pixmap in the icon, and then having QIcon select
the correct one based on the target devicePixelRatio.

Extend the qiconhighdpi test to also cover QPixmapIconEngine,
via the addPixmap() API.

The corner cases of pixmap lookup can be much complicated.
QIconLoaderEngine and QPixmapIconEngine should ideally
have identical behavior in order to avoid surprises.

Change-Id: I17552cc61755bff9553c4a462e3983ac6759c13b
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
bb10
Morten Johan Sørvig 2020-08-12 15:19:54 +02:00
parent e6cba05b66
commit 46a552583f
3 changed files with 109 additions and 33 deletions

View File

@ -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<QSize> 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 {

View File

@ -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<QSize> 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<QPixmapIconEngineEntry> pixmaps;
friend Q_GUI_EXPORT QDataStream &operator<<(QDataStream &s, const QIcon &icon);

View File

@ -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");