Skip to content

Commit b5fb559

Browse files
hebastoknst
hebasto
authored and
knst
committed
Merge bitcoin-core/gui#18: Add peertablesortproxy module
5a4a15d qt, refactor: Drop no longer used PeerTableModel::getRowByNodeId func (Hennadii Stepanov) 9a9f180 qt, refactor: Drop no longer used PeerTableModel::sort function (Hennadii Stepanov) 778a64a qt: Use PeerTableSortProxy for sorting peer table (Hennadii Stepanov) df2d165 qt: Add peertablesortproxy module (Hennadii Stepanov) Pull request description: The "Peers" table in the "Node" window does not hold multiple selection after sorting. This PR introduces a `QSortFilterProxyModel` subclass, that is a standard Qt [practice](https://doc.qt.io/qt-5/model-view-programming.html#custom-sorting-models) for such cases. Now the sorting code is encapsulated into the dedicated Qt class, and we do not need to maintain it. Fixes #283 (additionally). --- On **master** (7ae86b3): - rows are sorted by "Ping", and a selection is made ![Screenshot from 2020-11-28 22-53-11](https://user-images.githubusercontent.com/32963518/100525900-96eaed00-31cc-11eb-86e7-72ede3b8b33c.png) - rows are sorted by "NodeId", and the previous selection is _lost_ ![Screenshot from 2020-11-28 22-53-21](https://user-images.githubusercontent.com/32963518/100525904-9c483780-31cc-11eb-957c-06f53d7d31ab.png) With **this PR**: - rows are sorted by "Ping", and a selection is made ![Screenshot from 2020-11-28 22-39-41](https://user-images.githubusercontent.com/32963518/100525776-06aca800-31cc-11eb-8c4e-9c6566fe80fe.png) - rows are sorted by "NodeId", and the row are still selected ![Screenshot from 2020-11-28 22-39-53](https://user-images.githubusercontent.com/32963518/100525791-2348e000-31cc-11eb-8b78-716a5551d7ec.png) ACKs for top commit: jarolrod: re-ACK 5a4a15d, tested on macOS 11.2 Qt 5.15.2 after rebase promag: Tested ACK 5a4a15d. Tree-SHA512: f81c1385892fbf1a46ffb98b42094ca1cc97da52114bbbc94fedb553899b1f18c26a349e186bba6e27922a89426bd61e8bc88b1f7832512dbe211b5f834e076e
1 parent 1cdd9fb commit b5fb559

File tree

9 files changed

+91
-155
lines changed

9 files changed

+91
-155
lines changed

‎src/Makefile.qt.include‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ QT_MOC_CPP = \
6969
qt/moc_optionsmodel.cpp \
7070
qt/moc_overviewpage.cpp \
7171
qt/moc_peertablemodel.cpp \
72+
qt/moc_peertablesortproxy.cpp \
7273
qt/moc_paymentserver.cpp \
7374
qt/moc_psbtoperationsdialog.cpp \
7475
qt/moc_qrdialog.cpp \
@@ -146,6 +147,7 @@ BITCOIN_QT_H = \
146147
qt/overviewpage.h \
147148
qt/paymentserver.h \
148149
qt/peertablemodel.h \
150+
qt/peertablesortproxy.h \
149151
qt/psbtoperationsdialog.h \
150152
qt/qrdialog.h \
151153
qt/qrimagewidget.h \
@@ -227,6 +229,7 @@ BITCOIN_QT_BASE_CPP = \
227229
qt/optionsdialog.cpp \
228230
qt/optionsmodel.cpp \
229231
qt/peertablemodel.cpp \
232+
qt/peertablesortproxy.cpp \
230233
qt/qvalidatedlineedit.cpp \
231234
qt/qvaluecombobox.cpp \
232235
qt/rpcconsole.cpp \

‎src/qt/clientmodel.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <qt/guiconstants.h>
1010
#include <qt/guiutil.h>
1111
#include <qt/peertablemodel.h>
12+
#include <qt/peertablesortproxy.h>
1213

1314
#include <evo/deterministicmns.h>
1415

@@ -42,7 +43,11 @@ ClientModel::ClientModel(interfaces::Node& node, OptionsModel *_optionsModel, QO
4243
{
4344
cachedBestHeaderHeight = -1;
4445
cachedBestHeaderTime = -1;
46+
4547
peerTableModel = new PeerTableModel(m_node, this);
48+
m_peer_table_sort_proxy = new PeerTableSortProxy(this);
49+
m_peer_table_sort_proxy->setSourceModel(peerTableModel);
50+
4651
banTableModel = new BanTableModel(m_node, this);
4752
mnListCached = std::make_shared<CDeterministicMNList>();
4853

@@ -219,6 +224,11 @@ PeerTableModel *ClientModel::getPeerTableModel()
219224
return peerTableModel;
220225
}
221226

227+
PeerTableSortProxy* ClientModel::peerTableSortProxy()
228+
{
229+
return m_peer_table_sort_proxy;
230+
}
231+
222232
BanTableModel *ClientModel::getBanTableModel()
223233
{
224234
return banTableModel;

‎src/qt/clientmodel.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class BanTableModel;
2020
class CBlockIndex;
2121
class OptionsModel;
2222
class PeerTableModel;
23+
class PeerTableSortProxy;
2324
enum class SynchronizationState;
2425

2526
QT_BEGIN_NAMESPACE
@@ -58,6 +59,7 @@ class ClientModel : public QObject
5859
interfaces::CoinJoin::Options& coinJoinOptions() const { return m_node.coinJoinOptions(); }
5960
OptionsModel *getOptionsModel();
6061
PeerTableModel *getPeerTableModel();
62+
PeerTableSortProxy* peerTableSortProxy();
6163
BanTableModel *getBanTableModel();
6264

6365
//! Return number of connections, default is in- and outbound (total)
@@ -109,6 +111,7 @@ class ClientModel : public QObject
109111
std::unique_ptr<interfaces::Handler> m_handler_notify_additional_data_sync_progess_changed;
110112
OptionsModel *optionsModel;
111113
PeerTableModel *peerTableModel;
114+
PeerTableSortProxy* m_peer_table_sort_proxy{nullptr};
112115
BanTableModel *banTableModel;
113116

114117
//! A thread to interact with m_node asynchronously

‎src/qt/peertablemodel.cpp‎

Lines changed: 1 addition & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -11,56 +11,19 @@
1111

1212
#include <utility>
1313

14-
#include <QDebug>
1514
#include <QList>
1615
#include <QTimer>
1716

18-
bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombinedStats &right) const
19-
{
20-
const CNodeStats *pLeft = &(left.nodeStats);
21-
const CNodeStats *pRight = &(right.nodeStats);
22-
23-
if (order == Qt::DescendingOrder)
24-
std::swap(pLeft, pRight);
25-
26-
switch (static_cast<PeerTableModel::ColumnIndex>(column)) {
27-
case PeerTableModel::NetNodeId:
28-
return pLeft->nodeid < pRight->nodeid;
29-
case PeerTableModel::Address:
30-
return pLeft->m_addr_name.compare(pRight->m_addr_name) < 0;
31-
case PeerTableModel::ConnectionType:
32-
return pLeft->m_conn_type < pRight->m_conn_type;
33-
case PeerTableModel::Network:
34-
return pLeft->m_network < pRight->m_network;
35-
case PeerTableModel::Ping:
36-
return pLeft->m_min_ping_time < pRight->m_min_ping_time;
37-
case PeerTableModel::Sent:
38-
return pLeft->nSendBytes < pRight->nSendBytes;
39-
case PeerTableModel::Received:
40-
return pLeft->nRecvBytes < pRight->nRecvBytes;
41-
case PeerTableModel::Subversion:
42-
return pLeft->cleanSubVer.compare(pRight->cleanSubVer) < 0;
43-
} // no default case, so the compiler can warn about missing cases
44-
assert(false);
45-
}
46-
4717
// private implementation
4818
class PeerTablePriv
4919
{
5020
public:
5121
/** Local cache of peer information */
5222
QList<CNodeCombinedStats> cachedNodeStats;
53-
/** Column to sort nodes by (default to unsorted) */
54-
int sortColumn{-1};
55-
/** Order (ascending or descending) to sort nodes by */
56-
Qt::SortOrder sortOrder;
57-
/** Index of rows by node ID */
58-
std::map<NodeId, int> mapNodeRows;
5923

6024
/** Pull a full list of peers from vNodes into our cache */
6125
void refreshPeers(interfaces::Node& node)
6226
{
63-
{
6427
cachedNodeStats.clear();
6528

6629
interfaces::Node::NodesStats nodes_stats;
@@ -75,17 +38,6 @@ class PeerTablePriv
7538
stats.nodeStateStats = std::get<2>(node_stats);
7639
cachedNodeStats.append(stats);
7740
}
78-
}
79-
80-
if (sortColumn >= 0)
81-
// sort cacheNodeStats (use stable sort to prevent rows jumping around unnecessarily)
82-
std::stable_sort(cachedNodeStats.begin(), cachedNodeStats.end(), NodeLessThan(sortColumn, sortOrder));
83-
84-
// build index map
85-
mapNodeRows.clear();
86-
int row = 0;
87-
for (const CNodeCombinedStats& stats : cachedNodeStats)
88-
mapNodeRows.insert(std::pair<NodeId, int>(stats.nodeStats.nodeid, row++));
8941
}
9042

9143
int size() const
@@ -196,10 +148,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const
196148
} // no default case, so the compiler can warn about missing cases
197149
assert(false);
198150
} else if (role == StatsRole) {
199-
switch (index.column()) {
200-
case NetNodeId: return QVariant::fromValue(rec);
201-
default: return QVariant();
202-
}
151+
return QVariant::fromValue(rec);
203152
}
204153

