From 066cf0a0d7b46c60fccb945dbc0e9f858c4d6967 Mon Sep 17 00:00:00 2001 From: Weng Xuetian Date: Sat, 27 Feb 2016 10:40:54 -0800 Subject: [PATCH] Fix svg iconPath resolving in IconItem This fixes iconPath is not correctly set when its size changes. REVIEW: 127201 --- .../hicolor/22x22/apps/bug359388.svg | 0 .../22/tst-plasma-framework-test-icon.svg | 72 ++ .../32/tst-plasma-framework-test-icon.svg | 763 ++++++++++++++++++ .../icons/test-theme/apps/48/konversation.svg | 317 ++++++++ autotests/data/icons/test-theme/index.theme | 39 + autotests/iconitemtest.cpp | 59 +- autotests/iconitemtest.h | 7 + src/declarativeimports/core/iconitem.cpp | 8 +- 8 files changed, 1256 insertions(+), 9 deletions(-) rename autotests/data/{icons => bug359388}/hicolor/22x22/apps/bug359388.svg (100%) create mode 100644 autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg create mode 100644 autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg create mode 100644 autotests/data/icons/test-theme/apps/48/konversation.svg create mode 100644 autotests/data/icons/test-theme/index.theme diff --git a/autotests/data/icons/hicolor/22x22/apps/bug359388.svg b/autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg similarity index 100% rename from autotests/data/icons/hicolor/22x22/apps/bug359388.svg rename to autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg diff --git a/autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg b/autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg new file mode 100644 index 000000000..8739af5a6 --- /dev/null +++ b/autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg @@ -0,0 +1,72 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + + + diff --git a/autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg b/autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg new file mode 100644 index 000000000..940d46d60 --- /dev/null +++ b/autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg @@ -0,0 +1,763 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Plasm + + + + + + + + + + diff --git a/autotests/data/icons/test-theme/apps/48/konversation.svg b/autotests/data/icons/test-theme/apps/48/konversation.svg new file mode 100644 index 000000000..68027b883 --- /dev/null +++ b/autotests/data/icons/test-theme/apps/48/konversation.svg @@ -0,0 +1,317 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + image/svg+xml + + + + + + + + + + + + diff --git a/autotests/data/icons/test-theme/index.theme b/autotests/data/icons/test-theme/index.theme new file mode 100644 index 000000000..52a72759d --- /dev/null +++ b/autotests/data/icons/test-theme/index.theme @@ -0,0 +1,39 @@ +[Icon Theme] +Name=Unittest Theme + +DisplayDepth=32 + +Inherits=breeze + +DesktopDefault=48 +DesktopSizes=16,22,32,48,64,128,256 +ToolbarDefault=22 +ToolbarSizes=16,22,32,48 +MainToolbarDefault=22 +MainToolbarSizes=16,22,32,48 +SmallDefault=16 +SmallSizes=16,22,32,48 +PanelDefault=32 +PanelSizes=16,22,32,48,64,128,256 +DialogDefault=32 +DialogSizes=16,22,32,48,64,128,256 + +########## Directories +########## ordered by category and alphabetically + +Directories=apps/22,apps/32 + +[apps/22] +Size=22 +Context=Applications +Type=Fixed + +[apps/32] +Size=32 +Context=Applications +Type=Fixed + +[apps/48] +Size=48 +Context=Applications +Type=Fixed diff --git a/autotests/iconitemtest.cpp b/autotests/iconitemtest.cpp index ea98c9df6..9b3667098 100644 --- a/autotests/iconitemtest.cpp +++ b/autotests/iconitemtest.cpp @@ -46,6 +46,26 @@ static bool imageIsEmpty(const QImage &img) void IconItemTest::initTestCase() { + // make our theme in search path + qputenv("XDG_DATA_DIRS", + qgetenv("XDG_DATA_DIRS") + ":" + QFINDTESTDATA("data").toLocal8Bit()); + + // set default icon theme to test-theme + QStandardPaths::setTestModeEnabled(true); + + QString configPath = QStandardPaths::writableLocation(QStandardPaths::GenericConfigLocation); + + if(!QDir(configPath).mkpath(QStringLiteral("."))) { + qFatal("Failed to create test configuration directory."); + } + + QFile::remove(configPath); + + KConfigGroup cg(KSharedConfig::openConfig(), "Icons"); + cg.writeEntry("Theme", "test-theme"); + KConfigGroup plasmaConfig(KSharedConfig::openConfig("plasmarc"), "Theme"); + plasmaConfig.writeEntry("name", "default"); + m_view = new QQuickView(); m_view->setSource(QUrl::fromLocalFile(QFINDTESTDATA("data/view.qml"))); m_view->show(); @@ -88,6 +108,11 @@ QImage IconItemTest::grabImage(QQuickItem *item) return grab->image(); } +Plasma::Svg *IconItemTest::findPlasmaSvg(QQuickItem *item) +{ + return item->findChild(); +} + // ------ Tests void IconItemTest::invalidIcon() { @@ -106,7 +131,7 @@ void IconItemTest::invalidIcon() void IconItemTest::usesPlasmaTheme() { Plasma::Theme theme; - if (!theme.themeName().startsWith("breeze")) { + if (!theme.themeName().startsWith("default")) { QSKIP("Current Plasma theme is not Breeze."); } @@ -132,7 +157,7 @@ void IconItemTest::usesPlasmaTheme() img1 = grabImage(item2); // This depends on konversation icon being different in Plasma Breeze theme - // and normal Breeze icon theme + // and our test icon theme QVERIFY(img1 != img2); } @@ -191,23 +216,47 @@ void IconItemTest::bug_359388() { QString name("bug359388"); KIconLoader iconLoader("tst_plasma-framework"); + QIcon customThemeIcon(new KIconEngine(name, &iconLoader)); if (iconLoader.hasIcon(name)) { QSKIP("Current icon theme has 'bug359388' icon."); } - iconLoader.addAppDir("tst_plasma-framework", QFINDTESTDATA("data/icons")); - QIcon customThemeIcon(new KIconEngine(name, &iconLoader)); + iconLoader.addAppDir("tst_plasma-framework", QFINDTESTDATA("data/bug359388")); QQuickItem *item1 = createIconItem(); item1->setProperty("source", customThemeIcon); QVERIFY(item1->property("valid").toBool()); QQuickItem *item2 = createIconItem(); - item2->setProperty("source", QIcon(QFINDTESTDATA("data/icons/hicolor/22x22/apps/" + name + ".svg"))); + item2->setProperty("source", QIcon(QFINDTESTDATA("data/bug359388/hicolor/22x22/apps/" + name + ".svg"))); QVERIFY(item2->property("valid").toBool()); QCOMPARE(grabImage(item1), grabImage(item2)); } +void IconItemTest::loadSvg() +{ + QString name("tst-plasma-framework-test-icon"); + + QQuickItem *item = createIconItem(); + item->setProperty("animated", false); + item->setSize(QSize(22, 22)); + item->setProperty("source", name); + QVERIFY(item->property("valid").toBool()); + + Plasma::Svg *svg; + svg = findPlasmaSvg(item); + Q_ASSERT(svg); + QCOMPARE(svg->imagePath(), QFINDTESTDATA("data/icons/test-theme/apps/22/" + name + ".svg")); + + // we only have 32x32 and 22x22 version in the theme, thus 32x32 is a better match. + item->setSize(QSize(64, 64)); + // just to update the icon + grabImage(item); + svg = findPlasmaSvg(item); + Q_ASSERT(svg); + QCOMPARE(svg->imagePath(), QFINDTESTDATA("data/icons/test-theme/apps/32/" + name + ".svg")); +} + QTEST_MAIN(IconItemTest) diff --git a/autotests/iconitemtest.h b/autotests/iconitemtest.h index 9fd3063c2..b27680a8f 100644 --- a/autotests/iconitemtest.h +++ b/autotests/iconitemtest.h @@ -23,6 +23,11 @@ #include #include +namespace Plasma +{ +class Svg; +} + class IconItemTest : public QObject { Q_OBJECT @@ -36,10 +41,12 @@ private Q_SLOTS: void animation(); void animationAfterHide(); void bug_359388(); + void loadSvg(); private: QQuickItem *createIconItem(); QImage grabImage(QQuickItem *item); + Plasma::Svg *findPlasmaSvg(QQuickItem *item); QQuickView *m_view; }; diff --git a/src/declarativeimports/core/iconitem.cpp b/src/declarativeimports/core/iconitem.cpp index c65a9a7b0..95528474b 100644 --- a/src/declarativeimports/core/iconitem.cpp +++ b/src/declarativeimports/core/iconitem.cpp @@ -134,9 +134,9 @@ void IconItem::setSource(const QVariant &source) const auto *iconTheme = KIconLoader::global()->theme(); QString iconPath; if (iconTheme) { - iconTheme->iconPath(sourceString + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest); + iconPath = iconTheme->iconPath(sourceString + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest); if (iconPath.isEmpty()) { - iconPath = iconTheme->iconPath(sourceString + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest); + iconPath = iconTheme->iconPath(sourceString + QLatin1String(".svgz"), qMin(width(), height()), KIconLoader::MatchBest); } } else { qWarning() << "KIconLoader has no theme set"; @@ -412,9 +412,9 @@ void IconItem::loadPixmap() const auto *iconTheme = KIconLoader::global()->theme(); QString iconPath; if (iconTheme) { - QString iconPath = iconTheme->iconPath(source().toString() + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest); + iconPath = iconTheme->iconPath(source().toString() + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest); if (iconPath.isEmpty()) { - iconPath = iconTheme->iconPath(source().toString() + QLatin1String(".svg"), qMin(width(), height()), KIconLoader::MatchBest); + iconPath = iconTheme->iconPath(source().toString() + QLatin1String(".svgz"), qMin(width(), height()), KIconLoader::MatchBest); } } else { qWarning() << "KIconLoader has no theme set";