From 0514cb304db220ca26730f6d8c54745478098941 Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 27 Sep 2022 00:47:18 +0800 Subject: [PATCH 1/3] Fix coding style --- src/gui/torrentcontentmodel.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/gui/torrentcontentmodel.cpp b/src/gui/torrentcontentmodel.cpp index 664b73256..f3c5446b8 100644 --- a/src/gui/torrentcontentmodel.cpp +++ b/src/gui/torrentcontentmodel.cpp @@ -270,7 +270,7 @@ bool TorrentContentModel::allFiltered() const int TorrentContentModel::columnCount(const QModelIndex &parent) const { if (parent.isValid()) - return static_cast(parent.internalPointer())->columnCount(); + return static_cast(parent.internalPointer())->columnCount(); return m_rootItem->columnCount(); } @@ -282,7 +282,7 @@ bool TorrentContentModel::setData(const QModelIndex &index, const QVariant &valu if ((index.column() == TorrentContentModelItem::COL_NAME) && (role == Qt::CheckStateRole)) { - auto *item = static_cast(index.internalPointer()); + auto *item = static_cast(index.internalPointer()); qDebug("setData(%s, %d)", qUtf8Printable(item->name()), value.toInt()); BitTorrent::DownloadPriority prio = BitTorrent::DownloadPriority::Normal; @@ -306,7 +306,7 @@ bool TorrentContentModel::setData(const QModelIndex &index, const QVariant &valu if (role == Qt::EditRole) { Q_ASSERT(index.isValid()); - auto *item = static_cast(index.internalPointer()); + auto *item = static_cast(index.internalPointer()); switch (index.column()) { case TorrentContentModelItem::COL_NAME: @@ -336,14 +336,14 @@ bool TorrentContentModel::setData(const QModelIndex &index, const QVariant &valu TorrentContentModelItem::ItemType TorrentContentModel::itemType(const QModelIndex &index) const { - return static_cast(index.internalPointer())->itemType(); + return static_cast(index.internalPointer())->itemType(); } int TorrentContentModel::getFileIndex(const QModelIndex &index) { - auto *item = static_cast(index.internalPointer()); + auto *item = static_cast(index.internalPointer()); if (item->itemType() == TorrentContentModelItem::FileType) - return static_cast(item)->fileIndex(); + return static_cast(item)->fileIndex(); Q_ASSERT(item->itemType() == TorrentContentModelItem::FileType); return -1; @@ -354,7 +354,7 @@ QVariant TorrentContentModel::data(const QModelIndex &index, const int role) con if (!index.isValid()) return {}; - auto *item = static_cast(index.internalPointer()); + auto *item = static_cast(index.internalPointer()); switch (role) { @@ -443,7 +443,7 @@ QModelIndex TorrentContentModel::index(int row, int column, const QModelIndex &p if (!parent.isValid()) parentItem = m_rootItem; else - parentItem = static_cast(parent.internalPointer()); + parentItem = static_cast(parent.internalPointer()); Q_ASSERT(parentItem); if (row >= parentItem->childCount()) @@ -460,7 +460,7 @@ QModelIndex TorrentContentModel::parent(const QModelIndex &index) const if (!index.isValid()) return {}; - auto *childItem = static_cast(index.internalPointer()); + auto *childItem = static_cast(index.internalPointer()); if (!childItem) return {}; @@ -480,7 +480,7 @@ int TorrentContentModel::rowCount(const QModelIndex &parent) const if (!parent.isValid()) parentItem = m_rootItem; else - parentItem = dynamic_cast(static_cast(parent.internalPointer())); + parentItem = dynamic_cast(static_cast(parent.internalPointer())); return parentItem ? parentItem->childCount() : 0; } @@ -555,7 +555,7 @@ void TorrentContentModel::selectAll() { for (int i = 0; i < m_rootItem->childCount(); ++i) { - TorrentContentModelItem* child = m_rootItem->child(i); + TorrentContentModelItem *child = m_rootItem->child(i); if (child->priority() == BitTorrent::DownloadPriority::Ignored) child->setPriority(BitTorrent::DownloadPriority::Normal); } From 6c8b31420cc444a4e1b66513c0726f687574cf3b Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 27 Sep 2022 01:08:44 +0800 Subject: [PATCH 2/3] Remove unused functions --- src/gui/torrentcontentmodel.cpp | 18 ------------------ src/gui/torrentcontentmodel.h | 4 ---- 2 files changed, 22 deletions(-) diff --git a/src/gui/torrentcontentmodel.cpp b/src/gui/torrentcontentmodel.cpp index f3c5446b8..52775bcd3 100644 --- a/src/gui/torrentcontentmodel.cpp +++ b/src/gui/torrentcontentmodel.cpp @@ -550,21 +550,3 @@ void TorrentContentModel::setupModelData(const BitTorrent::AbstractFileStorage & } emit layoutChanged(); } - -void TorrentContentModel::selectAll() -{ - for (int i = 0; i < m_rootItem->childCount(); ++i) - { - TorrentContentModelItem *child = m_rootItem->child(i); - if (child->priority() == BitTorrent::DownloadPriority::Ignored) - child->setPriority(BitTorrent::DownloadPriority::Normal); - } - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); -} - -void TorrentContentModel::selectNone() -{ - for (int i = 0; i < m_rootItem->childCount(); ++i) - m_rootItem->child(i)->setPriority(BitTorrent::DownloadPriority::Ignored); - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); -} diff --git a/src/gui/torrentcontentmodel.h b/src/gui/torrentcontentmodel.h index a1baf0666..e8219e145 100644 --- a/src/gui/torrentcontentmodel.h +++ b/src/gui/torrentcontentmodel.h @@ -79,10 +79,6 @@ public: signals: void filteredFilesChanged(); -public slots: - void selectAll(); - void selectNone(); - private: TorrentContentModelFolder *m_rootItem = nullptr; QVector m_filesIndex; From 9a20aa51def232e5821e2a5234670eb52372e5df Mon Sep 17 00:00:00 2001 From: Chocobo1 Date: Tue, 27 Sep 2022 01:09:05 +0800 Subject: [PATCH 3/3] Fix torrent content checkboxes not updated properly And reduce emitting redundant 'data updated' signals. Closes #17144, #17764. --- src/gui/torrentcontentfiltermodel.cpp | 8 +- src/gui/torrentcontentmodel.cpp | 160 ++++++++++++++++++-------- src/gui/torrentcontentmodel.h | 2 + 3 files changed, 113 insertions(+), 57 deletions(-) diff --git a/src/gui/torrentcontentfiltermodel.cpp b/src/gui/torrentcontentfiltermodel.cpp index 59e2e4771..9ff5043a1 100644 --- a/src/gui/torrentcontentfiltermodel.cpp +++ b/src/gui/torrentcontentfiltermodel.cpp @@ -110,17 +110,13 @@ bool TorrentContentFilterModel::lessThan(const QModelIndex &left, const QModelIn void TorrentContentFilterModel::selectAll() { for (int i = 0; i < rowCount(); ++i) - setData(index(i, 0), Qt::Checked, Qt::CheckStateRole); - - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); + setData(index(i, TorrentContentModelItem::COL_NAME), Qt::Checked, Qt::CheckStateRole); } void TorrentContentFilterModel::selectNone() { for (int i = 0; i < rowCount(); ++i) - setData(index(i, 0), Qt::Unchecked, Qt::CheckStateRole); - - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); + setData(index(i, TorrentContentModelItem::COL_NAME), Qt::Unchecked, Qt::CheckStateRole); } bool TorrentContentFilterModel::hasFiltered(const QModelIndex &folder) const diff --git a/src/gui/torrentcontentmodel.cpp b/src/gui/torrentcontentmodel.cpp index 52775bcd3..ff4d23ce2 100644 --- a/src/gui/torrentcontentmodel.cpp +++ b/src/gui/torrentcontentmodel.cpp @@ -220,7 +220,7 @@ void TorrentContentModel::updateFilesProgress(const QVector &fp) // Update folders progress in the tree m_rootItem->recalculateProgress(); m_rootItem->recalculateAvailability(); - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); + notifyModelUpdate(index(0, 0)); } void TorrentContentModel::updateFilesPriorities(const QVector &fprio) @@ -233,7 +233,7 @@ void TorrentContentModel::updateFilesPriorities(const QVectorsetPriority(static_cast(fprio[i])); - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); + notifyModelUpdate(index(0, 0)); } void TorrentContentModel::updateFilesAvailability(const QVector &fa) @@ -247,7 +247,7 @@ void TorrentContentModel::updateFilesAvailability(const QVector &fa) m_filesIndex[i]->setAvailability(fa[i]); // Update folders progress in the tree m_rootItem->recalculateProgress(); - emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1))); + notifyModelUpdate(index(0, 0)); } QVector TorrentContentModel::getFilePriorities() const @@ -269,13 +269,11 @@ bool TorrentContentModel::allFiltered() const int TorrentContentModel::columnCount(const QModelIndex &parent) const { - if (parent.isValid()) - return static_cast(parent.internalPointer())->columnCount(); - - return m_rootItem->columnCount(); + Q_UNUSED(parent); + return TorrentContentModelItem::NB_COL; } -bool TorrentContentModel::setData(const QModelIndex &index, const QVariant &value, int role) +bool TorrentContentModel::setData(const QModelIndex &index, const QVariant &value, const int role) { if (!index.isValid()) return false; @@ -283,52 +281,71 @@ bool TorrentContentModel::setData(const QModelIndex &index, const QVariant &valu if ((index.column() == TorrentContentModelItem::COL_NAME) && (role == Qt::CheckStateRole)) { auto *item = static_cast(index.internalPointer()); - qDebug("setData(%s, %d)", qUtf8Printable(item->name()), value.toInt()); - BitTorrent::DownloadPriority prio = BitTorrent::DownloadPriority::Normal; - if (value.toInt() == Qt::PartiallyChecked) - prio = BitTorrent::DownloadPriority::Mixed; - else if (value.toInt() == Qt::Unchecked) - prio = BitTorrent::DownloadPriority::Ignored; + const BitTorrent::DownloadPriority currentPrio = item->priority(); + const auto checkState = static_cast(value.toInt()); + const BitTorrent::DownloadPriority newPrio = (checkState == Qt::PartiallyChecked) + ? BitTorrent::DownloadPriority::Mixed + : ((checkState == Qt::Unchecked) + ? BitTorrent::DownloadPriority::Ignored + : BitTorrent::DownloadPriority::Normal); - if (item->priority() != prio) + if (currentPrio != newPrio) { - item->setPriority(prio); + item->setPriority(newPrio); // Update folders progress in the tree m_rootItem->recalculateProgress(); m_rootItem->recalculateAvailability(); - emit dataChanged(this->index(0, 0), this->index((rowCount() - 1), (columnCount() - 1))); + + notifyModelUpdate(index); emit filteredFilesChanged(); + + return true; } - return true; } if (role == Qt::EditRole) { - Q_ASSERT(index.isValid()); auto *item = static_cast(index.internalPointer()); + switch (index.column()) { case TorrentContentModelItem::COL_NAME: - item->setName(value.toString()); + { + const QString currentName = item->name(); + const QString newName = value.toString(); + if (currentName != newName) + { + item->setName(newName); + emit dataChanged(index, index); + return true; + } + } break; + case TorrentContentModelItem::COL_PRIO: { - const BitTorrent::DownloadPriority previousPrio = item->priority(); + const BitTorrent::DownloadPriority currentPrio = item->priority(); const auto newPrio = static_cast(value.toInt()); - item->setPriority(newPrio); - if ((newPrio != previousPrio) && ((newPrio == BitTorrent::DownloadPriority::Ignored) - || (previousPrio == BitTorrent::DownloadPriority::Ignored))) + if (currentPrio != newPrio) { - emit filteredFilesChanged(); + item->setPriority(newPrio); + + notifyModelUpdate(index); + if ((newPrio == BitTorrent::DownloadPriority::Ignored) + || (currentPrio == BitTorrent::DownloadPriority::Ignored)) + { + emit filteredFilesChanged(); + } + + return true; } } break; + default: - return false; + break; } - emit dataChanged(index, index); - return true; } return false; @@ -392,8 +409,10 @@ QVariant TorrentContentModel::data(const QModelIndex &index, const int role) con return item->underlyingData(index.column()); default: - return {}; + break; } + + return {}; } Qt::ItemFlags TorrentContentModel::flags(const QModelIndex &index) const @@ -431,19 +450,14 @@ QVariant TorrentContentModel::headerData(int section, Qt::Orientation orientatio } } -QModelIndex TorrentContentModel::index(int row, int column, const QModelIndex &parent) const +QModelIndex TorrentContentModel::index(const int row, const int column, const QModelIndex &parent) const { - if (parent.isValid() && (parent.column() != 0)) - return {}; - - if (column >= TorrentContentModelItem::NB_COL) + if (column >= columnCount()) return {}; - TorrentContentModelFolder *parentItem = nullptr; - if (!parent.isValid()) - parentItem = m_rootItem; - else - parentItem = static_cast(parent.internalPointer()); + const TorrentContentModelFolder *parentItem = parent.isValid() + ? static_cast(parent.internalPointer()) + : m_rootItem; Q_ASSERT(parentItem); if (row >= parentItem->childCount()) @@ -452,6 +466,7 @@ QModelIndex TorrentContentModel::index(int row, int column, const QModelIndex &p TorrentContentModelItem *childItem = parentItem->child(row); if (childItem) return createIndex(row, column, childItem); + return {}; } @@ -460,28 +475,26 @@ QModelIndex TorrentContentModel::parent(const QModelIndex &index) const if (!index.isValid()) return {}; - auto *childItem = static_cast(index.internalPointer()); - if (!childItem) + const auto *item = static_cast(index.internalPointer()); + if (!item) return {}; - TorrentContentModelItem *parentItem = childItem->parent(); + TorrentContentModelItem *parentItem = item->parent(); if (parentItem == m_rootItem) return {}; + // From https://doc.qt.io/qt-6/qabstractitemmodel.html#parent: + // A common convention used in models that expose tree data structures is that only items + // in the first column have children. For that case, when reimplementing this function in + // a subclass the column of the returned QModelIndex would be 0. return createIndex(parentItem->row(), 0, parentItem); } int TorrentContentModel::rowCount(const QModelIndex &parent) const { - if (parent.column() > 0) - return 0; - - TorrentContentModelFolder *parentItem = nullptr; - if (!parent.isValid()) - parentItem = m_rootItem; - else - parentItem = dynamic_cast(static_cast(parent.internalPointer())); - + const TorrentContentModelFolder *parentItem = parent.isValid() + ? dynamic_cast(static_cast(parent.internalPointer())) + : m_rootItem; return parentItem ? parentItem->childCount() : 0; } @@ -550,3 +563,48 @@ void TorrentContentModel::setupModelData(const BitTorrent::AbstractFileStorage & } emit layoutChanged(); } + +void TorrentContentModel::notifyModelUpdate(const QModelIndex &index) +{ + Q_ASSERT(index.isValid()); + + const int lastColumnIndex = columnCount(index) - 1; + + // emit itself + emit dataChanged(index.siblingAtColumn(0), index.siblingAtColumn(lastColumnIndex)); + + // propagate up the model + QModelIndex parentIndex = parent(index); + while (parentIndex.isValid()) + { + emit dataChanged(parentIndex.siblingAtColumn(0), parentIndex.siblingAtColumn(lastColumnIndex)); + parentIndex = parent(parentIndex); + } + + // propagate down the model + QVector parentIndexes; + + if (hasChildren(index)) + parentIndexes.push_back(index); + + while (!parentIndexes.isEmpty()) + { + const QModelIndex parent = parentIndexes.takeLast(); + + const int childCount = rowCount(parent); + const QModelIndex childTopLeft = this->index(0, 0, parent); + const QModelIndex childBottomRight = this->index((childCount - 1), lastColumnIndex, parent); + + // emit this generation + emit dataChanged(childTopLeft, childBottomRight); + + // check generations further down + parentIndexes.reserve(childCount); + for (int i = 0; i < childCount; ++i) + { + const QModelIndex child = childTopLeft.siblingAtRow(i); + if (hasChildren(child)) + parentIndexes.push_back(child); + } + } +} diff --git a/src/gui/torrentcontentmodel.h b/src/gui/torrentcontentmodel.h index e8219e145..6505fc679 100644 --- a/src/gui/torrentcontentmodel.h +++ b/src/gui/torrentcontentmodel.h @@ -80,6 +80,8 @@ signals: void filteredFilesChanged(); private: + void notifyModelUpdate(const QModelIndex &index); + TorrentContentModelFolder *m_rootItem = nullptr; QVector m_filesIndex; QFileIconProvider *m_fileIconProvider = nullptr;