205154
return QVariant();
@@ -241,19 +190,3 @@ void PeerTableModel::refresh()
241190
priv->refreshPeers(m_node);
242191
Q_EMIT layoutChanged();
243192
}
244-
245-
int PeerTableModel::getRowByNodeId(NodeId nodeid)
246-
{
247-
std::map<NodeId, int>::iterator it = priv->mapNodeRows.find(nodeid);
248-
if (it == priv->mapNodeRows.end())
249-
return -1;
250-
251-
return it->second;
252-
}
253-
254-
void PeerTableModel::sort(int column, Qt::SortOrder order)
255-
{
256-
priv->sortColumn = column;
257-
priv->sortOrder = order;
258-
refresh();
259-
}

‎src/qt/peertablemodel.h‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,6 @@ struct CNodeCombinedStats {
3030
};
3131
Q_DECLARE_METATYPE(CNodeCombinedStats*)
3232

33-
class NodeLessThan
34-
{
35-
public:
36-
NodeLessThan(int nColumn, Qt::SortOrder fOrder) :
37-
column(nColumn), order(fOrder) {}
38-
bool operator()(const CNodeCombinedStats &left, const CNodeCombinedStats &right) const;
39-
40-
private:
41-
int column;
42-
Qt::SortOrder order;
43-
};
44-
4533
/**
4634
Qt model providing information about connected peers, similar to the
4735
"getpeerinfo" RPC call. Used by the rpc console UI.
@@ -53,7 +41,6 @@ class PeerTableModel : public QAbstractTableModel
5341
public:
5442
explicit PeerTableModel(interfaces::Node& node, QObject* parent);
5543
~PeerTableModel();
56-
int getRowByNodeId(NodeId nodeid);
5744
void startAutoRefresh();
5845
void stopAutoRefresh();
5946

@@ -80,7 +67,6 @@ class PeerTableModel : public QAbstractTableModel
8067
QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
8168
QModelIndex index(int row, int column, const QModelIndex &parent) const override;
8269
Qt::ItemFlags flags(const QModelIndex &index) const override;
83-
void sort(int column, Qt::SortOrder order) override;
8470
/*@}*/
8571

