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 <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
bb10
Giuseppe D'Angelo 2021-09-06 20:40:17 +02:00
parent 33238ea2c6
commit 51e28dc54f
2 changed files with 32 additions and 43 deletions

View File

@ -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<PixmapEntry>();
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<ScalableEntry>();
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<QIconLoaderEngineEntry> iconEntry;
if (currentDir.exists(pngIconName)) {
PixmapEntry *iconEntry = new PixmapEntry;
iconEntry = std::make_unique<PixmapEntry>();
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<PixmapEntry>();
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<ScalableEntry>();
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<QSize> 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<QSize> 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 {

View File

@ -63,6 +63,9 @@
#include <QtCore/QList>
#include <QtCore/QTypeInfo>
#include <vector>
#include <memory>
QT_BEGIN_NAMESPACE
class QIconLoader;
@ -111,7 +114,7 @@ struct PixmapEntry : public QIconLoaderEngineEntry
QPixmap basePixmap;
};
typedef QList<QIconLoaderEngineEntry*> QThemeIconEntries;
using QThemeIconEntries = std::vector<std::unique_ptr<QIconLoaderEngineEntry>>;
struct QThemeIconInfo
{