From 51e28dc54fab3caa9e2b3bea4fd7a2fb5724d7c7 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Mon, 6 Sep 2021 20:40:17 +0200 Subject: QIconLoader: code tidies Turn QThemeIconEntries into an owning container (std::vector of unique_ptr), so that code using QThemeIconInfo doesn't have to manage ownership (and forget to do so, and cause bugs like QTBUG-93050). The fallout is mostly on isEmpty() vs empty(); as drive-by fixes: * use auto; * use make_unique (no raw news); * turn a few indexed loops into range-based ones; * streamline an if-else-if chain; * turn a !(a == b) condition into a != b. Change-Id: Ie3ac9de57c80ed3184ec0d15c847f81306ef48ca Reviewed-by: Volker Hilsheimer Reviewed-by: Qt CI Bot --- src/gui/image/qiconloader.cpp | 70 +++++++++++++++++-------------------------- src/gui/image/qiconloader_p.h | 5 +++- 2 files changed, 32 insertions(+), 43 deletions(-) (limited to 'src/gui/image') diff --git a/src/gui/image/qiconloader.cpp b/src/gui/image/qiconloader.cpp index e5aa7cd0ac..807ed4921c 100644 --- a/src/gui/image/qiconloader.cpp +++ b/src/gui/image/qiconloader.cpp @@ -446,7 +446,7 @@ QThemeIconInfo QIconLoader::findIconHelper(const QString &themeName, QStringView iconNameFallback(iconName); // Iterate through all icon's fallbacks in current theme - while (info.entries.isEmpty()) { + while (info.entries.empty()) { const QString svgIconName = iconNameFallback + QLatin1String(".svg"); const QString pngIconName = iconNameFallback + QLatin1String(".png"); @@ -481,25 +481,25 @@ QThemeIconInfo QIconLoader::findIconHelper(const QString &themeName, const QString subDir = contentDir + dirInfo.path + QLatin1Char('/'); const QString pngPath = subDir + pngIconName; if (QFile::exists(pngPath)) { - PixmapEntry *iconEntry = new PixmapEntry; + auto iconEntry = std::make_unique(); iconEntry->dir = dirInfo; iconEntry->filename = pngPath; // Notice we ensure that pixmap entries always come before // scalable to preserve search order afterwards - info.entries.prepend(iconEntry); + info.entries.insert(info.entries.begin(), std::move(iconEntry)); } else if (m_supportsSvg) { const QString svgPath = subDir + svgIconName; if (QFile::exists(svgPath)) { - ScalableEntry *iconEntry = new ScalableEntry; + auto iconEntry = std::make_unique(); iconEntry->dir = dirInfo; iconEntry->filename = svgPath; - info.entries.append(iconEntry); + info.entries.push_back(std::move(iconEntry)); } } } } - if (!info.entries.isEmpty()) { + if (!info.entries.empty()) { info.iconName = iconNameFallback.toString(); break; } @@ -512,7 +512,7 @@ QThemeIconInfo QIconLoader::findIconHelper(const QString &themeName, iconNameFallback.truncate(indexOfDash); } - if (info.entries.isEmpty()) { + if (info.entries.empty()) { const QStringList parents = theme.parents(); // Search recursively through inherited themes for (int i = 0 ; i < parents.size() ; ++i) { @@ -522,7 +522,7 @@ QThemeIconInfo QIconLoader::findIconHelper(const QString &themeName, if (!visited.contains(parentTheme)) // guard against recursion info = findIconHelper(parentTheme, iconName, visited); - if (!info.entries.isEmpty()) // success + if (!info.entries.empty()) // success break; } } @@ -540,29 +540,28 @@ QThemeIconInfo QIconLoader::lookupFallbackIcon(const QString &iconName) const const auto searchPaths = QIcon::fallbackSearchPaths(); for (const QString &iconDir: searchPaths) { QDir currentDir(iconDir); + std::unique_ptr iconEntry; if (currentDir.exists(pngIconName)) { - PixmapEntry *iconEntry = new PixmapEntry; + iconEntry = std::make_unique(); iconEntry->dir.type = QIconDirInfo::Fallback; iconEntry->filename = currentDir.filePath(pngIconName); - info.entries.append(iconEntry); - break; } else if (currentDir.exists(xpmIconName)) { - PixmapEntry *iconEntry = new PixmapEntry; + iconEntry = std::make_unique(); iconEntry->dir.type = QIconDirInfo::Fallback; iconEntry->filename = currentDir.filePath(xpmIconName); - info.entries.append(iconEntry); - break; } else if (m_supportsSvg && currentDir.exists(svgIconName)) { - ScalableEntry *iconEntry = new ScalableEntry; + iconEntry = std::make_unique(); iconEntry->dir.type = QIconDirInfo::Fallback; iconEntry->filename = currentDir.filePath(svgIconName); - info.entries.append(iconEntry); + } + if (iconEntry) { + info.entries.push_back(std::move(iconEntry)); break; } } - if (!info.entries.isEmpty()) + if (!info.entries.empty()) info.iconName = iconName; return info; @@ -572,8 +571,8 @@ QThemeIconInfo QIconLoader::loadIcon(const QString &name) const { if (!themeName().isEmpty()) { QStringList visited; - const QThemeIconInfo iconInfo = findIconHelper(themeName(), name, visited); - if (!iconInfo.entries.isEmpty()) + QThemeIconInfo iconInfo = findIconHelper(themeName(), name, visited); + if (!iconInfo.entries.empty()) return iconInfo; return lookupFallbackIcon(name); @@ -591,10 +590,7 @@ QIconLoaderEngine::QIconLoaderEngine(const QString& iconName) { } -QIconLoaderEngine::~QIconLoaderEngine() -{ - qDeleteAll(m_info.entries); -} +QIconLoaderEngine::~QIconLoaderEngine() = default; QIconLoaderEngine::QIconLoaderEngine(const QIconLoaderEngine &other) : QIconEngine(other), @@ -621,18 +617,13 @@ bool QIconLoaderEngine::write(QDataStream &out) const bool QIconLoaderEngine::hasIcon() const { - return !(m_info.entries.isEmpty()); + return !(m_info.entries.empty()); } // 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(); - - Q_ASSERT(m_info.entries.size() == 0); + if (QIconLoader::instance()->themeKey() != m_key) { m_info = QIconLoader::instance()->loadIcon(m_iconName); m_key = QIconLoader::instance()->themeKey(); } @@ -711,25 +702,21 @@ QIconLoaderEngineEntry *QIconLoaderEngine::entryForSize(const QThemeIconInfo &in // Note that m_info.entries are sorted so that png-files // come first - const int numEntries = info.entries.size(); - // Search for exact matches first - for (int i = 0; i < numEntries; ++i) { - QIconLoaderEngineEntry *entry = info.entries.at(i); + for (const auto &entry : info.entries) { if (directoryMatchesSize(entry->dir, iconsize, scale)) { - return entry; + return entry.get(); } } // Find the minimum distance icon int minimalSize = INT_MAX; QIconLoaderEngineEntry *closestMatch = nullptr; - for (int i = 0; i < numEntries; ++i) { - QIconLoaderEngineEntry *entry = info.entries.at(i); + for (const auto &entry : info.entries) { int distance = directorySizeDistance(entry->dir, iconsize, scale); if (distance < minimalSize) { minimalSize = distance; - closestMatch = entry; + closestMatch = entry.get(); } } return closestMatch; @@ -840,7 +827,7 @@ QString QIconLoaderEngine::iconName() bool QIconLoaderEngine::isNull() { ensureLoaded(); - return m_info.entries.isEmpty(); + return m_info.entries.empty(); } QPixmap QIconLoaderEngine::scaledPixmap(const QSize &size, QIcon::Mode mode, QIcon::State state, qreal scale) @@ -856,13 +843,12 @@ QList QIconLoaderEngine::availableSizes(QIcon::Mode mode, QIcon::State st Q_UNUSED(mode); Q_UNUSED(state); ensureLoaded(); - const int N = m_info.entries.size(); + const qsizetype N = qsizetype(m_info.entries.size()); QList sizes; sizes.reserve(N); // Gets all sizes from the DirectoryInfo entries - for (int i = 0; i < N; ++i) { - const QIconLoaderEngineEntry *entry = m_info.entries.at(i); + for (const auto &entry : m_info.entries) { if (entry->dir.type == QIconDirInfo::Fallback) { sizes.append(QIcon(entry->filename).availableSizes()); } else { diff --git a/src/gui/image/qiconloader_p.h b/src/gui/image/qiconloader_p.h index d02b2da2cc..ad085459e3 100644 --- a/src/gui/image/qiconloader_p.h +++ b/src/gui/image/qiconloader_p.h @@ -63,6 +63,9 @@ #include #include +#include +#include + QT_BEGIN_NAMESPACE class QIconLoader; @@ -111,7 +114,7 @@ struct PixmapEntry : public QIconLoaderEngineEntry QPixmap basePixmap; }; -typedef QList QThemeIconEntries; +using QThemeIconEntries = std::vector>; struct QThemeIconInfo { -- cgit v1.2.3