diff options
author | Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> | 2021-04-28 20:51:58 +0300 |
---|---|---|
committer | Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> | 2021-04-28 20:57:52 +0300 |
commit | 328da3355787e76184d6bb16e6cf04eca760cbc6 (patch) | |
tree | af3aafac1bfa4b5005d65bdde931409b12edbbff /src | |
parent | 549d20a31beaf71c722970af32d0076b70b5ffcb (diff) | |
parent | 5a4a15d2b4456272fd8aa080195f40a09576ae01 (diff) | |
download | bitcoin-328da3355787e76184d6bb16e6cf04eca760cbc6.tar.xz |
Merge bitcoin-core/gui#18: Add peertablesortproxy module
5a4a15d2b4456272fd8aa080195f40a09576ae01 qt, refactor: Drop no longer used PeerTableModel::getRowByNodeId func (Hennadii Stepanov)
9a9f180df0d51396fee2468681df6dd935b0248e qt, refactor: Drop no longer used PeerTableModel::sort function (Hennadii Stepanov)
778a64af209e4fa692a3aca8376ba1bd5e1af881 qt: Use PeerTableSortProxy for sorting peer table (Hennadii Stepanov)
df2d165ba9e0acc53f36a326f68f57ad9c297872 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** (7ae86b3c6845873ca96650fc69beb4ae5285c801):
- 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 5a4a15d2b4456272fd8aa080195f40a09576ae01, tested on macOS 11.2 Qt 5.15.2 after rebase
promag:
Tested ACK 5a4a15d2b4456272fd8aa080195f40a09576ae01.
Tree-SHA512: f81c1385892fbf1a46ffb98b42094ca1cc97da52114bbbc94fedb553899b1f18c26a349e186bba6e27922a89426bd61e8bc88b1f7832512dbe211b5f834e076e
Diffstat (limited to 'src')
-rw-r--r-- | src/Makefile.qt.include | 3 | ||||
-rw-r--r-- | src/qt/clientmodel.cpp | 10 | ||||
-rw-r--r-- | src/qt/clientmodel.h | 3 | ||||
-rw-r--r-- | src/qt/peertablemodel.cpp | 69 | ||||
-rw-r--r-- | src/qt/peertablemodel.h | 14 | ||||
-rw-r--r-- | src/qt/peertablesortproxy.cpp | 43 | ||||
-rw-r--r-- | src/qt/peertablesortproxy.h | 25 | ||||
-rw-r--r-- | src/qt/rpcconsole.cpp | 75 | ||||
-rw-r--r-- | src/qt/rpcconsole.h | 4 |
9 files changed, 91 insertions, 155 deletions
diff --git a/src/Makefile.qt.include b/src/Makefile.qt.include index a573247afa..caa8500ffa 100644 --- a/src/Makefile.qt.include +++ b/src/Makefile.qt.include @@ -61,6 +61,7 @@ QT_MOC_CPP = \ qt/moc_optionsmodel.cpp \ qt/moc_overviewpage.cpp \ qt/moc_peertablemodel.cpp \ + qt/moc_peertablesortproxy.cpp \ qt/moc_paymentserver.cpp \ qt/moc_psbtoperationsdialog.cpp \ qt/moc_qrimagewidget.cpp \ @@ -134,6 +135,7 @@ BITCOIN_QT_H = \ qt/overviewpage.h \ qt/paymentserver.h \ qt/peertablemodel.h \ + qt/peertablesortproxy.h \ qt/platformstyle.h \ qt/psbtoperationsdialog.h \ qt/qrimagewidget.h \ @@ -232,6 +234,7 @@ BITCOIN_QT_BASE_CPP = \ qt/optionsdialog.cpp \ qt/optionsmodel.cpp \ qt/peertablemodel.cpp \ + qt/peertablesortproxy.cpp \ qt/platformstyle.cpp \ qt/qvalidatedlineedit.cpp \ qt/qvaluecombobox.cpp \ diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp index f2c555de52..04161020b2 100644 --- a/src/qt/clientmodel.cpp +++ b/src/qt/clientmodel.cpp @@ -8,6 +8,7 @@ #include <qt/guiconstants.h> #include <qt/guiutil.h> #include <qt/peertablemodel.h> +#include <qt/peertablesortproxy.h> #include <clientversion.h> #include <interfaces/handler.h> @@ -38,7 +39,11 @@ ClientModel::ClientModel(interfaces::Node& node, OptionsModel *_optionsModel, QO { cachedBestHeaderHeight = -1; cachedBestHeaderTime = -1; + peerTableModel = new PeerTableModel(m_node, this); + m_peer_table_sort_proxy = new PeerTableSortProxy(this); + m_peer_table_sort_proxy->setSourceModel(peerTableModel); + banTableModel = new BanTableModel(m_node, this); QTimer* timer = new QTimer; @@ -184,6 +189,11 @@ PeerTableModel *ClientModel::getPeerTableModel() return peerTableModel; } +PeerTableSortProxy* ClientModel::peerTableSortProxy() +{ + return m_peer_table_sort_proxy; +} + BanTableModel *ClientModel::getBanTableModel() { return banTableModel; diff --git a/src/qt/clientmodel.h b/src/qt/clientmodel.h index 7ac4cc040b..7a199ef19c 100644 --- a/src/qt/clientmodel.h +++ b/src/qt/clientmodel.h @@ -17,6 +17,7 @@ class BanTableModel; class CBlockIndex; class OptionsModel; class PeerTableModel; +class PeerTableSortProxy; enum class SynchronizationState; namespace interfaces { @@ -54,6 +55,7 @@ public: interfaces::Node& node() const { return m_node; } OptionsModel *getOptionsModel(); PeerTableModel *getPeerTableModel(); + PeerTableSortProxy* peerTableSortProxy(); BanTableModel *getBanTableModel(); //! Return number of connections, default is in- and outbound (total) @@ -96,6 +98,7 @@ private: std::unique_ptr<interfaces::Handler> m_handler_notify_header_tip; OptionsModel *optionsModel; PeerTableModel *peerTableModel; + PeerTableSortProxy* m_peer_table_sort_proxy{nullptr}; BanTableModel *banTableModel; //! A thread to interact with m_node asynchronously diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 3459bf4cf8..6c4e326011 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -11,56 +11,19 @@ #include <utility> -#include <QDebug> #include <QList> #include <QTimer> -bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombinedStats &right) const -{ - const CNodeStats *pLeft = &(left.nodeStats); - const CNodeStats *pRight = &(right.nodeStats); - - if (order == Qt::DescendingOrder) - std::swap(pLeft, pRight); - - switch (static_cast<PeerTableModel::ColumnIndex>(column)) { - case PeerTableModel::NetNodeId: - return pLeft->nodeid < pRight->nodeid; - case PeerTableModel::Address: - return pLeft->addrName.compare(pRight->addrName) < 0; - case PeerTableModel::ConnectionType: - return pLeft->m_conn_type < pRight->m_conn_type; - case PeerTableModel::Network: - return pLeft->m_network < pRight->m_network; - case PeerTableModel::Ping: - return pLeft->m_min_ping_time < pRight->m_min_ping_time; - case PeerTableModel::Sent: - return pLeft->nSendBytes < pRight->nSendBytes; - case PeerTableModel::Received: - return pLeft->nRecvBytes < pRight->nRecvBytes; - case PeerTableModel::Subversion: - return pLeft->cleanSubVer.compare(pRight->cleanSubVer) < 0; - } // no default case, so the compiler can warn about missing cases - assert(false); -} - // private implementation class PeerTablePriv { public: /** Local cache of peer information */ QList<CNodeCombinedStats> cachedNodeStats; - /** Column to sort nodes by (default to unsorted) */ - int sortColumn{-1}; - /** Order (ascending or descending) to sort nodes by */ - Qt::SortOrder sortOrder; - /** Index of rows by node ID */ - std::map<NodeId, int> mapNodeRows; /** Pull a full list of peers from vNodes into our cache */ void refreshPeers(interfaces::Node& node) { - { cachedNodeStats.clear(); interfaces::Node::NodesStats nodes_stats; @@ -74,17 +37,6 @@ public: stats.nodeStateStats = std::get<2>(node_stats); cachedNodeStats.append(stats); } - } - - if (sortColumn >= 0) - // sort cacheNodeStats (use stable sort to prevent rows jumping around unnecessarily) - std::stable_sort(cachedNodeStats.begin(), cachedNodeStats.end(), NodeLessThan(sortColumn, sortOrder)); - - // build index map - mapNodeRows.clear(); - int row = 0; - for (const CNodeCombinedStats& stats : cachedNodeStats) - mapNodeRows.insert(std::pair<NodeId, int>(stats.nodeStats.nodeid, row++)); } int size() const @@ -194,10 +146,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const } // no default case, so the compiler can warn about missing cases assert(false); } else if (role == StatsRole) { - switch (index.column()) { - case NetNodeId: return QVariant::fromValue(rec); - default: return QVariant(); - } + return QVariant::fromValue(rec); } return QVariant(); @@ -239,19 +188,3 @@ void PeerTableModel::refresh() priv->refreshPeers(m_node); Q_EMIT layoutChanged(); } - -int PeerTableModel::getRowByNodeId(NodeId nodeid) -{ - std::map<NodeId, int>::iterator it = priv->mapNodeRows.find(nodeid); - if (it == priv->mapNodeRows.end()) - return -1; - - return it->second; -} - -void PeerTableModel::sort(int column, Qt::SortOrder order) -{ - priv->sortColumn = column; - priv->sortOrder = order; - refresh(); -} diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 0823235ec0..9c7bc25da2 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -30,18 +30,6 @@ struct CNodeCombinedStats { }; Q_DECLARE_METATYPE(CNodeCombinedStats*) -class NodeLessThan -{ -public: - NodeLessThan(int nColumn, Qt::SortOrder fOrder) : - column(nColumn), order(fOrder) {} - bool operator()(const CNodeCombinedStats &left, const CNodeCombinedStats &right) const; - -private: - int column; - Qt::SortOrder order; -}; - /** Qt model providing information about connected peers, similar to the "getpeerinfo" RPC call. Used by the rpc console UI. @@ -53,7 +41,6 @@ class PeerTableModel : public QAbstractTableModel public: explicit PeerTableModel(interfaces::Node& node, QObject* parent); ~PeerTableModel(); - int getRowByNodeId(NodeId nodeid); void startAutoRefresh(); void stopAutoRefresh(); @@ -80,7 +67,6 @@ public: QVariant headerData(int section, Qt::Orientation orientation, int role) const override; QModelIndex index(int row, int column, const QModelIndex &parent) const override; Qt::ItemFlags flags(const QModelIndex &index) const override; - void sort(int column, Qt::SortOrder order) override; /*@}*/ public Q_SLOTS: diff --git a/src/qt/peertablesortproxy.cpp b/src/qt/peertablesortproxy.cpp new file mode 100644 index 0000000000..78932da8d4 --- /dev/null +++ b/src/qt/peertablesortproxy.cpp @@ -0,0 +1,43 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <qt/peertablesortproxy.h> + +#include <qt/peertablemodel.h> +#include <util/check.h> + +#include <QModelIndex> +#include <QString> +#include <QVariant> + +PeerTableSortProxy::PeerTableSortProxy(QObject* parent) + : QSortFilterProxyModel(parent) +{ +} + +bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelIndex& right_index) const +{ + const CNodeStats left_stats = Assert(sourceModel()->data(left_index, PeerTableModel::StatsRole).value<CNodeCombinedStats*>())->nodeStats; + const CNodeStats right_stats = Assert(sourceModel()->data(right_index, PeerTableModel::StatsRole).value<CNodeCombinedStats*>())->nodeStats; + + switch (static_cast<PeerTableModel::ColumnIndex>(left_index.column())) { + case PeerTableModel::NetNodeId: + return left_stats.nodeid < right_stats.nodeid; + case PeerTableModel::Address: + return left_stats.addrName.compare(right_stats.addrName) < 0; + case PeerTableModel::ConnectionType: + return left_stats.m_conn_type < right_stats.m_conn_type; + case PeerTableModel::Network: + return left_stats.m_network < right_stats.m_network; + case PeerTableModel::Ping: + return left_stats.m_min_ping_time < right_stats.m_min_ping_time; + case PeerTableModel::Sent: + return left_stats.nSendBytes < right_stats.nSendBytes; + case PeerTableModel::Received: + return left_stats.nRecvBytes < right_stats.nRecvBytes; + case PeerTableModel::Subversion: + return left_stats.cleanSubVer.compare(right_stats.cleanSubVer) < 0; + } // no default case, so the compiler can warn about missing cases + assert(false); +} diff --git a/src/qt/peertablesortproxy.h b/src/qt/peertablesortproxy.h new file mode 100644 index 0000000000..1879f6b400 --- /dev/null +++ b/src/qt/peertablesortproxy.h @@ -0,0 +1,25 @@ +// Copyright (c) 2020 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_QT_PEERTABLESORTPROXY_H +#define BITCOIN_QT_PEERTABLESORTPROXY_H + +#include <QSortFilterProxyModel> + +QT_BEGIN_NAMESPACE +class QModelIndex; +QT_END_NAMESPACE + +class PeerTableSortProxy : public QSortFilterProxyModel +{ + Q_OBJECT + +public: + explicit PeerTableSortProxy(QObject* parent = nullptr); + +protected: + bool lessThan(const QModelIndex& left_index, const QModelIndex& right_index) const override; +}; + +#endif // BITCOIN_QT_PEERTABLESORTPROXY_H diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 85412ca75b..006f60e7a1 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -9,13 +9,14 @@ #include <qt/rpcconsole.h> #include <qt/forms/ui_debugwindow.h> +#include <chainparams.h> +#include <interfaces/node.h> +#include <netbase.h> #include <qt/bantablemodel.h> #include <qt/clientmodel.h> +#include <qt/peertablesortproxy.h> #include <qt/platformstyle.h> #include <qt/walletmodel.h> -#include <chainparams.h> -#include <interfaces/node.h> -#include <netbase.h> #include <rpc/client.h> #include <rpc/server.h> #include <util/strencodings.h> @@ -606,7 +607,7 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_ connect(model, &ClientModel::mempoolSizeChanged, this, &RPCConsole::setMempoolSize); // set up peer table - ui->peerWidget->setModel(model->getPeerTableModel()); + ui->peerWidget->setModel(model->peerTableSortProxy()); ui->peerWidget->verticalHeader()->hide(); ui->peerWidget->setSelectionBehavior(QAbstractItemView::SelectRows); ui->peerWidget->setSelectionMode(QAbstractItemView::ExtendedSelection); @@ -627,10 +628,7 @@ void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_ // peer table signal handling - update peer details when selecting new node connect(ui->peerWidget->selectionModel(), &QItemSelectionModel::selectionChanged, this, &RPCConsole::updateDetailWidget); - // peer table signal handling - update peer details when new nodes are added to the model - connect(model->getPeerTableModel(), &PeerTableModel::layoutChanged, this, &RPCConsole::peerLayoutChanged); - // peer table signal handling - cache selected node ids - connect(model->getPeerTableModel(), &PeerTableModel::layoutAboutToBeChanged, this, &RPCConsole::peerLayoutAboutToChange); + connect(model->getPeerTableModel(), &PeerTableModel::layoutChanged, this, &RPCConsole::updateDetailWidget); // set up ban table ui->banlistWidget->setModel(model->getBanTableModel()); @@ -1014,67 +1012,6 @@ void RPCConsole::updateTrafficStats(quint64 totalBytesIn, quint64 totalBytesOut) ui->lblBytesOut->setText(GUIUtil::formatBytes(totalBytesOut)); } -void RPCConsole::peerLayoutAboutToChange() -{ - cachedNodeids.clear(); - for (const QModelIndex& peer : GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId)) { - const auto stats = peer.data(PeerTableModel::StatsRole).value<CNodeCombinedStats*>(); - cachedNodeids.append(stats->nodeStats.nodeid); - } -} - -void RPCConsole::peerLayoutChanged() -{ - if (!clientModel || !clientModel->getPeerTableModel()) - return; - - bool fUnselect = false; - bool fReselect = false; - - if (cachedNodeids.empty()) // no node selected yet - return; - - // find the currently selected row - int selectedRow = -1; - QModelIndexList selectedModelIndex = ui->peerWidget->selectionModel()->selectedIndexes(); - if (!selectedModelIndex.isEmpty()) { - selectedRow = selectedModelIndex.first().row(); - } - - // check if our detail node has a row in the table (it may not necessarily - // be at selectedRow since its position can change after a layout change) - int detailNodeRow = clientModel->getPeerTableModel()->getRowByNodeId(cachedNodeids.first()); - - if (detailNodeRow < 0) - { - // detail node disappeared from table (node disconnected) - fUnselect = true; - } - else - { - if (detailNodeRow != selectedRow) - { - // detail node moved position - fUnselect = true; - fReselect = true; - } - } - - if (fUnselect && selectedRow >= 0) { - clearSelectedNode(); - } - - if (fReselect) - { - for(int i = 0; i < cachedNodeids.size(); i++) - { - ui->peerWidget->selectRow(clientModel->getPeerTableModel()->getRowByNodeId(cachedNodeids.at(i))); - } - } - - updateDetailWidget(); -} - void RPCConsole::updateDetailWidget() { const QList<QModelIndex> selected_peers = GUIUtil::getEntryData(ui->peerWidget, PeerTableModel::NetNodeId); diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index b9806e40c9..5182d60a0d 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -118,10 +118,6 @@ public Q_SLOTS: void browseHistory(int offset); /** Scroll console view to end */ void scrollToEnd(); - /** Handle selection caching before update */ - void peerLayoutAboutToChange(); - /** Handle updated peer information */ - void peerLayoutChanged(); /** Disconnect a selected node on the Peers tab */ void disconnectSelectedNode(); /** Ban a selected node on the Peers tab */ |