summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRuslan Nigmatullin <euroelessar@yandex.ru>2014-03-31 03:54:06 +0400
committerKonstantin Ritt <ritt.ks@gmail.com>2015-02-02 23:28:19 +0000
commita40d390a3bbc1919843000b427a168180b83a344 (patch)
tree05121f7154fed3bc8a167460f9cc8aca0687995f
parenta9066ecf646134009494f68a0365d4549d999c66 (diff)
Fixed icons lookup in QIcon::fromTheme
This commit fixes incorrect logic of icons' lookup if there are fallbacks or more than one theme's directory. According to Icon Theme Specification, Directory Layout section, theme can be spread across several base directories by having subdirectories of the same name. This makes possible to extend system themes by application-specific icons without making of collisions with other applications. According to Icon Naming Specification, Icon Naming Guidelines section, icon name may contain dashes to separate levels of specificity in icon names. This makes possible to set in application very specific icon which may be not in every theme. So it can fallback to less specific one. Change-Id: Iafc813902a3646be56e8f1d3a2fdbf8fd32ac542 Reviewed-by: Friedemann Kleint <Friedemann.Kleint@theqtcompany.com> Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
-rw-r--r--src/gui/image/qicon.cpp2
-rw-r--r--src/gui/image/qiconloader.cpp124
-rw-r--r--src/gui/image/qiconloader_p.h20
-rw-r--r--tests/auto/gui/image/qicon/qicon.pro2
-rw-r--r--tests/auto/gui/image/qicon/second_icons/testtheme/32x32/actions/appointment-new.png (renamed from tests/auto/gui/image/qicon/icons/testtheme/32x32/actions/appointment-new.png)bin2399 -> 2399 bytes
-rw-r--r--tests/auto/gui/image/qicon/tst_qicon.cpp18
-rw-r--r--tests/auto/gui/image/qicon/tst_qicon.qrc2
7 files changed, 105 insertions, 63 deletions
diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp
index d885729cbd..1b99a5d8dc 100644
--- a/src/gui/image/qicon.cpp
+++ b/src/gui/image/qicon.cpp
@@ -1197,7 +1197,7 @@ bool QIcon::hasThemeIcon(const QString &name)
{
QIcon icon = fromTheme(name);
- return !icon.isNull();
+ return icon.name() == name;
}
diff --git a/src/gui/image/qiconloader.cpp b/src/gui/image/qiconloader.cpp
index 06491f1155..4510125816 100644
--- a/src/gui/image/qiconloader.cpp
+++ b/src/gui/image/qiconloader.cpp
@@ -172,11 +172,15 @@ QIconTheme::QIconTheme(const QString &themeName)
for ( int i = 0 ; i < iconDirs.size() ; ++i) {
QDir iconDir(iconDirs[i]);
QString themeDir = iconDir.path() + QLatin1Char('/') + themeName;
- themeIndex.setFileName(themeDir + QLatin1String("/index.theme"));
- if (themeIndex.exists()) {
- m_contentDir = themeDir;
- m_valid = true;
- break;
+ QFileInfo themeDirInfo(themeDir);
+
+ if (themeDirInfo.isDir())
+ m_contentDirs << themeDir;
+
+ if (!m_valid) {
+ themeIndex.setFileName(themeDir + QLatin1String("/index.theme"));
+ if (themeIndex.exists())
+ m_valid = true;
}
}
#ifndef QT_NO_SETTINGS
@@ -239,11 +243,11 @@ QIconTheme::QIconTheme(const QString &themeName)
#endif //QT_NO_SETTINGS
}
-QThemeIconEntries QIconLoader::findIconHelper(const QString &themeName,
- const QString &iconName,
- QStringList &visited) const
+QThemeIconInfo QIconLoader::findIconHelper(const QString &themeName,
+ const QString &iconName,
+ QStringList &visited) const
{
- QThemeIconEntries entries;
+ QThemeIconInfo info;
Q_ASSERT(!themeName.isEmpty());
QPixmap pixmap;
@@ -260,34 +264,54 @@ QThemeIconEntries QIconLoader::findIconHelper(const QString &themeName,
themeList.insert(themeName, theme);
}
- QString contentDir = theme.contentDir() + QLatin1Char('/');
+ const QStringList contentDirs = theme.contentDirs();
const QVector<QIconDirInfo> subDirs = theme.keyList();
- const QString svgext(QLatin1String(".svg"));
- const QString pngext(QLatin1String(".png"));
-
- // Add all relevant files
- for (int i = 0; i < subDirs.size() ; ++i) {
- const QIconDirInfo &dirInfo = subDirs.at(i);
- QString subdir = dirInfo.path;
- QDir currentDir(contentDir + subdir);
- if (currentDir.exists(iconName + pngext)) {
- PixmapEntry *iconEntry = new PixmapEntry;
- iconEntry->dir = dirInfo;
- iconEntry->filename = currentDir.filePath(iconName + pngext);
- // Notice we ensure that pixmap entries always come before
- // scalable to preserve search order afterwards
- entries.prepend(iconEntry);
- } else if (m_supportsSvg &&
- currentDir.exists(iconName + svgext)) {
- ScalableEntry *iconEntry = new ScalableEntry;
- iconEntry->dir = dirInfo;
- iconEntry->filename = currentDir.filePath(iconName + svgext);
- entries.append(iconEntry);
+ QString iconNameFallback = iconName;
+
+ // Iterate through all icon's fallbacks in current theme
+ while (info.entries.isEmpty()) {
+ const QString svgIconName = iconNameFallback + QLatin1String(".svg");
+ const QString pngIconName = iconNameFallback + QLatin1String(".png");
+
+ // Add all relevant files
+ for (int i = 0; i < contentDirs.size(); ++i) {
+ QString contentDir = contentDirs.at(i) + QLatin1Char('/');
+ for (int j = 0; j < subDirs.size() ; ++j) {
+ const QIconDirInfo &dirInfo = subDirs.at(j);
+ QString subdir = dirInfo.path;
+ QDir currentDir(contentDir + subdir);
+ if (currentDir.exists(pngIconName)) {
+ PixmapEntry *iconEntry = new PixmapEntry;
+ iconEntry->dir = dirInfo;
+ iconEntry->filename = currentDir.filePath(pngIconName);
+ // Notice we ensure that pixmap entries always come before
+ // scalable to preserve search order afterwards
+ info.entries.prepend(iconEntry);
+ } else if (m_supportsSvg &&
+ currentDir.exists(svgIconName)) {
+ ScalableEntry *iconEntry = new ScalableEntry;
+ iconEntry->dir = dirInfo;
+ iconEntry->filename = currentDir.filePath(svgIconName);
+ info.entries.append(iconEntry);
+ }
+ }
+ }
+
+ if (!info.entries.isEmpty()) {
+ info.iconName = iconNameFallback;
+ break;
}
+
+ // If it's possible - find next fallback for the icon
+ const int indexOfDash = iconNameFallback.lastIndexOf(QLatin1Char('-'));
+ if (indexOfDash == -1)
+ break;
+
+ iconNameFallback.truncate(indexOfDash);
}
- if (entries.isEmpty()) {
+ if (info.entries.isEmpty()) {
const QStringList parents = theme.parents();
// Search recursively through inherited themes
for (int i = 0 ; i < parents.size() ; ++i) {
@@ -295,23 +319,23 @@ QThemeIconEntries QIconLoader::findIconHelper(const QString &themeName,
const QString parentTheme = parents.at(i).trimmed();
if (!visited.contains(parentTheme)) // guard against recursion
- entries = findIconHelper(parentTheme, iconName, visited);
+ info = findIconHelper(parentTheme, iconName, visited);
- if (!entries.isEmpty()) // success
+ if (!info.entries.isEmpty()) // success
break;
}
}
- return entries;
+ return info;
}
-QThemeIconEntries QIconLoader::loadIcon(const QString &name) const
+QThemeIconInfo QIconLoader::loadIcon(const QString &name) const
{
if (!themeName().isEmpty()) {
QStringList visited;
return findIconHelper(themeName(), name, visited);
}
- return QThemeIconEntries();
+ return QThemeIconInfo();
}
@@ -325,7 +349,7 @@ QIconLoaderEngine::QIconLoaderEngine(const QString& iconName)
QIconLoaderEngine::~QIconLoaderEngine()
{
- qDeleteAll(m_entries);
+ qDeleteAll(m_info.entries);
}
QIconLoaderEngine::QIconLoaderEngine(const QIconLoaderEngine &other)
@@ -353,17 +377,19 @@ bool QIconLoaderEngine::write(QDataStream &out) const
bool QIconLoaderEngine::hasIcon() const
{
- return !(m_entries.isEmpty());
+ return !(m_info.entries.isEmpty());
}
// Lazily load the icon
void QIconLoaderEngine::ensureLoaded()
{
if (!(QIconLoader::instance()->themeKey() == m_key)) {
+ qDeleteAll(m_info.entries);
+ m_info.entries.clear();
+ m_info.iconName.clear();
- qDeleteAll(m_entries);
-
- m_entries = QIconLoader::instance()->loadIcon(m_iconName);
+ Q_ASSERT(m_info.entries.size() == 0);
+ m_info = QIconLoader::instance()->loadIcon(m_iconName);
m_key = QIconLoader::instance()->themeKey();
}
}
@@ -433,14 +459,14 @@ QIconLoaderEngineEntry *QIconLoaderEngine::entryForSize(const QSize &size)
{
int iconsize = qMin(size.width(), size.height());
- // Note that m_entries are sorted so that png-files
+ // Note that m_info.entries are sorted so that png-files
// come first
- const int numEntries = m_entries.size();
+ const int numEntries = m_info.entries.size();
// Search for exact matches first
for (int i = 0; i < numEntries; ++i) {
- QIconLoaderEngineEntry *entry = m_entries.at(i);
+ QIconLoaderEngineEntry *entry = m_info.entries.at(i);
if (directoryMatchesSize(entry->dir, iconsize)) {
return entry;
}
@@ -450,7 +476,7 @@ QIconLoaderEngineEntry *QIconLoaderEngine::entryForSize(const QSize &size)
int minimalSize = INT_MAX;
QIconLoaderEngineEntry *closestMatch = 0;
for (int i = 0; i < numEntries; ++i) {
- QIconLoaderEngineEntry *entry = m_entries.at(i);
+ QIconLoaderEngineEntry *entry = m_info.entries.at(i);
int distance = directorySizeDistance(entry->dir, iconsize);
if (distance < minimalSize) {
minimalSize = distance;
@@ -554,13 +580,13 @@ void QIconLoaderEngine::virtual_hook(int id, void *data)
{
QIconEngine::AvailableSizesArgument &arg
= *reinterpret_cast<QIconEngine::AvailableSizesArgument*>(data);
- const int N = m_entries.size();
+ const int N = m_info.entries.size();
QList<QSize> sizes;
sizes.reserve(N);
// Gets all sizes from the DirectoryInfo entries
for (int i = 0; i < N; ++i) {
- int size = m_entries.at(i)->dir.size;
+ int size = m_info.entries.at(i)->dir.size;
sizes.append(QSize(size, size));
}
arg.sizes.swap(sizes); // commit
@@ -569,7 +595,7 @@ void QIconLoaderEngine::virtual_hook(int id, void *data)
case QIconEngine::IconNameHook:
{
QString &name = *reinterpret_cast<QString*>(data);
- name = m_iconName;
+ name = m_info.iconName;
}
break;
default:
diff --git a/src/gui/image/qiconloader_p.h b/src/gui/image/qiconloader_p.h
index 50a6e6d3a7..8ee730f307 100644
--- a/src/gui/image/qiconloader_p.h
+++ b/src/gui/image/qiconloader_p.h
@@ -106,6 +106,12 @@ struct PixmapEntry : public QIconLoaderEngineEntry
typedef QList<QIconLoaderEngineEntry*> QThemeIconEntries;
+struct QThemeIconInfo
+{
+ QThemeIconEntries entries;
+ QString iconName;
+};
+
class QIconLoaderEngine : public QIconEngine
{
public:
@@ -126,7 +132,7 @@ private:
void virtual_hook(int id, void *data);
QIconLoaderEngineEntry *entryForSize(const QSize &size);
QIconLoaderEngine(const QIconLoaderEngine &other);
- QThemeIconEntries m_entries;
+ QThemeIconInfo m_info;
QString m_iconName;
uint m_key;
@@ -140,11 +146,11 @@ public:
QIconTheme() : m_valid(false) {}
QStringList parents() { return m_parents; }
QVector<QIconDirInfo> keyList() { return m_keyList; }
- QString contentDir() { return m_contentDir; }
+ QStringList contentDirs() { return m_contentDirs; }
bool isValid() { return m_valid; }
private:
- QString m_contentDir;
+ QStringList m_contentDirs;
QVector<QIconDirInfo> m_keyList;
QStringList m_parents;
bool m_valid;
@@ -154,7 +160,7 @@ class Q_GUI_EXPORT QIconLoader
{
public:
QIconLoader();
- QThemeIconEntries loadIcon(const QString &iconName) const;
+ QThemeIconInfo loadIcon(const QString &iconName) const;
uint themeKey() const { return m_themeKey; }
QString themeName() const { return m_userTheme.isEmpty() ? m_systemTheme : m_userTheme; }
@@ -169,9 +175,9 @@ public:
void ensureInitialized();
private:
- QThemeIconEntries findIconHelper(const QString &themeName,
- const QString &iconName,
- QStringList &visited) const;
+ QThemeIconInfo findIconHelper(const QString &themeName,
+ const QString &iconName,
+ QStringList &visited) const;
uint m_themeKey;
bool m_supportsSvg;
bool m_initialized;
diff --git a/tests/auto/gui/image/qicon/qicon.pro b/tests/auto/gui/image/qicon/qicon.pro
index f48fe90dd8..6ff20ec8fa 100644
--- a/tests/auto/gui/image/qicon/qicon.pro
+++ b/tests/auto/gui/image/qicon/qicon.pro
@@ -7,4 +7,4 @@ qtHaveModule(widgets): QT += widgets
SOURCES += tst_qicon.cpp
RESOURCES = tst_qicon.qrc
-TESTDATA += icons/* *.png *.svg *.svgz
+TESTDATA += icons/* second_icons/* *.png *.svg *.svgz
diff --git a/tests/auto/gui/image/qicon/icons/testtheme/32x32/actions/appointment-new.png b/tests/auto/gui/image/qicon/second_icons/testtheme/32x32/actions/appointment-new.png
index 85daef3b0b..85daef3b0b 100644
--- a/tests/auto/gui/image/qicon/icons/testtheme/32x32/actions/appointment-new.png
+++ b/tests/auto/gui/image/qicon/second_icons/testtheme/32x32/actions/appointment-new.png
Binary files differ
diff --git a/tests/auto/gui/image/qicon/tst_qicon.cpp b/tests/auto/gui/image/qicon/tst_qicon.cpp
index 550dd5bc33..add6da9efb 100644
--- a/tests/auto/gui/image/qicon/tst_qicon.cpp
+++ b/tests/auto/gui/image/qicon/tst_qicon.cpp
@@ -559,10 +559,12 @@ void tst_QIcon::task184901_badCache()
void tst_QIcon::fromTheme()
{
- QString searchPath = QLatin1String(":/icons");
- QIcon::setThemeSearchPaths(QStringList() << searchPath);
- QVERIFY(QIcon::themeSearchPaths().size() == 1);
- QCOMPARE(searchPath, QIcon::themeSearchPaths()[0]);
+ QString firstSearchPath = QLatin1String(":/icons");
+ QString secondSearchPath = QLatin1String(":/second_icons");
+ QIcon::setThemeSearchPaths(QStringList() << firstSearchPath << secondSearchPath);
+ QVERIFY(QIcon::themeSearchPaths().size() == 2);
+ QCOMPARE(firstSearchPath, QIcon::themeSearchPaths()[0]);
+ QCOMPARE(secondSearchPath, QIcon::themeSearchPaths()[1]);
QString themeName("testtheme");
QIcon::setThemeName(themeName);
@@ -576,6 +578,14 @@ void tst_QIcon::fromTheme()
QVERIFY(appointmentIcon.availableSizes().contains(QSize(32, 32)));
QVERIFY(appointmentIcon.availableSizes().contains(QSize(22, 22)));
+ // Test fallback to less specific icon
+ QIcon specificAppointmentIcon = QIcon::fromTheme("appointment-new-specific");
+ QVERIFY(!QIcon::hasThemeIcon("appointment-new-specific"));
+ QVERIFY(QIcon::hasThemeIcon("appointment-new"));
+ QCOMPARE(specificAppointmentIcon.name(), QString::fromLatin1("appointment-new"));
+ QCOMPARE(specificAppointmentIcon.availableSizes(), appointmentIcon.availableSizes());
+ QCOMPARE(specificAppointmentIcon.pixmap(32).cacheKey(), appointmentIcon.pixmap(32).cacheKey());
+
// Test icon from parent theme
QIcon abIcon = QIcon::fromTheme("address-book-new");
QVERIFY(!abIcon.isNull());
diff --git a/tests/auto/gui/image/qicon/tst_qicon.qrc b/tests/auto/gui/image/qicon/tst_qicon.qrc
index dc11a87ddd..1505ca925b 100644
--- a/tests/auto/gui/image/qicon/tst_qicon.qrc
+++ b/tests/auto/gui/image/qicon/tst_qicon.qrc
@@ -5,7 +5,7 @@
<file>rect.png</file>
<file>./icons/testtheme/16x16/actions/appointment-new.png</file>
<file>./icons/testtheme/22x22/actions/appointment-new.png</file>
-<file>./icons/testtheme/32x32/actions/appointment-new.png</file>
+<file>./second_icons/testtheme/32x32/actions/appointment-new.png</file>
<file>./icons/testtheme/index.theme</file>
<file>./icons/testtheme/scalable/actions/svg-only.svg</file>
<file>./icons/themeparent/16x16/actions/address-book-new.png</file>