From 6c4e237d4b43db5c3f9dc01f8c0d3313f3bd2605 Mon Sep 17 00:00:00 2001 From: isanae <14251494+isanae@users.noreply.github.com> Date: Sun, 19 Jul 2020 16:48:31 -0400 Subject: fixed crash because items were sorted while being expanded when expanding all or updating the tree, only sort once at the end cache file types --- src/filetree.cpp | 16 ++++- src/filetree.h | 3 + src/filetreeitem.cpp | 167 ++++++++++++++++++++++++++++++++++---------------- src/filetreeitem.h | 5 +- src/filetreemodel.cpp | 59 +++++++++++++----- src/filetreemodel.h | 9 ++- 6 files changed, 187 insertions(+), 72 deletions(-) (limited to 'src') diff --git a/src/filetree.cpp b/src/filetree.cpp index 2d92c954..1dadfaad 100644 --- a/src/filetree.cpp +++ b/src/filetree.cpp @@ -802,11 +802,11 @@ void FileTree::addCommonMenus(QMenu& menu) .addTo(menu); MenuItem(tr("Ex&pand All")) - .callback([&]{ m_tree->expandAll(); }) + .callback([&]{ expandAll(); }) .addTo(menu); MenuItem(tr("&Collapse All")) - .callback([&]{ m_tree->collapseAll(); }) + .callback([&]{ collapseAll(); }) .addTo(menu); } @@ -820,3 +820,15 @@ QModelIndex FileTree::proxiedIndex(const QModelIndex& index) return index; } } + +void FileTree::collapseAll() +{ + m_tree->collapseAll(); +} + +void FileTree::expandAll() +{ + m_model->aboutToExpandAll(); + m_tree->expandAll(); + m_model->expandedAll(); +} diff --git a/src/filetree.h b/src/filetree.h index 2669e53b..8d71abb3 100644 --- a/src/filetree.h +++ b/src/filetree.h @@ -25,6 +25,9 @@ public: bool fullyLoaded() const; void ensureFullyLoaded(); + void expandAll(); + void collapseAll(); + void open(FileTreeItem* item=nullptr); void openHooked(FileTreeItem* item=nullptr); void preview(FileTreeItem* item=nullptr); diff --git a/src/filetreeitem.cpp b/src/filetreeitem.cpp index 788b4129..49bc65ac 100644 --- a/src/filetreeitem.cpp +++ b/src/filetreeitem.cpp @@ -14,9 +14,7 @@ constexpr bool AlwaysSortDirectoriesFirst = true; const QString& directoryFileType() { - static QString name; - - if (name.isEmpty()) { + static const QString name = [] { const DWORD flags = SHGFI_TYPENAME; SHFILEINFOW sfi = {}; @@ -30,15 +28,88 @@ const QString& directoryFileType() "SHGetFileInfoW failed for folder file type, {}", formatSystemMessage(e)); - name = "File folder"; + return QString("File folder"); } else { - name = QString::fromWCharArray(sfi.szTypeName); + return QString::fromWCharArray(sfi.szTypeName); } - } + }(); + + return name; +} + +const QString& cachedFileTypeNoExtension() +{ + static const QString name = [] { + const DWORD flags = SHGFI_TYPENAME; + SHFILEINFOW sfi = {}; + + // dummy filename with no extension + const auto r = SHGetFileInfoW(L"file", 0, &sfi, sizeof(sfi), flags); + + if (!r) { + const auto e = GetLastError(); + + log::error( + "SHGetFileInfoW failed for file without extension, {}", + formatSystemMessage(e)); + + return QString("File"); + } else { + return QString::fromWCharArray(sfi.szTypeName); + } + }(); return name; } +const QString& cachedFileType(const std::wstring& file, bool isOnFilesystem) +{ + static std::map> map; + static std::mutex mutex; + + const auto dot = file.find_last_of(L'.'); + if (dot == std::wstring::npos) { + return cachedFileTypeNoExtension(); + } + + std::scoped_lock lock(mutex); + const auto sv = std::wstring_view(file.c_str() + dot, file.size() - dot); + + auto itor = map.find(sv); + if (itor != map.end()) { + return itor->second; + } + + + DWORD flags = SHGFI_TYPENAME; + + if (!isOnFilesystem) { + // files from archives are not on the filesystem; this flag forces + // SHGetFileInfoW() to only work with the filename + flags |= SHGFI_USEFILEATTRIBUTES; + } + + SHFILEINFOW sfi = {}; + const auto r = SHGetFileInfoW(file.c_str(), 0, &sfi, sizeof(sfi), flags); + + QString s; + + if (!r) { + const auto e = GetLastError(); + + log::error( + "SHGetFileInfoW failed for '{}', {}", + file, formatSystemMessage(e)); + + s = cachedFileTypeNoExtension(); + } else { + s = QString::fromWCharArray(sfi.szTypeName); + } + + return map.emplace(sv, s).first->second; +} + + FileTreeItem::FileTreeItem( FileTreeModel* model, FileTreeItem* parent, @@ -176,50 +247,59 @@ public: } }; -void FileTreeItem::sort() +void FileTreeItem::queueSort() { if (!m_children.empty()) { - m_model->sortItem(*this, true); + m_model->queueSortItem(this); + } +} + +void FileTreeItem::makeSortingStale() +{ + m_sortingStale = true; + + for (auto& c : m_children) { + c->makeSortingStale(); } } void FileTreeItem::sort(int column, Qt::SortOrder order, bool force) { - if (!force && !m_expanded) { + if (!m_expanded) { m_sortingStale = true; return; } - if (m_sortingStale) { + if (m_sortingStale || force) { //log::debug("sorting is stale for {}, sorting now", debugName()); m_sortingStale = false; - } - std::sort(m_children.begin(), m_children.end(), [&](auto&& a, auto&& b) { - int r = 0; - - if (a->isDirectory() && !b->isDirectory()) { - if constexpr (AlwaysSortDirectoriesFirst) { - return true; + std::sort(m_children.begin(), m_children.end(), [&](auto&& a, auto&& b) { + int r = 0; + + if (a->isDirectory() && !b->isDirectory()) { + if constexpr (AlwaysSortDirectoriesFirst) { + return true; + } else { + r = -1; + } + } else if (!a->isDirectory() && b->isDirectory()) { + if constexpr (AlwaysSortDirectoriesFirst) { + return false; + } else { + r = 1; + } } else { - r = -1; + r = FileTreeItem::Sorter::compare(column, a.get(), b.get()); } - } else if (!a->isDirectory() && b->isDirectory()) { - if constexpr (AlwaysSortDirectoriesFirst) { - return false; + + if (order == Qt::AscendingOrder) { + return (r < 0); } else { - r = 1; + return (r > 0); } - } else { - r = FileTreeItem::Sorter::compare(column, a.get(), b.get()); - } - - if (order == Qt::AscendingOrder) { - return (r < 0); - } else { - return (r > 0); - } - }); + }); + } for (auto& child : m_children) { child->sort(column, order, force); @@ -321,28 +401,11 @@ void FileTreeItem::getFileType() const return; } - DWORD flags = SHGFI_TYPENAME; - - if (isFromArchive()) { - // files from archives are not on the filesystem; this flag forces - // SHGetFileInfoW() to only work with the filename - flags |= SHGFI_USEFILEATTRIBUTES; - } - - SHFILEINFOW sfi = {}; - const auto r = SHGetFileInfoW( - m_wsRealPath.c_str(), 0, &sfi, sizeof(sfi), flags); - - if (!r) { - const auto e = GetLastError(); - - log::error( - "SHGetFileInfoW failed for '{}', {}", - m_realPath, formatSystemMessage(e)); - + const auto& t = cachedFileType(m_wsRealPath, !isFromArchive()); + if (t.isEmpty()) { m_fileType.fail(); } else { - m_fileType.set(QString::fromWCharArray(sfi.szTypeName)); + m_fileType.set(t); } } diff --git a/src/filetreeitem.h b/src/filetreeitem.h index 2092782e..750e4719 100644 --- a/src/filetreeitem.h +++ b/src/filetreeitem.h @@ -93,6 +93,7 @@ public: } void sort(int column, Qt::SortOrder order, bool force); + void makeSortingStale(); FileTreeItem* parent() { @@ -223,7 +224,7 @@ public: m_expanded = b; if (m_expanded && m_sortingStale) { - sort(); + queueSort(); } } @@ -314,7 +315,7 @@ private: std::wstring dataRelativeParentPath, bool isDirectory, std::wstring file); void getFileType() const; - void sort(); + void queueSort(); }; #endif // MODORGANIZER_FILETREEITEM_INCLUDED diff --git a/src/filetreemodel.cpp b/src/filetreemodel.cpp index 11901c43..8790f981 100644 --- a/src/filetreemodel.cpp +++ b/src/filetreemodel.cpp @@ -196,13 +196,13 @@ void* makeInternalPointer(FileTreeItem* item) FileTreeModel::FileTreeModel(OrganizerCore& core, QObject* parent) : QAbstractItemModel(parent), m_core(core), m_enabled(true), m_root(FileTreeItem::createDirectory(this, nullptr, L"", L"")), - m_flags(NoFlags), m_fullyLoaded(false) + m_flags(NoFlags), m_fullyLoaded(false), m_sortingEnabled(true) { m_root->setExpanded(true); + m_sortTimer.setSingleShot(true); connect(&m_removeTimer, &QTimer::timeout, [&]{ removeItems(); }); connect(&m_sortTimer, &QTimer::timeout, [&]{ sortItems(); }); - connect(&m_iconPendingTimer, &QTimer::timeout, [&]{ updatePendingIcons(); }); } @@ -212,6 +212,7 @@ void FileTreeModel::refresh() m_fullyLoaded = false; update(*m_root, *m_core.directoryStructure(), L"", false); + sortItem(*m_root, false); } void FileTreeModel::clear() @@ -253,6 +254,17 @@ void FileTreeModel::setEnabled(bool b) m_enabled = b; } +void FileTreeModel::aboutToExpandAll() +{ + m_sortingEnabled = false; +} + +void FileTreeModel::expandedAll() +{ + m_sortingEnabled = true; + sortItem(*m_root, false); +} + const FileTreeModel::SortInfo& FileTreeModel::sortInfo() const { return m_sort; @@ -360,6 +372,10 @@ void FileTreeModel::doFetchMore(const QModelIndex& parent, bool forFetch) const auto parentPath = item->dataRelativeParentPath(); update(*item, *parentEntry, parentPath.toStdWString(), forFetch); + + if (!forFetch) { + sortItem(*item, false); + } } QVariant FileTreeModel::data(const QModelIndex& index, int role) const @@ -485,7 +501,7 @@ void FileTreeModel::sort(int column, Qt::SortOrder order) m_sort.column = column; m_sort.order = order; - sortItem(*m_root, false); + sortItem(*m_root, true); } FileTreeItem* FileTreeModel::itemFromIndex(const QModelIndex& index) const @@ -558,11 +574,15 @@ void FileTreeModel::update( } if (added) { + parentItem.makeSortingStale(); + // see comment at the top of this file - if (forFetching) - queueSortItem(&parentItem); - else - sortItem(parentItem, true); + if (forFetching) { + // don't pass a specific item, this will start a timer and re-sort the + // whole tree, which is faster than potentially queuing every single + // node if the whole tree is expanded + queueSortItem(nullptr); + } } } @@ -873,21 +893,32 @@ void FileTreeModel::removeItems() void FileTreeModel::queueSortItem(FileTreeItem* item) { - m_sortItems.push_back(item); + if (!m_sortingEnabled) { + return; + } + + if (item) { + m_sortItems.push_back(item); + } + m_sortTimer.start(1); } void FileTreeModel::sortItems() { // see comment at the top of this file - trace(log::debug("sort item timer: sorting {} items", m_sortItems.size())); - auto copy = std::move(m_sortItems); - m_sortItems.clear(); - m_sortTimer.stop(); + if (m_sortItems.empty()) { + sortItem(*m_root, false); + } else { + log::debug("sort item timer: sorting {} items", m_sortItems.size()); - for (auto&& f : copy) { - sortItem(*f, true); + auto items = std::move(m_sortItems); + m_sortItems.clear(); + + for (auto* item : items) { + sortItem(*item, false); + } } } diff --git a/src/filetreemodel.h b/src/filetreemodel.h index 334a0577..4894e4be 100644 --- a/src/filetreemodel.h +++ b/src/filetreemodel.h @@ -61,6 +61,10 @@ public: bool enabled() const; void setEnabled(bool b); + void aboutToExpandAll(); + void expandedAll(); + + const SortInfo& sortInfo() const; QModelIndex index(int row, int col, const QModelIndex& parent={}) const override; @@ -77,6 +81,7 @@ public: FileTreeItem* itemFromIndex(const QModelIndex& index) const; void sortItem(FileTreeItem& item, bool force); + void queueSortItem(FileTreeItem* item); private: class Range; @@ -92,11 +97,12 @@ private: mutable QTimer m_iconPendingTimer; SortInfo m_sort; bool m_fullyLoaded; + bool m_sortingEnabled; // see top of filetreemodel.cpp std::vector m_removeItems; - QTimer m_removeTimer; std::vector m_sortItems; + QTimer m_removeTimer; QTimer m_sortTimer; @@ -118,7 +124,6 @@ private: void queueRemoveItem(FileTreeItem* item); void removeItems(); - void queueSortItem(FileTreeItem* item); void sortItems(); -- cgit v1.3.1 From 0c7265be4f897834fc6126068c1423b6d2defdaf Mon Sep 17 00:00:00 2001 From: isanae <14251494+isanae@users.noreply.github.com> Date: Mon, 20 Jul 2020 19:02:14 -0400 Subject: only sort once at the end when fully loading for search temporarily disable filtering completely when fully loading instead of just disabling recursive filtering, this could close already expanded nodes if their parent directories didn't match --- src/datatab.cpp | 4 ++-- src/filetreemodel.cpp | 10 ++++++---- src/filetreemodel.h | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/datatab.cpp b/src/datatab.cpp index af7eb18f..21c205e5 100644 --- a/src/datatab.cpp +++ b/src/datatab.cpp @@ -118,9 +118,9 @@ void DataTab::updateTree() void DataTab::ensureFullyLoaded() { if (!m_filetree->fullyLoaded()) { - m_filter.proxyModel()->setRecursiveFilteringEnabled(false); + m_filter.setFilteringEnabled(false); m_filetree->ensureFullyLoaded(); - m_filter.proxyModel()->setRecursiveFilteringEnabled(true); + m_filter.setFilteringEnabled(true); } } diff --git a/src/filetreemodel.cpp b/src/filetreemodel.cpp index 8790f981..59025479 100644 --- a/src/filetreemodel.cpp +++ b/src/filetreemodel.cpp @@ -227,7 +227,7 @@ void FileTreeModel::clear() void FileTreeModel::recursiveFetchMore(const QModelIndex& m) { if (canFetchMore(m)) { - doFetchMore(m, false); + doFetchMore(m, false, false); } for (int i=0; idataRelativeParentPath(); update(*item, *parentEntry, parentPath.toStdWString(), forFetch); - if (!forFetch) { + if (!forFetch && doSort) { sortItem(*item, false); } } diff --git a/src/filetreemodel.h b/src/filetreemodel.h index 4894e4be..73a18d91 100644 --- a/src/filetreemodel.h +++ b/src/filetreemodel.h @@ -119,7 +119,7 @@ private: FileTreeItem& parentItem, const MOShared::DirectoryEntry& parentEntry, const std::wstring& parentPath, bool forFetching); - void doFetchMore(const QModelIndex& parent, bool forFetch); + void doFetchMore(const QModelIndex& parent, bool forFetch, bool doSort); void queueRemoveItem(FileTreeItem* item); void removeItems(); -- cgit v1.3.1