From fb6210a99a1eda0f7f7197ed57f3c61a82132dff Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 5 May 2020 00:12:36 +0200 Subject: [PATCH] cleanup: invert control-flow when resetting the core structures To reset the core data structures, the mobile and desktop UIs were calling into the dive-list models, which then reset the core data structures, themselves and the unrelated locationinformation model. The UI code then reset various other things, such as the TankInformation model or the map. . This was unsatisfying from a control-flow perspective, as the models should display the core data, not act on it. Moreover, this meant lots of intricate intermodule-dependencies. Thus, straighten up the control flow: give the C core the possibility to send a "all data reset" event. And do that in those functions that reset the core data structures. Let each module react to this event by itself. This removes inter-module dependencies. For example, the MainWindow now doesn't have to reset the TankInfoModel or the MapWidget. Then, to reset the core data structures, let the UI code simply directly call the respective core functions. Signed-off-by: Berthold Stoeger --- core/divelist.c | 9 +++++++ core/qthelper.cpp | 6 +++++ core/qthelper.h | 2 ++ core/subsurface-qt/divelistnotifier.h | 3 +++ desktop-widgets/mainwindow.cpp | 5 +--- desktop-widgets/mapwidget.cpp | 1 + mobile-widgets/qmlmanager.cpp | 12 +++------ qt-models/divelocationmodel.cpp | 1 + qt-models/divelocationmodel.h | 1 + qt-models/divetripmodel.cpp | 18 +++----------- qt-models/divetripmodel.h | 8 ++---- qt-models/filtermodels.cpp | 5 ---- qt-models/filtermodels.h | 1 - qt-models/gpslistmodel.cpp | 2 ++ qt-models/mobilelistmodel.cpp | 36 +++++++++++++++++++++------ qt-models/mobilelistmodel.h | 5 ++-- qt-models/tankinfomodel.cpp | 2 ++ qt-models/weightsysteminfomodel.cpp | 2 ++ 18 files changed, 70 insertions(+), 49 deletions(-) diff --git a/core/divelist.c b/core/divelist.c index 4325df595..f224b3414 100644 --- a/core/divelist.c +++ b/core/divelist.c @@ -835,6 +835,9 @@ void process_loaded_dives() autogroup_dives(&dive_table, &trip_table); fulltext_populate(); + + /* Inform frontend of reset data. This should reset all the models. */ + emit_reset_signal(); } /* @@ -1045,6 +1048,9 @@ void add_imported_dives(struct dive_table *import_table, struct trip_table *impo * Choose the newest dive as selected (if any) */ current_dive = dive_table.nr > 0 ? dive_table.dives[dive_table.nr - 1] : NULL; mark_divelist_changed(true); + + /* Inform frontend of reset data. This should reset all the models. */ + emit_reset_signal(); } /* Helper function for process_imported_dives(): @@ -1373,6 +1379,9 @@ void clear_dive_file_data() reset_min_datafile_version(); clear_git_id(); + + /* Inform frontend of reset data. This should reset all the models. */ + emit_reset_signal(); } bool dive_less_than(const struct dive *a, const struct dive *b) diff --git a/core/qthelper.cpp b/core/qthelper.cpp index e543ad1c1..c5ab24427 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -3,6 +3,7 @@ #include "dive.h" #include "core/settings/qPrefLanguage.h" #include "core/settings/qPrefUpdateManager.h" +#include "core/subsurface-qt/divelistnotifier.h" #include "subsurface-string.h" #include "subsurface-string.h" #include "gettextfromc.h" @@ -1675,3 +1676,8 @@ std::vector get_cylinder_map_for_add(int count, int n) std::iota(mapping.begin() + n, mapping.end(), n + 1); return mapping; } + +extern "C" void emit_reset_signal() +{ + emit diveListNotifier.dataReset(); +} diff --git a/core/qthelper.h b/core/qthelper.h index 8bb0bbdfa..8cd31b29c 100644 --- a/core/qthelper.h +++ b/core/qthelper.h @@ -155,6 +155,8 @@ pressure_t string_to_pressure(const char *str); volume_t string_to_volume(const char *str, pressure_t workp); fraction_t string_to_fraction(const char *str); char *get_changes_made(); +void emit_reset_signal(); + #ifdef __cplusplus } #endif diff --git a/core/subsurface-qt/divelistnotifier.h b/core/subsurface-qt/divelistnotifier.h index 2931e443e..aa82a4bef 100644 --- a/core/subsurface-qt/divelistnotifier.h +++ b/core/subsurface-qt/divelistnotifier.h @@ -78,6 +78,9 @@ struct TripField { class DiveListNotifier : public QObject { Q_OBJECT signals: + // The core structures were completely reset. Repopulate all models. + void dataReset(); + // Note that there are no signals for trips being added and created // because these events never happen without a dive being added, removed or moved. // The dives are always sorted according to the dives_less_than() function of the core. diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index dfe00bfe1..f4eadab16 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/desktop-widgets/mainwindow.cpp @@ -408,15 +408,12 @@ MainWindow *MainWindow::instance() void MainWindow::refreshDisplay(bool doRecreateDiveList) { mainTab->reload(); - TankInfoModel::instance()->update(); if (doRecreateDiveList) diveList->reload(); - MapWidget::instance()->reload(); setApplicationState(ApplicationState::Default); diveList->setEnabled(true); diveList->setFocus(); - WSInfoModel::instance()->update(); ui.actionAutoGroup->setChecked(autogroup); } @@ -647,7 +644,7 @@ void MainWindow::closeCurrentFile() { /* free the dives and trips */ clear_git_id(); - MultiFilterSortModel::instance()->clear(); // this clears all the core data structures + clear_dive_file_data(); // this clears all the core data structures and resets the models setCurrentFile(nullptr); diveList->setSortOrder(DiveTripModelBase::NR, Qt::DescendingOrder); MapWidget::instance()->reload(); diff --git a/desktop-widgets/mapwidget.cpp b/desktop-widgets/mapwidget.cpp index 0e04ae282..2d23f0916 100644 --- a/desktop-widgets/mapwidget.cpp +++ b/desktop-widgets/mapwidget.cpp @@ -28,6 +28,7 @@ MapWidget::MapWidget(QWidget *parent) : QQuickWidget(parent) setResizeMode(QQuickWidget::SizeRootObjectToView); connect(this, &QQuickWidget::statusChanged, this, &MapWidget::doneLoading); connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &MapWidget::divesChanged); + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &MapWidget::reload); setSource(urlMapWidget); } diff --git a/mobile-widgets/qmlmanager.cpp b/mobile-widgets/qmlmanager.cpp index 8e614ec2b..36394364b 100644 --- a/mobile-widgets/qmlmanager.cpp +++ b/mobile-widgets/qmlmanager.cpp @@ -339,7 +339,7 @@ void QMLManager::applicationStateChanged(Qt::ApplicationState state) void QMLManager::openLocalThenRemote(QString url) { // clear out the models and the fulltext index - MobileModels::instance()->clear(); + clear_dive_file_data(); setDiveListProcessing(true); setNotificationText(tr("Open local dive data file")); appendTextToLog(QString("Open dive data file %1 - git_local only is %2").arg(url).arg(git_local_only)); @@ -383,7 +383,6 @@ void QMLManager::openLocalThenRemote(QString url) // the following steps can take a long time, so provide updates setNotificationText(tr("Processing %1 dives").arg(dive_table.nr)); process_loaded_dives(); - MobileModels::instance()->reset(); setNotificationText(tr("%1 dives loaded from local dive data file").arg(dive_table.nr)); } if (qPrefCloudStorage::cloud_verification_status() == qPrefCloudStorage::CS_NEED_TO_VERIFY) { @@ -612,8 +611,7 @@ void QMLManager::saveCloudCredentials(const QString &newEmail, const QString &ne syncLoadFromCloud(); manager()->clearAccessCache(); // remove any chached credentials clear_git_id(); // invalidate our remembered GIT SHA - MobileModels::instance()->clear(); - GpsListModel::instance()->clear(); + clear_dive_file_data(); setStartPageText(tr("Attempting to open cloud storage with new credentials")); // since we changed credentials, we need to try to connect to the cloud, regardless // of whether we're in offline mode or not, to make sure the repository is synced @@ -687,7 +685,7 @@ void QMLManager::loadDivesWithValidCredentials() // if we aren't switching from no-cloud mode, let's clear the dive data if (!noCloudToCloud) { appendTextToLog("Clear out in memory dive data"); - MobileModels::instance()->clear(); + clear_dive_file_data(); } else { appendTextToLog("Switching from no cloud mode; keep in memory dive data"); } @@ -717,7 +715,6 @@ void QMLManager::loadDivesWithValidCredentials() if (noCloudToCloud) { git_storage_update_progress(qPrintable(tr("Loading dives from local storage ('no cloud' mode)"))); mergeLocalRepo(); - MobileModels::instance()->reset(); appendTextToLog(QStringLiteral("%1 dives loaded after importing nocloud local storage").arg(dive_table.nr)); noCloudToCloud = false; mark_divelist_changed(true); @@ -779,7 +776,6 @@ void QMLManager::consumeFinishedLoad() prefs.show_ccr_sensors = git_prefs.show_ccr_sensors; prefs.pp_graphs.po2 = git_prefs.pp_graphs.po2; process_loaded_dives(); - MobileModels::instance()->reset(); appendTextToLog(QStringLiteral("%1 dives loaded").arg(dive_table.nr)); if (dive_table.nr == 0) setStartPageText(tr("Cloud storage open successfully. No dives in dive list.")); @@ -787,7 +783,7 @@ void QMLManager::consumeFinishedLoad() void QMLManager::refreshDiveList() { - MobileModels::instance()->reset(); + MobileModels::instance()->invalidate(); } // Ouch. Editing a dive might create a dive site or change an existing dive site. diff --git a/qt-models/divelocationmodel.cpp b/qt-models/divelocationmodel.cpp index 26cf3353e..f8bad01a7 100644 --- a/qt-models/divelocationmodel.cpp +++ b/qt-models/divelocationmodel.cpp @@ -26,6 +26,7 @@ LocationInformationModel::LocationInformationModel(QObject *obj) : QAbstractTabl connect(&diveListNotifier, &DiveListNotifier::diveSiteDeleted, this, &LocationInformationModel::diveSiteDeleted); connect(&diveListNotifier, &DiveListNotifier::diveSiteChanged, this, &LocationInformationModel::diveSiteChanged); connect(&diveListNotifier, &DiveListNotifier::diveSiteDivesChanged, this, &LocationInformationModel::diveSiteDivesChanged); + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &LocationInformationModel::update); } int LocationInformationModel::columnCount(const QModelIndex &) const diff --git a/qt-models/divelocationmodel.h b/qt-models/divelocationmodel.h index 661debc53..623296de2 100644 --- a/qt-models/divelocationmodel.h +++ b/qt-models/divelocationmodel.h @@ -31,6 +31,7 @@ public: public slots: void update(); +private slots: void diveSiteDiveCountChanged(struct dive_site *ds); void diveSiteAdded(struct dive_site *ds, int idx); void diveSiteDeleted(struct dive_site *ds, int idx); diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index ed5542fcc..db086ca59 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -476,27 +476,13 @@ void DiveTripModelBase::initSelection() select_newest_visible_dive(); } -void DiveTripModelBase::clear() -{ - Command::clear(); // If we clear the dive list, all undo-information becomes stalte. - beginResetModel(); - clear_dive_file_data(); - clearData(); - LocationInformationModel::instance()->update(); - oldCurrent = nullptr; - emit diveListNotifier.divesSelected({}); // Inform profile, etc of changed selection - endResetModel(); - emit diveListNotifier.numShownChanged(); -} - // Currently only used by the mobile models void DiveTripModelBase::reset() { beginResetModel(); + oldCurrent = nullptr; clearData(); populate(); - uiNotification(tr("setting up dive sites")); - LocationInformationModel::instance()->update(); uiNotification(tr("finish populating data store")); endResetModel(); uiNotification(tr("setting up internal data structures")); @@ -721,6 +707,7 @@ DiveTripModelTree::DiveTripModelTree(QObject *parent) : DiveTripModelBase(parent connect(&diveListNotifier, &DiveListNotifier::pictureOffsetChanged, this, &DiveTripModelTree::diveChanged); connect(&diveListNotifier, &DiveListNotifier::picturesRemoved, this, &DiveTripModelTree::diveChanged); connect(&diveListNotifier, &DiveListNotifier::picturesAdded, this, &DiveTripModelTree::diveChanged); + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &DiveTripModelTree::reset); populate(); } @@ -1485,6 +1472,7 @@ DiveTripModelList::DiveTripModelList(QObject *parent) : DiveTripModelBase(parent connect(&diveListNotifier, &DiveListNotifier::pictureOffsetChanged, this, &DiveTripModelList::diveChanged); connect(&diveListNotifier, &DiveListNotifier::picturesRemoved, this, &DiveTripModelList::diveChanged); connect(&diveListNotifier, &DiveListNotifier::picturesAdded, this, &DiveTripModelList::diveChanged); + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &DiveTripModelList::reset); populate(); } diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h index 3e7955b5b..4b2376bd9 100644 --- a/qt-models/divetripmodel.h +++ b/qt-models/divetripmodel.h @@ -64,12 +64,6 @@ public: // Call after having set the model to be informed of the current selection. void initSelection(); - // Clear all dives - void clear(); - - // Reload data - void reset(); - Qt::ItemFlags flags(const QModelIndex &index) const; QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; DiveTripModelBase(QObject *parent = 0); @@ -78,6 +72,8 @@ public: // Used for sorting. This is a bit of a layering violation, as sorting should be performed // by the higher-up QSortFilterProxyModel, but it makes things so much easier! virtual bool lessThan(const QModelIndex &i1, const QModelIndex &i2) const = 0; +protected slots: + void reset(); signals: // The propagation of selection changes is complex. // The control flow of dive-selection goes: diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp index 9b2ab6a7b..a5fae4520 100644 --- a/qt-models/filtermodels.cpp +++ b/qt-models/filtermodels.cpp @@ -33,11 +33,6 @@ void MultiFilterSortModel::resetModel(DiveTripModelBase::Layout layout) LocationInformationModel::instance()->update(); } -void MultiFilterSortModel::clear() -{ - model->clear(); -} - // Translate selection into local indices and re-emit signal void MultiFilterSortModel::selectionChangedSlot(const QVector &indices) { diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h index 588bfe2b5..8895f5b2c 100644 --- a/qt-models/filtermodels.h +++ b/qt-models/filtermodels.h @@ -17,7 +17,6 @@ public: bool lessThan(const QModelIndex &, const QModelIndex &) const override; void resetModel(DiveTripModelBase::Layout layout); - void clear(); signals: void selectionChanged(const QVector &indices); void currentDiveChanged(QModelIndex index); diff --git a/qt-models/gpslistmodel.cpp b/qt-models/gpslistmodel.cpp index 33eb8a5ea..c463c3ed2 100644 --- a/qt-models/gpslistmodel.cpp +++ b/qt-models/gpslistmodel.cpp @@ -1,10 +1,12 @@ // SPDX-License-Identifier: GPL-2.0 #include "qt-models/gpslistmodel.h" +#include "core/subsurface-qt/divelistnotifier.h" #include "core/qthelper.h" #include GpsListModel::GpsListModel() { + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &GpsListModel::update); } void GpsListModel::update() diff --git a/qt-models/mobilelistmodel.cpp b/qt-models/mobilelistmodel.cpp index 6dd74a57c..17ec252d7 100644 --- a/qt-models/mobilelistmodel.cpp +++ b/qt-models/mobilelistmodel.cpp @@ -507,6 +507,18 @@ void MobileListModel::changed(const QModelIndex &topLeft, const QModelIndex &bot } } +void MobileListModel::invalidate() +{ + // Qt's model/view API can't handle empty ranges and we have to subtract one from the last item, + // because ranges are given as [first,last] (i.e. last inclusive). + int rows = rowCount(QModelIndex()); + if (rows <= 0) + return; + QModelIndex fromIdx = createIndex(0, 0); + QModelIndex toIdx = createIndex(rows - 1, 0); + dataChanged(fromIdx, toIdx); +} + void MobileListModel::unexpand() { if (expandedRow < 0) @@ -903,6 +915,18 @@ void MobileSwipeModel::changed(const QModelIndex &topLeft, const QModelIndex &bo emit currentDiveChanged(fromIdx); } +void MobileSwipeModel::invalidate() +{ + // Qt's model/view API can't handle empty ranges and we have to subtract one from the last item, + // because ranges are given as [first,last] (i.e. last inclusive). + int rows = rowCount(QModelIndex()); + if (rows <= 0) + return; + QModelIndex fromIdx = createIndex(0, 0); + QModelIndex toIdx = createIndex(rows - 1, 0); + dataChanged(fromIdx, toIdx); +} + QVariant MobileSwipeModel::data(const QModelIndex &index, int role) const { return source->data(mapToSource(index), role); @@ -925,7 +949,6 @@ MobileModels::MobileModels() : lm(&source), sm(&source) { - reset(); } MobileListModel *MobileModels::listModel() @@ -938,12 +961,9 @@ MobileSwipeModel *MobileModels::swipeModel() return &sm; } -void MobileModels::clear() +// This is called when the settings changed. Instead of rebuilding the model, send a changed signal on all entries. +void MobileModels::invalidate() { - source.clear(); -} - -void MobileModels::reset() -{ - source.reset(); + sm.invalidate(); + sm.invalidate(); } diff --git a/qt-models/mobilelistmodel.h b/qt-models/mobilelistmodel.h index c898ed118..e315e9ebd 100644 --- a/qt-models/mobilelistmodel.h +++ b/qt-models/mobilelistmodel.h @@ -77,6 +77,7 @@ public: MobileListModel(DiveTripModelBase *source); void expand(int row); void unexpand(); + void invalidate(); Q_INVOKABLE void toggle(int row); Q_PROPERTY(int shown READ shown NOTIFY shownChanged); signals: @@ -121,6 +122,7 @@ public: MobileSwipeModel(DiveTripModelBase *source); static MobileSwipeModel *instance(); void resetModel(DiveTripModelBase::Layout layout); // Switch between tree and list view + void invalidate(); private: struct IndexRange { int first, last; @@ -175,8 +177,7 @@ public: static MobileModels *instance(); MobileListModel *listModel(); MobileSwipeModel *swipeModel(); - void clear(); // Clear all dive data - void reset(); // Reset model after having reloaded the core data + void invalidate(); // Invalidate all entries to force a re-render. private: MobileModels(); DiveTripModelTree source; diff --git a/qt-models/tankinfomodel.cpp b/qt-models/tankinfomodel.cpp index df1841fa3..8d78cf3d5 100644 --- a/qt-models/tankinfomodel.cpp +++ b/qt-models/tankinfomodel.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include "qt-models/tankinfomodel.h" #include "core/dive.h" +#include "core/subsurface-qt/divelistnotifier.h" #include "core/gettextfromc.h" #include "core/metrics.h" @@ -85,6 +86,7 @@ int TankInfoModel::rowCount(const QModelIndex&) const TankInfoModel::TankInfoModel() { setHeaderDataStrings(QStringList() << tr("Description") << tr("ml") << tr("bar")); + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &TankInfoModel::update); update(); } diff --git a/qt-models/weightsysteminfomodel.cpp b/qt-models/weightsysteminfomodel.cpp index 21d6f538f..9c28d5f64 100644 --- a/qt-models/weightsysteminfomodel.cpp +++ b/qt-models/weightsysteminfomodel.cpp @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #include "qt-models/weightsysteminfomodel.h" +#include "core/subsurface-qt/divelistnotifier.h" #include "core/dive.h" #include "core/metrics.h" #include "core/gettextfromc.h" @@ -74,6 +75,7 @@ int WSInfoModel::rowCount(const QModelIndex&) const WSInfoModel::WSInfoModel() { setHeaderDataStrings(QStringList() << tr("Description") << tr("kg")); + connect(&diveListNotifier, &DiveListNotifier::dataReset, this, &WSInfoModel::update); update(); }