From bce81fd9927eb8540c0029306ad87fcaa4a811ca Mon Sep 17 00:00:00 2001 From: Eike Ziller Date: Thu, 8 Apr 2021 16:31:31 +0200 Subject: [PATCH] Projects: Create QIcons in the UI thread Creating QIcons elsewhere is not safe because of image reader plugin loading and the pixmap cache. Fixes: QTCREATORBUG-25301 Change-Id: Ia22a0cd571f808d7f5c639353fdf2e548743f8ca Reviewed-by: Jarek Kobus --- .../cmakeprojectmanager/cmakeprojectnodes.cpp | 13 +-- .../fileapidataextractor.cpp | 3 +- .../cmakeprojectmanager/projecttreehelper.cpp | 8 +- .../project/projecttree/mesonprojectnodes.cpp | 9 +- src/plugins/projectexplorer/projectnodes.cpp | 86 +++++++++++++++++-- src/plugins/projectexplorer/projectnodes.h | 22 ++++- src/plugins/qbsprojectmanager/qbsnodes.cpp | 11 +-- .../qmlprojectmanager/qmlprojectnodes.cpp | 3 +- src/plugins/resourceeditor/resourcenode.cpp | 2 +- 9 files changed, 116 insertions(+), 41 deletions(-) diff --git a/src/plugins/cmakeprojectmanager/cmakeprojectnodes.cpp b/src/plugins/cmakeprojectmanager/cmakeprojectnodes.cpp index 211aca4065d..201e1ccca10 100644 --- a/src/plugins/cmakeprojectmanager/cmakeprojectnodes.cpp +++ b/src/plugins/cmakeprojectmanager/cmakeprojectnodes.cpp @@ -46,17 +46,14 @@ CMakeInputsNode::CMakeInputsNode(const Utils::FilePath &cmakeLists) : { setPriority(Node::DefaultPriority - 10); // Bottom most! setDisplayName(QCoreApplication::translate("CMakeFilesProjectNode", "CMake Modules")); - static const QIcon modulesIcon = Core::FileIconProvider::directoryIcon( - ProjectExplorer::Constants::FILEOVERLAY_MODULES); - setIcon(modulesIcon); + setIcon(DirectoryIcon(ProjectExplorer::Constants::FILEOVERLAY_MODULES)); setListInProject(false); } CMakeListsNode::CMakeListsNode(const Utils::FilePath &cmakeListPath) : ProjectExplorer::ProjectNode(cmakeListPath) { - static QIcon folderIcon = Core::FileIconProvider::directoryIcon(Constants::FILE_OVERLAY_CMAKE); - setIcon(folderIcon); + setIcon(DirectoryIcon(Constants::FILE_OVERLAY_CMAKE)); setListInProject(false); } @@ -74,9 +71,7 @@ CMakeProjectNode::CMakeProjectNode(const Utils::FilePath &directory) : ProjectExplorer::ProjectNode(directory) { setPriority(Node::DefaultProjectPriority + 1000); - static const QIcon productIcon = Core::FileIconProvider::directoryIcon( - ProjectExplorer::Constants::FILEOVERLAY_PRODUCT); - setIcon(productIcon); + setIcon(DirectoryIcon(ProjectExplorer::Constants::FILEOVERLAY_PRODUCT)); setListInProject(false); } @@ -90,7 +85,7 @@ CMakeTargetNode::CMakeTargetNode(const Utils::FilePath &directory, const QString { m_target = target; setPriority(Node::DefaultProjectPriority + 900); - setIcon(QIcon(":/projectexplorer/images/build.png")); // TODO: Use proper icon! + setIcon(":/projectexplorer/images/build.png"); // TODO: Use proper icon! setListInProject(false); setProductType(ProductType::Other); } diff --git a/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp b/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp index 1474a8da4da..12ab1ce1c1d 100644 --- a/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp +++ b/src/plugins/cmakeprojectmanager/fileapidataextractor.cpp @@ -474,7 +474,8 @@ FolderNode *createSourceGroupNode(const QString &sourceGroupName, if (!existingNode) { auto node = createCMakeVFolder(sourceDirectory, Node::DefaultFolderPriority + 5, p); node->setListInProject(false); - node->setIcon(QIcon::fromTheme("edit-copy", ::Utils::Icons::COPY.icon())); + node->setIcon( + [] { return QIcon::fromTheme("edit-copy", ::Utils::Icons::COPY.icon()); }); existingNode = node.get(); diff --git a/src/plugins/cmakeprojectmanager/projecttreehelper.cpp b/src/plugins/cmakeprojectmanager/projecttreehelper.cpp index df63f64bdd2..edd3dd3c77f 100644 --- a/src/plugins/cmakeprojectmanager/projecttreehelper.cpp +++ b/src/plugins/cmakeprojectmanager/projecttreehelper.cpp @@ -182,14 +182,12 @@ void addHeaderNodes(ProjectNode *root, if (root->isEmpty()) return; - static QIcon headerNodeIcon = Core::FileIconProvider::directoryIcon( - ProjectExplorer::Constants::FILEOVERLAY_H); auto headerNode = std::make_unique(root->filePath()); headerNode->setPriority(Node::DefaultPriority - 5); headerNode->setDisplayName( QCoreApplication::translate("CMakeProjectManager::Internal::ProjectTreeHelper", "")); - headerNode->setIcon(headerNodeIcon); + headerNode->setIcon(DirectoryIcon(ProjectExplorer::Constants::FILEOVERLAY_H)); // Add scanned headers: for (const FileNode *fn : allFiles) { @@ -212,15 +210,13 @@ void addFileSystemNodes(ProjectNode *root, const QList &allFil { QTC_ASSERT(root, return ); - static QIcon fileSystemNodeIcon = Core::FileIconProvider::directoryIcon( - ProjectExplorer::Constants::FILEOVERLAY_UNKNOWN); auto fileSystemNode = std::make_unique(root->filePath()); // just before special nodes like "CMake Modules" fileSystemNode->setPriority(Node::DefaultPriority - 6); fileSystemNode->setDisplayName( QCoreApplication::translate("CMakeProjectManager::Internal::ProjectTreeHelper", "")); - fileSystemNode->setIcon(fileSystemNodeIcon); + fileSystemNode->setIcon(DirectoryIcon(ProjectExplorer::Constants::FILEOVERLAY_UNKNOWN)); for (const FileNode *fn : allFiles) { if (!fn->filePath().isChildOf(root->filePath())) diff --git a/src/plugins/mesonprojectmanager/project/projecttree/mesonprojectnodes.cpp b/src/plugins/mesonprojectmanager/project/projecttree/mesonprojectnodes.cpp index 41d202026f8..f84ca006d10 100644 --- a/src/plugins/mesonprojectmanager/project/projecttree/mesonprojectnodes.cpp +++ b/src/plugins/mesonprojectmanager/project/projecttree/mesonprojectnodes.cpp @@ -38,18 +38,15 @@ namespace Internal { MesonProjectNode::MesonProjectNode(const Utils::FilePath &directory) : ProjectExplorer::ProjectNode{directory} { - static const auto MesonIcon = QIcon(Constants::Icons::MESON); setPriority(Node::DefaultProjectPriority + 1000); - setIcon(MesonIcon); + setIcon(Constants::Icons::MESON); setListInProject(false); } MesonFileNode::MesonFileNode(const Utils::FilePath &file) : ProjectExplorer::ProjectNode{file} { - static const auto MesonFolderIcon = Core::FileIconProvider::directoryIcon( - Constants::Icons::MESON); - setIcon(MesonFolderIcon); + setIcon(ProjectExplorer::DirectoryIcon(Constants::Icons::MESON)); setListInProject(true); } @@ -58,7 +55,7 @@ MesonTargetNode::MesonTargetNode(const Utils::FilePath &directory, const QString , m_name{name} { setPriority(Node::DefaultProjectPriority + 900); - setIcon(QIcon(":/projectexplorer/images/build.png")); + setIcon(":/projectexplorer/images/build.png"); setListInProject(false); setShowWhenEmpty(true); setProductType(ProjectExplorer::ProductType::Other); diff --git a/src/plugins/projectexplorer/projectnodes.cpp b/src/plugins/projectexplorer/projectnodes.cpp index 93d8a412f58..82118822302 100644 --- a/src/plugins/projectexplorer/projectnodes.cpp +++ b/src/plugins/projectexplorer/projectnodes.cpp @@ -46,16 +46,19 @@ #include #include -#include #include +#include #include #include +#include #include #include namespace ProjectExplorer { +QHash DirectoryIcon::m_cache; + static FolderNode *recursiveFindOrCreateFolderNode(FolderNode *folder, const Utils::FilePath &directory, const Utils::FilePath &overrideBaseDir, @@ -481,16 +484,28 @@ QString FolderNode::displayName() const } /*! - Contains the icon that should be used in a view. Default is the directory icon - (QStyle::S_PDirIcon). - s\a setIcon() + Contains the icon that should be used in a view. Default is the directory icon + (QStyle::S_PDirIcon). Calling this method is only safe in the UI thread. + + \sa setIcon() */ QIcon FolderNode::icon() const { + QTC_CHECK(QThread::currentThread() == QCoreApplication::instance()->thread()); + // Instantiating the Icon provider is expensive. - if (m_icon.isNull()) - m_icon = Core::FileIconProvider::icon(QFileIconProvider::Folder); - return m_icon; + if (auto strPtr = Utils::get_if(&m_icon)) { + m_icon = QIcon(*strPtr); + } else if (auto directoryIconPtr = Utils::get_if(&m_icon)) { + m_icon = directoryIconPtr->icon(); + } else if (auto creatorPtr = Utils::get_if(&m_icon)) { + m_icon = (*creatorPtr)(); + } else { + auto iconPtr = Utils::get_if(&m_icon); + if (!iconPtr || iconPtr->isNull()) + m_icon = Core::FileIconProvider::icon(QFileIconProvider::Folder); + } + return Utils::get(m_icon); } Node *FolderNode::findNode(const std::function &filter) @@ -724,11 +739,38 @@ void FolderNode::setDisplayName(const QString &name) m_displayName = name; } +/*! + Sets the \a icon for this node. Note that creating QIcon instances is only safe in the UI thread. +*/ void FolderNode::setIcon(const QIcon &icon) { m_icon = icon; } +/*! + Sets the \a directoryIcon that is used to create the icon for this node on demand. +*/ +void FolderNode::setIcon(const DirectoryIcon &directoryIcon) +{ + m_icon = directoryIcon; +} + +/*! + Sets the \a path that is used to create the icon for this node on demand. +*/ +void FolderNode::setIcon(const QString &path) +{ + m_icon = path; +} + +/*! + Sets the \a iconCreator function that is used to create the icon for this node on demand. +*/ +void FolderNode::setIcon(const IconCreator &iconCreator) +{ + m_icon = iconCreator; +} + void FolderNode::setLocationInfo(const QVector &info) { m_locations = info; @@ -1048,4 +1090,34 @@ void ContainerNode::handleSubTreeChanged(FolderNode *node) m_project->handleSubTreeChanged(node); } +/*! + \class ProjectExplorer::DirectoryIcon + + The DirectoryIcon class represents a directory icon with an overlay. + + The QIcon is created on demand and globally cached, so other DirectoryIcon + instances with the same overlay share the same QIcon instance. +*/ + +/*! + Creates a DirectoryIcon for the specified \a overlay. +*/ +DirectoryIcon::DirectoryIcon(const QString &overlay) + : m_overlay(overlay) +{} + +/*! + Returns the icon for this DirectoryIcon. Calling this method is only safe in the UI thread. +*/ +QIcon DirectoryIcon::icon() const +{ + QTC_CHECK(QThread::currentThread() == QCoreApplication::instance()->thread()); + const auto it = m_cache.find(m_overlay); + if (it != m_cache.end()) + return it.value(); + const QIcon icon = Core::FileIconProvider::directoryIcon(m_overlay); + m_cache.insert(m_overlay, icon); + return icon; +} + } // namespace ProjectExplorer diff --git a/src/plugins/projectexplorer/projectnodes.h b/src/plugins/projectexplorer/projectnodes.h index 0fd53414d1b..beb609a3c46 100644 --- a/src/plugins/projectexplorer/projectnodes.h +++ b/src/plugins/projectexplorer/projectnodes.h @@ -34,6 +34,7 @@ #include #include #include +#include #include @@ -93,6 +94,20 @@ class FolderNode; class ProjectNode; class ContainerNode; +class PROJECTEXPLORER_EXPORT DirectoryIcon +{ +public: + explicit DirectoryIcon(const QString &overlay); + + QIcon icon() const; // only safe in UI thread + +private: + QString m_overlay; + static QHash m_cache; +}; + +using IconCreator = std::function; + // Documentation inside. class PROJECTEXPLORER_EXPORT Node { @@ -218,6 +233,7 @@ public: explicit FolderNode(const Utils::FilePath &folderPath); QString displayName() const override; + // only safe from UI thread QIcon icon() const; bool isFolderNodeType() const override { return true; } @@ -253,7 +269,11 @@ public: bool replaceSubtree(Node *oldNode, std::unique_ptr &&newNode); void setDisplayName(const QString &name); + // you have to make sure the QIcon is created in the UI thread if you are calling setIcon(QIcon) void setIcon(const QIcon &icon); + void setIcon(const DirectoryIcon &directoryIcon); + void setIcon(const QString &path); + void setIcon(const IconCreator &iconCreator); class LocationInfo { @@ -328,7 +348,7 @@ private: QString m_displayName; QString m_addFileFilter; - mutable QIcon m_icon; + mutable Utils::variant m_icon; bool m_showWhenEmpty = false; }; diff --git a/src/plugins/qbsprojectmanager/qbsnodes.cpp b/src/plugins/qbsprojectmanager/qbsnodes.cpp index 68731e30a12..1393583213a 100644 --- a/src/plugins/qbsprojectmanager/qbsnodes.cpp +++ b/src/plugins/qbsprojectmanager/qbsnodes.cpp @@ -73,8 +73,7 @@ const QbsProductNode *parentQbsProductNode(const ProjectExplorer::Node *node) QbsGroupNode::QbsGroupNode(const QJsonObject &grp) : ProjectNode(FilePath()), m_groupData(grp) { - static QIcon groupIcon = QIcon(QString(ProjectExplorer::Constants::FILEOVERLAY_GROUP)); - setIcon(groupIcon); + setIcon(ProjectExplorer::Constants::FILEOVERLAY_GROUP); setDisplayName(grp.value("name").toString()); setEnabled(grp.value("is-enabled").toBool()); } @@ -108,9 +107,7 @@ QVariant QbsGroupNode::data(Id role) const QbsProductNode::QbsProductNode(const QJsonObject &prd) : ProjectNode(FilePath()), m_productData(prd) { - static QIcon productIcon = Core::FileIconProvider::directoryIcon( - ProjectExplorer::Constants::FILEOVERLAY_PRODUCT); - setIcon(productIcon); + setIcon(DirectoryIcon(ProjectExplorer::Constants::FILEOVERLAY_PRODUCT)); if (prd.value("is-runnable").toBool()) { setProductType(ProductType::App); } else { @@ -265,9 +262,7 @@ QJsonObject QbsProductNode::mainGroup() const QbsProjectNode::QbsProjectNode(const QJsonObject &projectData) : ProjectNode(FilePath()), m_projectData(projectData) { - static QIcon projectIcon = Core::FileIconProvider::directoryIcon( - ProjectExplorer::Constants::FILEOVERLAY_QT); - setIcon(projectIcon); + setIcon(DirectoryIcon(ProjectExplorer::Constants::FILEOVERLAY_QT)); setDisplayName(projectData.value("name").toString()); } diff --git a/src/plugins/qmlprojectmanager/qmlprojectnodes.cpp b/src/plugins/qmlprojectmanager/qmlprojectnodes.cpp index cfa6c5e74ed..fe35539314a 100644 --- a/src/plugins/qmlprojectmanager/qmlprojectnodes.cpp +++ b/src/plugins/qmlprojectmanager/qmlprojectnodes.cpp @@ -40,8 +40,7 @@ QmlProjectNode::QmlProjectNode(Project *project) { setDisplayName(project->projectFilePath().toFileInfo().completeBaseName()); - static QIcon qmlProjectIcon = Core::FileIconProvider::directoryIcon(":/projectexplorer/images/fileoverlay_qml.png"); - setIcon(qmlProjectIcon); + setIcon(DirectoryIcon(":/projectexplorer/images/fileoverlay_qml.png")); } } // namespace Internal diff --git a/src/plugins/resourceeditor/resourcenode.cpp b/src/plugins/resourceeditor/resourcenode.cpp index 88c04bbfc2c..7b57a6eabf8 100644 --- a/src/plugins/resourceeditor/resourcenode.cpp +++ b/src/plugins/resourceeditor/resourcenode.cpp @@ -243,7 +243,7 @@ ResourceTopLevelNode::ResourceTopLevelNode(const FilePath &filePath, const QString &contents) : FolderNode(filePath) { - setIcon(FileIconProvider::icon(filePath.toFileInfo())); + setIcon([filePath] { return FileIconProvider::icon(filePath.toFileInfo()); }); setPriority(Node::DefaultFilePriority); setListInProject(true); setAddFileFilter("*.png; *.jpg; *.gif; *.svg; *.ico; *.qml; *.qml.ui");