Fix torrent content checkboxes not updated properly

And reduce emitting redundant 'data updated' signals.

Closes #17144, #17764.

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

src/gui/torrentcontentmodel.cpp

@@ -220,7 +220,7 @@ void TorrentContentModel::updateFilesProgress(const QVector<qreal> &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<BitTorrent::DownloadPriority> &fprio)
@@ -233,7 +233,7 @@ void TorrentContentModel::updateFilesPriorities(const QVector<BitTorrent::Downlo
     emit layoutAboutToBeChanged();
     for (int i = 0; i < fprio.size(); ++i)
         m_filesIndex[i]->setPriority(static_cast<BitTorrent::DownloadPriority>(fprio[i]));
-    emit dataChanged(index(0, 0), index((rowCount() - 1), (columnCount() - 1)));
+    notifyModelUpdate(index(0, 0));
 }
 
 void TorrentContentModel::updateFilesAvailability(const QVector<qreal> &fa)
@@ -247,7 +247,7 @@ void TorrentContentModel::updateFilesAvailability(const QVector<qreal> &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<BitTorrent::DownloadPriority> TorrentContentModel::getFilePriorities() const
@@ -269,13 +269,11 @@ bool TorrentContentModel::allFiltered() const
 
 int TorrentContentModel::columnCount(const QModelIndex &parent) const
 {
-    if (parent.isValid())
-        return static_cast<TorrentContentModelItem *>(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<TorrentContentModelItem *>(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<Qt::CheckState>(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<TorrentContentModelItem *>(index.internalPointer());
+
         switch (index.column())
         {
         case TorrentContentModelItem::COL_NAME:
-            item->setName(value.toString());
-            break;
-        case TorrentContentModelItem::COL_PRIO:
             {
-                const BitTorrent::DownloadPriority previousPrio = item->priority();
-                const auto newPrio = static_cast<BitTorrent::DownloadPriority>(value.toInt());
-                item->setPriority(newPrio);
-                if ((newPrio != previousPrio) && ((newPrio == BitTorrent::DownloadPriority::Ignored)
-                        || (previousPrio == BitTorrent::DownloadPriority::Ignored)))
+                const QString currentName = item->name();
+                const QString newName = value.toString();
+                if (currentName != newName)
                 {
-                    emit filteredFilesChanged();
+                    item->setName(newName);
+                    emit dataChanged(index, index);
+                    return true;
                 }
             }
             break;
+
+        case TorrentContentModelItem::COL_PRIO:
+            {
+                const BitTorrent::DownloadPriority currentPrio = item->priority();
+                const auto newPrio = static_cast<BitTorrent::DownloadPriority>(value.toInt());
+                if (currentPrio != newPrio)
+                {
+                    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))
+    if (column >= columnCount())
         return {};
 
-    if (column >= TorrentContentModelItem::NB_COL)
-        return {};
-
-    TorrentContentModelFolder *parentItem;
-    if (!parent.isValid())
-        parentItem = m_rootItem;
-    else
-        parentItem = static_cast<TorrentContentModelFolder *>(parent.internalPointer());
+    const TorrentContentModelFolder *parentItem = parent.isValid()
+        ? static_cast<TorrentContentModelFolder *>(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<TorrentContentModelItem *>(index.internalPointer());
-    if (!childItem)
+    const auto *item = static_cast<TorrentContentModelItem *>(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;
-    if (!parent.isValid())
-        parentItem = m_rootItem;
-    else
-        parentItem = dynamic_cast<TorrentContentModelFolder *>(static_cast<TorrentContentModelItem *>(parent.internalPointer()));
-
+    const TorrentContentModelFolder *parentItem = parent.isValid()
+        ? dynamic_cast<TorrentContentModelFolder *>(static_cast<TorrentContentModelItem *>(parent.internalPointer()))
+        : m_rootItem;
     return parentItem ? parentItem->childCount() : 0;
 }
 
@@ -536,3 +549,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<QModelIndex> 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);
+        }
+    }
+}

src/gui/torrentcontentmodel.h

@@ -80,7 +80,9 @@ signals:
     void filteredFilesChanged();
 
 private:
-    TorrentContentModelFolder *m_rootItem;
+    void notifyModelUpdate(const QModelIndex &index);
+
+    TorrentContentModelFolder *m_rootItem = nullptr;
     QVector<TorrentContentModelFile *> m_filesIndex;
     QFileIconProvider *m_fileIconProvider;
 };