8672
public Q_SLOTS:

‎src/qt/peertablesortproxy.cpp‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) 2020 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <qt/peertablesortproxy.h>
6+
7+
#include <qt/peertablemodel.h>
8+
#include <util/check.h>
9+
10+
#include <QModelIndex>
11+
#include <QString>
12+
#include <QVariant>
13+
14+
PeerTableSortProxy::PeerTableSortProxy(QObject* parent)
15+
: QSortFilterProxyModel(parent)
16+
{
17+
}
18+
19+
bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelIndex& right_index) const
20+
{
21+
const CNodeStats left_stats = Assert(sourceModel()->data(left_index, PeerTableModel::StatsRole).value<CNodeCombinedStats*>())->nodeStats;
22+
const CNodeStats right_stats = Assert(sourceModel()->data(right_index, PeerTableModel::StatsRole).value<CNodeCombinedStats*>())->nodeStats;
23+
24+
switch (static_cast<PeerTableModel::ColumnIndex>(left_index.column())) {
25+
case PeerTableModel::NetNodeId:
26+
return left_stats.nodeid < right_stats.nodeid;
27+
case PeerTableModel::Address:
28+
return left_stats.m_addr_name.compare(right_stats.m_addr_name) < 0;
29+
case PeerTableModel::ConnectionType:
30+
return left_stats.m_conn_type < right_stats.m_conn_type;
31+
case PeerTableModel::Network:
32+
return left_stats.m_network < right_stats.m_network;
33+
case PeerTableModel::Ping:
34+
return left_stats.m_min_ping_time < right_stats.m_min_ping_time;
35+
case PeerTableModel::Sent:
36+
return left_stats.nSendBytes < right_stats.nSendBytes;
37+
case PeerTableModel::Received:
38+
return left_stats.nRecvBytes < right_stats.nRecvBytes;
39+
case PeerTableModel::Subversion:
40+
return left_stats.cleanSubVer.compare(right_stats.cleanSubVer) < 0;
41+
} // no default case, so the compiler can warn about missing cases
42+
assert(false);
43+
}

