From efb7e5aa962d4a4047061996bbb50b6da4592cbc Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:54:43 +0200 Subject: [PATCH 1/3] qt, refactor: Use default arguments for overridden functions See Qt docs for QAbstractTableModel and QAbstractItemModel classes. --- src/qt/peertablemodel.cpp | 8 ++++---- src/qt/peertablemodel.h | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 11441481bb4..de567aacb7d 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -84,7 +84,7 @@ void PeerTableModel::stopAutoRefresh() timer->stop(); } -int PeerTableModel::rowCount(const QModelIndex &parent) const +int PeerTableModel::rowCount(const QModelIndex& parent) const { if (parent.isValid()) { return 0; @@ -92,7 +92,7 @@ int PeerTableModel::rowCount(const QModelIndex &parent) const return priv->size(); } -int PeerTableModel::columnCount(const QModelIndex &parent) const +int PeerTableModel::columnCount(const QModelIndex& parent) const { if (parent.isValid()) { return 0; @@ -100,7 +100,7 @@ int PeerTableModel::columnCount(const QModelIndex &parent) const return columns.length(); } -QVariant PeerTableModel::data(const QModelIndex &index, int role) const +QVariant PeerTableModel::data(const QModelIndex& index, int role) const { if(!index.isValid()) return QVariant(); @@ -172,7 +172,7 @@ Qt::ItemFlags PeerTableModel::flags(const QModelIndex &index) const return retval; } -QModelIndex PeerTableModel::index(int row, int column, const QModelIndex &parent) const +QModelIndex PeerTableModel::index(int row, int column, const QModelIndex& parent) const { Q_UNUSED(parent); CNodeCombinedStats *data = priv->index(row); diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 3d195342f19..dc2802d4a92 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -61,11 +61,11 @@ class PeerTableModel : public QAbstractTableModel /** @name Methods overridden from QAbstractTableModel @{*/ - int rowCount(const QModelIndex &parent) const override; - int columnCount(const QModelIndex &parent) const override; - QVariant data(const QModelIndex &index, int role) const override; - QVariant headerData(int section, Qt::Orientation orientation, int role) const override; - QModelIndex index(int row, int column, const QModelIndex &parent) const override; + int rowCount(const QModelIndex& parent = QModelIndex()) const override; + int columnCount(const QModelIndex& parent = QModelIndex()) const override; + QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override; + QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; + QModelIndex index(int row, int column, const QModelIndex& parent = QModelIndex()) const override; Qt::ItemFlags flags(const QModelIndex &index) const override; /*@}*/ From 1b66f6e556631a1a2d89aefba70a79894bd14fcd Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:55:10 +0200 Subject: [PATCH 2/3] qt: Drop PeerTablePriv class This commit does not change behavior. --- src/qt/peertablemodel.cpp | 61 ++++++++++----------------------------- src/qt/peertablemodel.h | 8 +++-- 2 files changed, 20 insertions(+), 49 deletions(-) diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index de567aacb7d..fe03365d76a 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -14,52 +14,11 @@ #include #include -// private implementation -class PeerTablePriv -{ -public: - /** Local cache of peer information */ - QList cachedNodeStats; - - /** Pull a full list of peers from vNodes into our cache */ - void refreshPeers(interfaces::Node& node) - { - cachedNodeStats.clear(); - - interfaces::Node::NodesStats nodes_stats; - node.getNodesStats(nodes_stats); - cachedNodeStats.reserve(nodes_stats.size()); - for (const auto& node_stats : nodes_stats) - { - CNodeCombinedStats stats; - stats.nodeStats = std::get<0>(node_stats); - stats.fNodeStateStatsAvailable = std::get<1>(node_stats); - stats.nodeStateStats = std::get<2>(node_stats); - cachedNodeStats.append(stats); - } - } - - int size() const - { - return cachedNodeStats.size(); - } - - CNodeCombinedStats *index(int idx) - { - if (idx >= 0 && idx < cachedNodeStats.size()) - return &cachedNodeStats[idx]; - - return nullptr; - } -}; - PeerTableModel::PeerTableModel(interfaces::Node& node, QObject* parent) : QAbstractTableModel(parent), m_node(node), timer(nullptr) { - priv.reset(new PeerTablePriv()); - // set up timer for auto refresh timer = new QTimer(this); connect(timer, &QTimer::timeout, this, &PeerTableModel::refresh); @@ -89,7 +48,7 @@ int PeerTableModel::rowCount(const QModelIndex& parent) const if (parent.isValid()) { return 0; } - return priv->size(); + return m_peers_data.size(); } int PeerTableModel::columnCount(const QModelIndex& parent) const @@ -175,16 +134,26 @@ Qt::ItemFlags PeerTableModel::flags(const QModelIndex &index) const QModelIndex PeerTableModel::index(int row, int column, const QModelIndex& parent) const { Q_UNUSED(parent); - CNodeCombinedStats *data = priv->index(row); - if (data) - return createIndex(row, column, data); + if (0 <= row && row < rowCount() && 0 <= column && column < columnCount()) { + return createIndex(row, column, const_cast(&m_peers_data[row])); + } + return QModelIndex(); } void PeerTableModel::refresh() { + interfaces::Node::NodesStats nodes_stats; + m_node.getNodesStats(nodes_stats); + decltype(m_peers_data) new_peers_data; + new_peers_data.reserve(nodes_stats.size()); + for (const auto& node_stats : nodes_stats) { + const CNodeCombinedStats stats{std::get<0>(node_stats), std::get<2>(node_stats), std::get<1>(node_stats)}; + new_peers_data.append(stats); + } + Q_EMIT layoutAboutToBeChanged(); - priv->refreshPeers(m_node); + m_peers_data.swap(new_peers_data); Q_EMIT layoutChanged(); } diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index dc2802d4a92..0d841ebf28c 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -8,10 +8,11 @@ #include // For CNodeStateStats #include -#include - #include +#include +#include #include +#include class PeerTablePriv; @@ -73,6 +74,8 @@ public Q_SLOTS: void refresh(); private: + //! Internal peer data structure. + QList m_peers_data{}; interfaces::Node& m_node; const QStringList columns{ /*: Title of Peers Table column which contains a @@ -99,7 +102,6 @@ public Q_SLOTS: /*: Title of Peers Table column which contains the peer's User Agent string. */ tr("User Agent")}; - std::unique_ptr priv; QTimer *timer; }; From ecbd91153875c8cdd5b92b840afc116f65e457fb Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 22 Feb 2021 09:57:06 +0200 Subject: [PATCH 3/3] qt: Handle peer addition/removal in a right way This change fixes a bug when a multiple rows selection gets inconsistent after a peer addition/removal. --- src/qt/peertablemodel.cpp | 29 ++++++++++++++++++++++++++--- src/qt/peertablemodel.h | 3 +++ src/qt/rpcconsole.cpp | 2 +- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index fe03365d76a..ba96837c15d 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -153,7 +153,30 @@ void PeerTableModel::refresh() new_peers_data.append(stats); } - Q_EMIT layoutAboutToBeChanged(); - m_peers_data.swap(new_peers_data); - Q_EMIT layoutChanged(); + // Handle peer addition or removal as suggested in Qt Docs. See: + // - https://doc.qt.io/qt-5/model-view-programming.html#inserting-and-removing-rows + // - https://doc.qt.io/qt-5/model-view-programming.html#resizable-models + // We take advantage of the fact that the std::vector returned + // by interfaces::Node::getNodesStats is sorted by nodeid. + for (int i = 0; i < m_peers_data.size();) { + if (i < new_peers_data.size() && m_peers_data.at(i).nodeStats.nodeid == new_peers_data.at(i).nodeStats.nodeid) { + ++i; + continue; + } + // A peer has been removed from the table. + beginRemoveRows(QModelIndex(), i, i); + m_peers_data.erase(m_peers_data.begin() + i); + endRemoveRows(); + } + + if (m_peers_data.size() < new_peers_data.size()) { + // Some peers have been added to the end of the table. + beginInsertRows(QModelIndex(), m_peers_data.size(), new_peers_data.size() - 1); + m_peers_data.swap(new_peers_data); + endInsertRows(); + } else { + m_peers_data.swap(new_peers_data); + } + + Q_EMIT changed(); } diff --git a/src/qt/peertablemodel.h b/src/qt/peertablemodel.h index 0d841ebf28c..0ff1b5dba77 100644 --- a/src/qt/peertablemodel.h +++ b/src/qt/peertablemodel.h @@ -73,6 +73,9 @@ class PeerTableModel : public QAbstractTableModel public Q_SLOTS: void refresh(); +Q_SIGNALS: + void changed(); + private: //! Internal peer data structure. QList m_peers_data{}; diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 08014556337..f9c2bea4373 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -656,7 +656,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); - connect(model->getPeerTableModel(), &PeerTableModel::layoutChanged, this, &RPCConsole::updateDetailWidget); + connect(model->getPeerTableModel(), &PeerTableModel::changed, this, &RPCConsole::updateDetailWidget); // set up ban table ui->banlistWidget->setModel(model->getBanTableModel());