From 8f12792186d46286b9e6714acddb2fee8c25aea3 Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Wed, 29 Jul 2020 11:56:23 -0500 Subject: [PATCH] fix: ensure details dialog is updated on selection (#1376) * fix: ensure details dialog is updated on selection --- qt/DetailsDialog.cc | 103 +++++++++++++++++++++----------------------- qt/DetailsDialog.h | 39 ++++++++++++++--- qt/RpcQueue.cc | 10 ++++- qt/RpcQueue.h | 5 +++ qt/Session.cc | 49 +++++++++++++-------- qt/Session.h | 17 +++++--- 6 files changed, 137 insertions(+), 86 deletions(-) diff --git a/qt/DetailsDialog.cc b/qt/DetailsDialog.cc index 29a4516d2..ea642efa2 100644 --- a/qt/DetailsDialog.cc +++ b/qt/DetailsDialog.cc @@ -54,6 +54,7 @@ class Session; namespace { +int constexpr DebounceIntervalMSec = 100; int constexpr RefreshIntervalMSec = 4000; char const constexpr* const PrefKey = "pref_key"; @@ -232,11 +233,16 @@ DetailsDialog::DetailsDialog(Session& session, Prefs& prefs, TorrentModel const& connect(&model_, &TorrentModel::torrentsChanged, this, &DetailsDialog::onTorrentsChanged); connect(&model_, &TorrentModel::torrentsEdited, this, &DetailsDialog::onTorrentsEdited); connect(&prefs_, &Prefs::changed, this, &DetailsDialog::refreshPref); - connect(&timer_, &QTimer::timeout, this, &DetailsDialog::onTimer); - onTimer(); - timer_.setSingleShot(false); - timer_.start(RefreshIntervalMSec); + // call refreshModel periodically + connect(&model_timer_, &QTimer::timeout, this, &DetailsDialog::refreshModel); + model_timer_.setSingleShot(false); + model_timer_.start(RefreshIntervalMSec); + refreshModel(); + + // set up the debounce timer + connect(&ui_debounce_timer_, &QTimer::timeout, this, &DetailsDialog::refreshUI); + ui_debounce_timer_.setSingleShot(true); } DetailsDialog::~DetailsDialog() @@ -255,9 +261,10 @@ void DetailsDialog::setIds(torrent_ids_t const& ids) ids_ = ids; session_.refreshDetailInfo(ids_); - changed_torrents_ = true; tracker_model_->refresh(model_, ids_); - onTimer(); + + refreshModel(); + refreshUI(); } } @@ -293,12 +300,7 @@ void DetailsDialog::refreshPref(int key) **** ***/ -void DetailsDialog::onTimer() -{ - getNewData(); -} - -void DetailsDialog::getNewData() +void DetailsDialog::refreshModel() { if (!ids_.empty()) { @@ -330,7 +332,7 @@ void DetailsDialog::onTorrentsChanged(torrent_ids_t const& ids, Torrent::fields_ { Q_UNUSED(fields) - if (have_pending_refresh_) + if (ui_debounce_timer_.isActive()) { return; } @@ -340,8 +342,19 @@ void DetailsDialog::onTorrentsChanged(torrent_ids_t const& ids, Torrent::fields_ return; } - have_pending_refresh_ = true; - QTimer::singleShot(100, this, SLOT(refresh())); + ui_debounce_timer_.start(DebounceIntervalMSec); +} + +void DetailsDialog::onSessionCalled(Session::Tag tag) +{ + if ((pending_changes_tags_.erase(tag) > 0) && canEdit()) + { + // no pending changes left, so stop listening + disconnect(pending_changes_connection_); + pending_changes_connection_ = {}; + + refreshModel(); + } } namespace @@ -379,7 +392,7 @@ void setIfIdle(QSpinBox* spin, int value) } // namespace -void DetailsDialog::refresh() +void DetailsDialog::refreshUI() { int const n = ids_.size(); bool const single = n == 1; @@ -907,7 +920,7 @@ void DetailsDialog::refresh() /// Options Tab /// - if (changed_torrents_ && !torrents.empty()) + if (canEdit() && !torrents.empty()) { int i; bool uniform; @@ -1166,27 +1179,21 @@ void DetailsDialog::refresh() peers_ = peers2; - if (!single) + if (single) + { + ui_.filesView->update(torrents[0]->files(), canEdit()); + } + else { ui_.filesView->clear(); } - if (single) - { - ui_.filesView->update(torrents[0]->files(), changed_torrents_); - } - - changed_torrents_ = false; - have_pending_refresh_ = false; setEnabled(true); } void DetailsDialog::setEnabled(bool enabled) { - for (int i = 0; i < ui_.tabs->count(); ++i) - { - ui_.tabs->widget(i)->setEnabled(enabled); - } + ui_.tabs->setEnabled(enabled); } /*** @@ -1220,14 +1227,12 @@ void DetailsDialog::onShowBackupTrackersToggled(bool val) void DetailsDialog::onHonorsSessionLimitsToggled(bool val) { - session_.torrentSet(ids_, TR_KEY_honorsSessionLimits, val); - getNewData(); + torrentSet(TR_KEY_honorsSessionLimits, val); } void DetailsDialog::onDownloadLimitedToggled(bool val) { - session_.torrentSet(ids_, TR_KEY_downloadLimited, val); - getNewData(); + torrentSet(TR_KEY_downloadLimited, val); } void DetailsDialog::onSpinBoxEditingFinished() @@ -1238,27 +1243,23 @@ void DetailsDialog::onSpinBoxEditingFinished() if (d != nullptr) { - session_.torrentSet(ids_, key, d->value()); + torrentSet(key, d->value()); } else { - session_.torrentSet(ids_, key, qobject_cast(spin)->value()); + torrentSet(key, qobject_cast(spin)->value()); } - - getNewData(); } void DetailsDialog::onUploadLimitedToggled(bool val) { - session_.torrentSet(ids_, TR_KEY_uploadLimited, val); - getNewData(); + torrentSet(TR_KEY_uploadLimited, val); } void DetailsDialog::onIdleModeChanged(int index) { int const val = ui_.idleCombo->itemData(index).toInt(); - session_.torrentSet(ids_, TR_KEY_seedIdleMode, val); - getNewData(); + torrentSet(TR_KEY_seedIdleMode, val); } void DetailsDialog::onIdleLimitChanged() @@ -1275,7 +1276,7 @@ void DetailsDialog::onIdleLimitChanged() void DetailsDialog::onRatioModeChanged(int index) { int const val = ui_.ratioCombo->itemData(index).toInt(); - session_.torrentSet(ids_, TR_KEY_seedRatioMode, val); + torrentSet(TR_KEY_seedRatioMode, val); } void DetailsDialog::onBandwidthPriorityChanged(int index) @@ -1283,8 +1284,7 @@ void DetailsDialog::onBandwidthPriorityChanged(int index) if (index != -1) { int const priority = ui_.bandwidthPriorityCombo->itemData(index).toInt(); - session_.torrentSet(ids_, TR_KEY_bandwidthPriority, priority); - getNewData(); + torrentSet(TR_KEY_bandwidthPriority, priority); } } @@ -1328,8 +1328,7 @@ void DetailsDialog::onAddTrackerClicked() else { auto const urls = QStringList{ url }; - session_.torrentSet(ids, TR_KEY_trackerAdd, urls); - getNewData(); + torrentSet(ids, TR_KEY_trackerAdd, urls); } } } @@ -1360,8 +1359,7 @@ void DetailsDialog::onEditTrackerClicked() QPair const id_url = qMakePair(tracker_info.st.id, newval); - session_.torrentSet(ids, TR_KEY_trackerReplace, id_url); - getNewData(); + torrentSet(ids, TR_KEY_trackerReplace, id_url); } } @@ -1382,11 +1380,10 @@ void DetailsDialog::onRemoveTrackerClicked() for (int const id : torrent_id_to_tracker_ids.uniqueKeys()) { torrent_ids_t const ids{ id }; - session_.torrentSet(ids, TR_KEY_trackerRemove, torrent_id_to_tracker_ids.values(id)); + torrentSet(ids, TR_KEY_trackerRemove, torrent_id_to_tracker_ids.values(id)); } selection_model->clearSelection(); - getNewData(); } void DetailsDialog::initOptionsTab() @@ -1518,15 +1515,13 @@ void DetailsDialog::onFilePriorityChanged(QSet const& indices, int priority break; } - session_.torrentSet(ids_, key, indices.values()); - getNewData(); + torrentSet(key, indices.values()); } void DetailsDialog::onFileWantedChanged(QSet const& indices, bool wanted) { tr_quark const key = wanted ? TR_KEY_files_wanted : TR_KEY_files_unwanted; - session_.torrentSet(ids_, key, indices.values()); - getNewData(); + torrentSet(key, indices.values()); } void DetailsDialog::onPathEdited(QString const& oldpath, QString const& newname) diff --git a/qt/DetailsDialog.h b/qt/DetailsDialog.h index ed24e281e..bf1ec2d0b 100644 --- a/qt/DetailsDialog.h +++ b/qt/DetailsDialog.h @@ -14,6 +14,7 @@ #include #include "BaseDialog.h" +#include "Session.h" #include "Typedefs.h" #include "ui_DetailsDialog.h" @@ -51,18 +52,17 @@ private: void initFilesTab(); void initOptionsTab(); - void getNewData(); - QIcon getStockIcon(QString const& freedesktop_name, int fallback); void setEnabled(bool); private slots: - void refresh(); + void refreshModel(); void refreshPref(int key); - void onTimer(); + void refreshUI(); void onTorrentsEdited(torrent_ids_t const& ids); void onTorrentsChanged(torrent_ids_t const& ids, Torrent::fields_t const& fields); + void onSessionCalled(Session::Tag tag); // Tracker tab void onTrackerSelectionChanged(); @@ -89,6 +89,32 @@ private slots: void onIdleLimitChanged(); private: + /* When a torrent property is edited in the details dialog (e.g. + file priority, speed limits, etc.), don't update those UI fields + until we know the server has processed the request. This keeps + the UI from appearing to undo the change if we receive a refresh + that was already in-flight _before_ the property was edited. */ + bool canEdit() const { return std::empty(pending_changes_tags_); } + std::unordered_set pending_changes_tags_; + QMetaObject::Connection pending_changes_connection_; + + template + void torrentSet(torrent_ids_t const& ids, tr_quark key, T val) + { + auto const tag = session_.torrentSet(ids, key, val); + pending_changes_tags_.insert(tag); + if (!pending_changes_connection_) + { + pending_changes_connection_ = connect(&session_, &Session::sessionCalled, this, &DetailsDialog::onSessionCalled); + } + } + + template + void torrentSet(tr_quark key, T val) + { + torrentSet(ids_, key, val); + } + Session& session_; Prefs& prefs_; TorrentModel const& model_; @@ -96,9 +122,8 @@ private: Ui::DetailsDialog ui_ = {}; torrent_ids_t ids_; - QTimer timer_; - bool changed_torrents_ = {}; - bool have_pending_refresh_ = {}; + QTimer model_timer_; + QTimer ui_debounce_timer_; TrackerModel* tracker_model_ = {}; TrackerModelFilter* tracker_filter_ = {}; diff --git a/qt/RpcQueue.cc b/qt/RpcQueue.cc index ef37aad0f..b1ad906c3 100644 --- a/qt/RpcQueue.cc +++ b/qt/RpcQueue.cc @@ -10,8 +10,16 @@ #include "RpcQueue.h" +namespace +{ + +auto next_tag = RpcQueue::Tag {}; + +} + RpcQueue::RpcQueue(QObject* parent) : - QObject(parent) + QObject(parent), + tag_(next_tag++) { connect(&future_watcher_, SIGNAL(finished()), SLOT(stepFinished())); } diff --git a/qt/RpcQueue.h b/qt/RpcQueue.h index 671143ed3..b46b9ab9f 100644 --- a/qt/RpcQueue.h +++ b/qt/RpcQueue.h @@ -8,6 +8,7 @@ #pragma once +#include #include #include @@ -47,6 +48,9 @@ public: // (hence it may be e. g. a lambda capturing local variables by reference). void run(); + using Tag = uint64_t; + Tag tag() const { return tag_; } + private: // Internally queued function. Takes the last response future, makes a // request and returns a new response future. @@ -138,6 +142,7 @@ private: } private: + Tag const tag_; bool tolerate_errors_ = {}; QFutureInterface promise_; QQueue> queue_; diff --git a/qt/Session.cc b/qt/Session.cc index 175bca458..126abf2e9 100644 --- a/qt/Session.cc +++ b/qt/Session.cc @@ -423,57 +423,71 @@ void addOptionalIds(tr_variant* args, torrent_ids_t const& ids) } // namespace -void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, double value) +Session::Tag Session::torrentSetImpl(tr_variant* args) +{ + auto* const q = new RpcQueue(); + auto const tag = q->tag(); + + q->add([this, args]() + { + return rpc_.exec(TR_KEY_torrent_set, args); + }); + q->add([this, tag]() + { + emit sessionCalled(tag); + }); + q->setTolerateErrors(); + q->run(); + + return tag; +} + +Session::Tag Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, double value) { tr_variant args; tr_variantInitDict(&args, 2); addOptionalIds(&args, ids); dictAdd(&args, key, value); - - exec(TR_KEY_torrent_set, &args); + return torrentSetImpl(&args); } -void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, int value) +Session::Tag Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, int value) { tr_variant args; tr_variantInitDict(&args, 2); addOptionalIds(&args, ids); dictAdd(&args, key, value); - - exec(TR_KEY_torrent_set, &args); + return torrentSetImpl(&args); } -void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, bool value) +Session::Tag Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, bool value) { tr_variant args; tr_variantInitDict(&args, 2); addOptionalIds(&args, ids); dictAdd(&args, key, value); - - exec(TR_KEY_torrent_set, &args); + return torrentSetImpl(&args); } -void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QStringList const& value) +Session::Tag Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QStringList const& value) { tr_variant args; tr_variantInitDict(&args, 2); addOptionalIds(&args, ids); dictAdd(&args, key, value); - - exec(TR_KEY_torrent_set, &args); + return torrentSetImpl(&args); } -void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QList const& value) +Session::Tag Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QList const& value) { tr_variant args; tr_variantInitDict(&args, 2); addOptionalIds(&args, ids); dictAdd(&args, key, value); - - exec(TR_KEY_torrent_set, &args); + return torrentSetImpl(&args); } -void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QPair const& value) +Session::Tag Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QPair const& value) { tr_variant args; tr_variantInitDict(&args, 2); @@ -481,8 +495,7 @@ void Session::torrentSet(torrent_ids_t const& ids, tr_quark const key, QPair #include "RpcClient.h" +#include "RpcQueue.h" #include "Torrent.h" #include "Typedefs.h" @@ -79,12 +80,14 @@ public: RpcResponseFuture exec(tr_quark method, tr_variant* args); RpcResponseFuture exec(std::string_view method, tr_variant* args); - void torrentSet(torrent_ids_t const& ids, tr_quark const key, bool val); - void torrentSet(torrent_ids_t const& ids, tr_quark const key, int val); - void torrentSet(torrent_ids_t const& ids, tr_quark const key, double val); - void torrentSet(torrent_ids_t const& ids, tr_quark const key, QList const& val); - void torrentSet(torrent_ids_t const& ids, tr_quark const key, QStringList const& val); - void torrentSet(torrent_ids_t const& ids, tr_quark const key, QPair const& val); + using Tag = RpcQueue::Tag; + Tag torrentSet(torrent_ids_t const& ids, tr_quark const key, bool val); + Tag torrentSet(torrent_ids_t const& ids, tr_quark const key, int val); + Tag torrentSet(torrent_ids_t const& ids, tr_quark const key, double val); + Tag torrentSet(torrent_ids_t const& ids, tr_quark const key, QList const& val); + Tag torrentSet(torrent_ids_t const& ids, tr_quark const key, QStringList const& val); + Tag torrentSet(torrent_ids_t const& ids, tr_quark const key, QPair const& val); + void torrentSetLocation(torrent_ids_t const& ids, QString const& path, bool do_move); void torrentRenamePath(torrent_ids_t const& ids, QString const& oldpath, QString const& newname); void addTorrent(AddData const& addme, tr_variant* top, bool trash_original); @@ -120,6 +123,7 @@ signals: void blocklistUpdated(int); void torrentsUpdated(tr_variant* torrent_list, bool complete_list); void torrentsRemoved(tr_variant* torrent_list); + void sessionCalled(Tag); void dataReadProgress(); void dataSendProgress(); void networkResponse(QNetworkReply::NetworkError code, QString const& message); @@ -131,6 +135,7 @@ private: void updateStats(tr_variant* args); void updateInfo(tr_variant* args); + Tag torrentSetImpl(tr_variant* args); void sessionSet(tr_quark const key, QVariant const& variant); void pumpRequests(); void sendTorrentRequest(std::string_view request, torrent_ids_t const& torrent_ids);