‎src/qt/peertablesortproxy.h‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright (c) 2020 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_QT_PEERTABLESORTPROXY_H
6+
#define BITCOIN_QT_PEERTABLESORTPROXY_H
7+
8+
#include <QSortFilterProxyModel>
9+
10+
QT_BEGIN_NAMESPACE
11+
class QModelIndex;
12+
QT_END_NAMESPACE
13+
14+
class PeerTableSortProxy : public QSortFilterProxyModel
15+
{
16+
Q_OBJECT
17+
18+
public:
19+
explicit PeerTableSortProxy(QObject* parent = nullptr);
20+
21+
protected:
22+
bool lessThan(const QModelIndex& left_index, const QModelIndex& right_index) const override;
23+
};
24+
25+
#endif // BITCOIN_QT_PEERTABLESORTPROXY_H

‎src/qt/rpcconsole.cpp‎

Lines changed: 6 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@
1212

1313
#include <evo/deterministicmns.h>
1414

15-
#include <qt/bantablemodel.h>
16-
#include <qt/clientmodel.h>
17-
#include <qt/walletmodel.h>
1815
#include <chainparams.h>
1916
#include <interfaces/node.h>
2017
#include <netbase.h>
18+
#include <qt/bantablemodel.h>
19+
#include <qt/clientmodel.h>
20+
#include <qt/peertablesortproxy.h>
21+
#include <qt/walletmodel.h>
2122
#include <rpc/client.h>
2223
#include <rpc/server.h>
2324
#include <util/strencodings.h>
@@ -694,7 +695,7 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
694695

695696

