From 1b2e65011d256fe6ed963935a4fe83524de8b701 Mon Sep 17 00:00:00 2001 From: Gabriele Date: Tue, 10 Feb 2015 17:21:22 +0100 Subject: [PATCH 1/2] Follow project coding style. Issue #2192. --- src/gui/transferlistsortmodel.h | 37 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/gui/transferlistsortmodel.h b/src/gui/transferlistsortmodel.h index 4580bbbce..797c542dc 100644 --- a/src/gui/transferlistsortmodel.h +++ b/src/gui/transferlistsortmodel.h @@ -35,33 +35,34 @@ #include #include "torrentfilterenum.h" -class TransferListSortModel : public QSortFilterProxyModel { - Q_OBJECT +class TransferListSortModel: public QSortFilterProxyModel +{ + Q_OBJECT public: - TransferListSortModel(QObject *parent = 0); + TransferListSortModel(QObject *parent = 0); - void setStatusFilter(const TorrentFilter::TorrentFilter &filter); - void setLabelFilter(const QString &label); - void disableLabelFilter(); - void setTrackerFilter(const QStringList &hashes); - void disableTrackerFilter(); + void setStatusFilter(const TorrentFilter::TorrentFilter &filter); + void setLabelFilter(const QString &label); + void disableLabelFilter(); + void setTrackerFilter(const QStringList &hashes); + void disableTrackerFilter(); private: - bool lessThan(const QModelIndex &left, const QModelIndex &right) const; - bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const; + bool lessThan(const QModelIndex &left, const QModelIndex &right) const; + bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const; - bool matchStatusFilter(int sourceRow, const QModelIndex &sourceParent) const; - bool matchLabelFilter(int sourceRow, const QModelIndex &sourceParent) const; - bool matchTrackerFilter(int sourceRow, const QModelIndex &sourceParent) const; + bool matchStatusFilter(int sourceRow, const QModelIndex &sourceParent) const; + bool matchLabelFilter(int sourceRow, const QModelIndex &sourceParent) const; + bool matchTrackerFilter(int sourceRow, const QModelIndex &sourceParent) const; private: - TorrentFilter::TorrentFilter filter0; + TorrentFilter::TorrentFilter filter0; - bool labelFilterEnabled; - QString labelFilter; - bool trackerFilterEnabled; - QStringList trackerFilter; + bool labelFilterEnabled; + QString labelFilter; + bool trackerFilterEnabled; + QStringList trackerFilter; }; #endif // TRANSFERLISTSORTMODEL_H From 1f77a03eb6f2cdbf5aa3fb6e7653cfa20a9c6cd9 Mon Sep 17 00:00:00 2001 From: Gabriele Date: Tue, 10 Feb 2015 17:25:17 +0100 Subject: [PATCH 2/2] Don't reorder the torrents in the transfer list if not necessary The current sorting algorithm is not stable and causes undesidered rearrangements of the transfer list when different torrents have same values in respect to the current sorting criterion. Fix this by using the priority, the seed date and the hash of the torrents as fallback values to determine the order. Closes #2158. Closes #2526. --- src/gui/transferlistsortmodel.cpp | 74 +++++++++++++++++++++---------- src/gui/transferlistsortmodel.h | 1 + 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/gui/transferlistsortmodel.cpp b/src/gui/transferlistsortmodel.cpp index 3e9af1121..7e108fb4b 100644 --- a/src/gui/transferlistsortmodel.cpp +++ b/src/gui/transferlistsortmodel.cpp @@ -93,10 +93,13 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex QVariant vL = left.data(); QVariant vR = right.data(); if (!(vL.isValid() && vR.isValid())) - return QSortFilterProxyModel::lessThan(left, right); + return lowerPositionThan(left, right); Q_ASSERT(vL.isValid()); Q_ASSERT(vR.isValid()); + if (vL == vR) + return lowerPositionThan(left, right); + bool res = false; if (misc::naturalSort(vL.toString(), vR.toString(), res)) return res; @@ -114,26 +117,7 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return vL < vR; } else if (column == TorrentModelItem::TR_PRIORITY) { - const int vL = left.data().toInt(); - const int vR = right.data().toInt(); - - // Seeding torrents should be sorted by their completed date instead. - if (vL == -1 && vR == -1) { - QAbstractItemModel *model = sourceModel(); - const QDateTime dateL = model->data(model->index(left.row(), TorrentModelItem::TR_SEED_DATE)).toDateTime(); - const QDateTime dateR = model->data(model->index(right.row(), TorrentModelItem::TR_SEED_DATE)).toDateTime(); - - //not valid dates should be sorted at the bottom. - if (!dateL.isValid()) return false; - if (!dateR.isValid()) return true; - - return dateL < dateR; - } - - // Seeding torrents should be at the bottom - if (vL == -1) return false; - if (vR == -1) return true; - return vL < vR; + return lowerPositionThan(left, right); } else if (column == TorrentModelItem::TR_PEERS || column == TorrentModelItem::TR_SEEDS) { int left_active = left.data().toInt(); @@ -142,9 +126,14 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex int right_total = right.data(Qt::UserRole).toInt(); // Active peers/seeds take precedence over total peers/seeds. - if (left_active == right_active) + if (left_active == right_active) { + if (left_total == right_total) + return lowerPositionThan(left, right); return (left_total < right_total); - else return (left_active < right_active); + } + else { + return (left_active < right_active); + } } else if (column == TorrentModelItem::TR_ETA) { const QAbstractItemModel *model = sourceModel(); @@ -212,7 +201,7 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex } } else if ((invalidL == false) && (invalidR == false)) { - return QSortFilterProxyModel::lessThan(left, right); + return lowerPositionThan(left, right); } else { return !invalidL; @@ -237,9 +226,46 @@ bool TransferListSortModel::lessThan(const QModelIndex &left, const QModelIndex return vL < vR; } + if (left.data() == right.data()) + return lowerPositionThan(left, right); + return QSortFilterProxyModel::lessThan(left, right); } +bool TransferListSortModel::lowerPositionThan(const QModelIndex &left, const QModelIndex &right) const +{ + const TorrentModel *model = dynamic_cast(sourceModel()); + + // Sort according to TR_PRIORITY + const int queueL = model->data(model->index(left.row(), TorrentModelItem::TR_PRIORITY)).toInt(); + const int queueR = model->data(model->index(right.row(), TorrentModelItem::TR_PRIORITY)).toInt(); + if (!(queueL < 0 && queueR < 0)) { + if (queueL > 0 && queueR > 0) + return queueL < queueR; + else if (queueL < 0) + return false; + else + return true; + } + + // Sort according to TR_SEED_DATE + const QDateTime dateL = model->data(model->index(left.row(), TorrentModelItem::TR_SEED_DATE)).toDateTime(); + const QDateTime dateR = model->data(model->index(right.row(), TorrentModelItem::TR_SEED_DATE)).toDateTime(); + if (dateL.isValid() && dateR.isValid()) { + if (dateL != dateR) + return dateL < dateR; + } + else if (dateL.isValid()) + return false; + else if (dateR.isValid()) + return true; + + // Finally, sort by hash + const QString hashL(model->torrentHash(left.row())); + const QString hashR(model->torrentHash(right.row())); + return hashL < hashR; +} + bool TransferListSortModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const { return matchStatusFilter(sourceRow, sourceParent) diff --git a/src/gui/transferlistsortmodel.h b/src/gui/transferlistsortmodel.h index 797c542dc..ec59587a6 100644 --- a/src/gui/transferlistsortmodel.h +++ b/src/gui/transferlistsortmodel.h @@ -50,6 +50,7 @@ public: private: bool lessThan(const QModelIndex &left, const QModelIndex &right) const; + bool lowerPositionThan(const QModelIndex &left, const QModelIndex &right) const; bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const; bool matchStatusFilter(int sourceRow, const QModelIndex &sourceParent) const;