696697
// set up peer table
697-
ui->peerWidget->setModel(model->getPeerTableModel());
698+
ui->peerWidget->setModel(model->peerTableSortProxy());
698699
ui->peerWidget->verticalHeader()->hide();
699700
ui->peerWidget->setSelectionBehavior(QAbstractItemView::SelectRows);
700701
ui->peerWidget->setSelectionMode(QAbstractItemView::ExtendedSelection);
@@ -716,10 +717,7 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_
716717

717718
// peer table signal handling - update peer details when selecting new node
718719
connect(ui->peerWidget->selectionModel(), &QItemSelectionModel::selectionChanged, this, &RPCConsole::updateDetailWidget);
719-
// peer table signal handling - update peer details when new nodes are added to the model
720-
connect(model->getPeerTableModel(), &PeerTableModel::layoutChanged, this, &RPCConsole::peerLayoutChanged);
721-
// peer table signal handling - cache selected node ids
722-
connect(model->getPeerTableModel(), &PeerTableModel::layoutAboutToBeChanged, this, &RPCConsole::peerLayoutAboutToChange);
720+
connect(model->getPeerTableModel(), &PeerTableModel::layoutChanged, this, &RPCConsole::updateDetailWidget);
723721

724722
// set up ban table
725723
ui->banlistWidget->setModel(model->getBanTableModel());
@@ -1198,67 +1196,6 @@ void RPCConsole::setTrafficGraphRange(TrafficGraphData::GraphRange range)
11981196
ui->lblGraphRange->setText(GUIUtil::formatDurationStr(std::chrono::minutes{TrafficGraphData::RangeMinutes[range]}));
11991197
}
12001198

1201-
void RPCConsole::peerLayoutAboutToChange()
1202-
{
1203-
cachedNodeids.clear();
1204-
for (const QModelIndex& peer : GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId)) {
1205-
const auto stats = peer.data(PeerTableModel::StatsRole).value<CNodeCombinedStats*>();
1206-
cachedNodeids.append(stats->nodeStats.nodeid);
1207-
}
1208-
}
1209-
1210-
void RPCConsole::peerLayoutChanged()
1211-
{
1212-
if (!clientModel || !clientModel->getPeerTableModel())
1213-
return;
1214-
1215-
bool fUnselect = false;
1216-
bool fReselect = false;
1217-
1218-
if (cachedNodeids.empty()) // no node selected yet
1219-
return;
1220-
1221-
// find the currently selected row
1222-
int selectedRow = -1;
1223-
QModelIndexList selectedModelIndex = ui->peerWidget->selectionModel()->selectedIndexes();
1224-
if (!selectedModelIndex.isEmpty()) {
1225-
selectedRow = selectedModelIndex.first().row();
1226-
}
1227-
1228-
// check if our detail node has a row in the table (it may not necessarily
1229-
// be at selectedRow since its position can change after a layout change)
1230-
int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(cachedNodeids.first());
1231-
1232-
if (detailNodeRow < 0)
1233-
{
1234-
// detail node disappeared from table (node disconnected)
1235-
fUnselect = true;
1236-
}
1237-
else
1238-
{
1239-
if (detailNodeRow != selectedRow)
1240-
{
1241-
// detail node moved position
1242-
fUnselect = true;
1243-
fReselect = true;
1244-
}
1245-
}
1246-
1247-
if (fUnselect && selectedRow >= 0) {
1248-
clearSelectedNode();
1249-
}
1250-
1251-
if (fReselect)
1252-
{
1253-
for(int i = 0; i < cachedNodeids.size(); i++)
1254-
{
1255-
ui->peerWidget->selectRow(clientModel->getPeerTableModel()->getRowByNodeId(cachedNodeids.at(i)));
1256-
}
1257-
}
1258-
1259-
updateDetailWidget();
1260-
}
1261-
12621199
void RPCConsole::updateDetailWidget()
12631200
{
12641201
const QList<QModelIndex> selected_peers = GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId);

0 commit comments

Comments
 (0)