From fb47c15cd81970f19aeb9777c4c576b973889674 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 14 Aug 2018 13:03:33 -0400 Subject: Filter: remove unused parameters from doFilter functions Change the signature from of the virtual doFilter() functions from bool doFilter(struct dive *d, QModelIndex&, QAbstractItemModel*) const; to bool LocationFilterModel::doFilter(struct dive *d) const; as the QModelIndex and QAbstractItemModel parameters were not used. This makes this functions independent from Qt's model/view framework. This is in preparation for making the undo-machinery compatible with the filtering. Signed-off-by: Berthold Stoeger --- qt-models/filtermodels.cpp | 10 +++++----- qt-models/filtermodels.h | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp index 12d755dc1..bd95e0ad9 100644 --- a/qt-models/filtermodels.cpp +++ b/qt-models/filtermodels.cpp @@ -132,7 +132,7 @@ int SuitsFilterModel::countDives(const char *s) const return count_dives_with_suit(s); } -bool SuitsFilterModel::doFilter(dive *d, QModelIndex&, QAbstractItemModel*) const +bool SuitsFilterModel::doFilter(dive *d) const { // rowCount() == 0 should never happen, because we have the "no suits" row // let's handle it gracefully anyway. @@ -196,7 +196,7 @@ void TagFilterModel::repopulate() updateList(list); } -bool TagFilterModel::doFilter(dive *d, QModelIndex&, QAbstractItemModel*) const +bool TagFilterModel::doFilter(dive *d) const { // If there's nothing checked, this should show everything // rowCount() == 0 should never happen, because we have the "no tags" row @@ -234,7 +234,7 @@ int BuddyFilterModel::countDives(const char *s) const return count_dives_with_person(s); } -bool BuddyFilterModel::doFilter(dive *d, QModelIndex&, QAbstractItemModel*) const +bool BuddyFilterModel::doFilter(dive *d) const { // If there's nothing checked, this should show everything // rowCount() == 0 should never happen, because we have the "no tags" row @@ -289,7 +289,7 @@ int LocationFilterModel::countDives(const char *s) const return count_dives_with_location(s); } -bool LocationFilterModel::doFilter(struct dive *d, QModelIndex&, QAbstractItemModel*) const +bool LocationFilterModel::doFilter(struct dive *d) const { // rowCount() == 0 should never happen, because we have the "no location" row // let's handle it gracefully anyway. @@ -412,7 +412,7 @@ bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &s return showTrip; } Q_FOREACH (FilterModelBase *model, models) { - if (!model->doFilter(d, index0, sourceModel())) + if (!model->doFilter(d)) shouldShow = false; } diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h index 1d950e735..2ccfec010 100644 --- a/qt-models/filtermodels.h +++ b/qt-models/filtermodels.h @@ -10,7 +10,7 @@ class FilterModelBase : public QStringListModel { Q_OBJECT public: - virtual bool doFilter(struct dive *d, QModelIndex &index0, QAbstractItemModel *sourceModel) const = 0; + virtual bool doFilter(struct dive *d) const = 0; void clearFilter(); void selectAll(); void invertSelection(); @@ -34,7 +34,7 @@ class TagFilterModel : public FilterModelBase { Q_OBJECT public: static TagFilterModel *instance(); - bool doFilter(struct dive *d, QModelIndex &index0, QAbstractItemModel *sourceModel) const; + bool doFilter(struct dive *d) const; public slots: void repopulate(); @@ -48,7 +48,7 @@ class BuddyFilterModel : public FilterModelBase { Q_OBJECT public: static BuddyFilterModel *instance(); - bool doFilter(struct dive *d, QModelIndex &index0, QAbstractItemModel *sourceModel) const; + bool doFilter(struct dive *d) const; public slots: void repopulate(); @@ -62,7 +62,7 @@ class LocationFilterModel : public FilterModelBase { Q_OBJECT public: static LocationFilterModel *instance(); - bool doFilter(struct dive *d, QModelIndex &index0, QAbstractItemModel *sourceModel) const; + bool doFilter(struct dive *d) const; public slots: void repopulate(); @@ -78,7 +78,7 @@ class SuitsFilterModel : public FilterModelBase { Q_OBJECT public: static SuitsFilterModel *instance(); - bool doFilter(struct dive *d, QModelIndex &index0, QAbstractItemModel *sourceModel) const; + bool doFilter(struct dive *d) const; public slots: void repopulate(); -- cgit v1.2.3-70-g09d2 From 8a394b9db42a21f684c6ae85005a55c29d060c38 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 14 Aug 2018 14:09:30 -0400 Subject: Filter: constify doFilter() argument Conceptually, the doFilter() functions shouldn't modify the dive they test. Therefore, make the argument const. To do this, constify the parameter of get_dive_location(), which likewise seems to be the right thing to do. Signed-off-by: Berthold Stoeger --- core/dive.c | 4 ++-- core/dive.h | 2 +- qt-models/filtermodels.cpp | 8 ++++---- qt-models/filtermodels.h | 12 +++++++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/core/dive.c b/core/dive.c index da4b19346..482ba585b 100644 --- a/core/dive.c +++ b/core/dive.c @@ -4202,9 +4202,9 @@ const char *get_dive_country(struct dive *dive) return NULL; } -char *get_dive_location(struct dive *dive) +const char *get_dive_location(const struct dive *dive) { - struct dive_site *ds = get_dive_site_by_uuid(dive->dive_site_uuid); + const struct dive_site *ds = get_dive_site_by_uuid(dive->dive_site_uuid); if (ds && ds->name) return ds->name; return NULL; diff --git a/core/dive.h b/core/dive.h index 98ac06fdc..01ec1fd3d 100644 --- a/core/dive.h +++ b/core/dive.h @@ -444,7 +444,7 @@ extern struct dive *get_dive(int nr); extern struct dive *get_dive_from_table(int nr, struct dive_table *dt); extern struct dive_site *get_dive_site_for_dive(struct dive *dive); extern const char *get_dive_country(struct dive *dive); -extern char *get_dive_location(struct dive *dive); +extern const char *get_dive_location(const struct dive *dive); extern unsigned int number_of_computers(struct dive *dive); extern struct divecomputer *get_dive_dc(struct dive *dive, int nr); extern timestamp_t dive_endtime(const struct dive *dive); diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp index bd95e0ad9..6e5605654 100644 --- a/qt-models/filtermodels.cpp +++ b/qt-models/filtermodels.cpp @@ -132,7 +132,7 @@ int SuitsFilterModel::countDives(const char *s) const return count_dives_with_suit(s); } -bool SuitsFilterModel::doFilter(dive *d) const +bool SuitsFilterModel::doFilter(const dive *d) const { // rowCount() == 0 should never happen, because we have the "no suits" row // let's handle it gracefully anyway. @@ -196,7 +196,7 @@ void TagFilterModel::repopulate() updateList(list); } -bool TagFilterModel::doFilter(dive *d) const +bool TagFilterModel::doFilter(const dive *d) const { // If there's nothing checked, this should show everything // rowCount() == 0 should never happen, because we have the "no tags" row @@ -234,7 +234,7 @@ int BuddyFilterModel::countDives(const char *s) const return count_dives_with_person(s); } -bool BuddyFilterModel::doFilter(dive *d) const +bool BuddyFilterModel::doFilter(const dive *d) const { // If there's nothing checked, this should show everything // rowCount() == 0 should never happen, because we have the "no tags" row @@ -289,7 +289,7 @@ int LocationFilterModel::countDives(const char *s) const return count_dives_with_location(s); } -bool LocationFilterModel::doFilter(struct dive *d) const +bool LocationFilterModel::doFilter(const dive *d) const { // rowCount() == 0 should never happen, because we have the "no location" row // let's handle it gracefully anyway. diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h index 2ccfec010..0c35f2612 100644 --- a/qt-models/filtermodels.h +++ b/qt-models/filtermodels.h @@ -7,10 +7,12 @@ #include #include +struct dive; + class FilterModelBase : public QStringListModel { Q_OBJECT public: - virtual bool doFilter(struct dive *d) const = 0; + virtual bool doFilter(const dive *d) const = 0; void clearFilter(); void selectAll(); void invertSelection(); @@ -34,7 +36,7 @@ class TagFilterModel : public FilterModelBase { Q_OBJECT public: static TagFilterModel *instance(); - bool doFilter(struct dive *d) const; + bool doFilter(const dive *d) const; public slots: void repopulate(); @@ -48,7 +50,7 @@ class BuddyFilterModel : public FilterModelBase { Q_OBJECT public: static BuddyFilterModel *instance(); - bool doFilter(struct dive *d) const; + bool doFilter(const dive *d) const; public slots: void repopulate(); @@ -62,7 +64,7 @@ class LocationFilterModel : public FilterModelBase { Q_OBJECT public: static LocationFilterModel *instance(); - bool doFilter(struct dive *d) const; + bool doFilter(const dive *d) const; public slots: void repopulate(); @@ -78,7 +80,7 @@ class SuitsFilterModel : public FilterModelBase { Q_OBJECT public: static SuitsFilterModel *instance(); - bool doFilter(struct dive *d) const; + bool doFilter(const dive *d) const; public slots: void repopulate(); -- cgit v1.2.3-70-g09d2 From 13fbca3f5522e31a81aaad0cfc830ee751918c31 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 14 Aug 2018 14:16:25 -0400 Subject: Filter: break out showDive() function from filterAcceptsRow() To make dive-filtering accessible from other parts of the code, break out the actual dive-filtering code into a function that takes a pointer-to-dive instead of QModelIndex. Signed-off-by: Berthold Stoeger --- qt-models/filtermodels.cpp | 79 ++++++++++++++++++++++++++-------------------- qt-models/filtermodels.h | 1 + 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp index 6e5605654..91c524c68 100644 --- a/qt-models/filtermodels.cpp +++ b/qt-models/filtermodels.cpp @@ -367,34 +367,10 @@ MultiFilterSortModel::MultiFilterSortModel(QObject *parent) : QSortFilterProxyMo { } -bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const +bool MultiFilterSortModel::showDive(const struct dive *d) const { - bool shouldShow = true; - QModelIndex index0 = sourceModel()->index(source_row, 0, source_parent); - QVariant diveVariant = sourceModel()->data(index0, DiveTripModel::DIVE_ROLE); - struct dive *d = (struct dive *)diveVariant.value(); - if (curr_dive_site) { - struct dive_site *ds = NULL; - if (!d) { // It's a trip, only show the ones that have dives to be shown. - bool showTrip = false; - for (int i = 0; i < sourceModel()->rowCount(index0); i++) { - QModelIndex child = sourceModel()->index(i, 0, index0); - d = (struct dive *)sourceModel()->data(child, DiveTripModel::DIVE_ROLE).value(); - ds = get_dive_site_by_uuid(d->dive_site_uuid); - if (!ds) - continue; - if (same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid) { - if (ds->uuid != curr_dive_site->uuid) { - qWarning() << "Warning, two different dive sites with same name have a different id" - << ds->uuid << "and" << curr_dive_site->uuid; - } - showTrip = true; // do not shortcircuit the loop or the counts will be wrong - } - } - return showTrip; - } - ds = get_dive_site_by_uuid(d->dive_site_uuid); + dive_site *ds = get_dive_site_by_uuid(d->dive_site_uuid); if (!ds) return false; return same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid; @@ -403,21 +379,56 @@ bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &s if (justCleared || models.isEmpty()) return true; - if (!d) { // It's a trip, only show the ones that have dives to be shown. + for (const FilterModelBase *model: models) { + if (!model->doFilter(d)) + return false; + } + + return true; +} + +bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &source_parent) const +{ + QModelIndex index0 = sourceModel()->index(source_row, 0, source_parent); + QVariant diveVariant = sourceModel()->data(index0, DiveTripModel::DIVE_ROLE); + struct dive *d = (struct dive *)diveVariant.value(); + + if (d) { + // Current row is a dive + bool show = showDive(d); + filter_dive(d, show); + return show; + } + + // From here on, the current row is a trip + if (curr_dive_site) { bool showTrip = false; for (int i = 0; i < sourceModel()->rowCount(index0); i++) { - if (filterAcceptsRow(i, index0)) + QModelIndex child = sourceModel()->index(i, 0, index0); + d = (struct dive *)sourceModel()->data(child, DiveTripModel::DIVE_ROLE).value(); + dive_site *ds = get_dive_site_by_uuid(d->dive_site_uuid); + if (!ds) + continue; + if (same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid) { + if (ds->uuid != curr_dive_site->uuid) { + qWarning() << "Warning, two different dive sites with same name have a different id" + << ds->uuid << "and" << curr_dive_site->uuid; + } showTrip = true; // do not shortcircuit the loop or the counts will be wrong + } } return showTrip; } - Q_FOREACH (FilterModelBase *model, models) { - if (!model->doFilter(d)) - shouldShow = false; - } - filter_dive(d, shouldShow); - return shouldShow; + if (justCleared || models.isEmpty()) + return true; + + bool showTrip = false; + for (int i = 0; i < sourceModel()->rowCount(index0); i++) { + if (filterAcceptsRow(i, index0)) + showTrip = true; // do not shortcircuit the loop or the counts will be wrong + } + return showTrip; } void MultiFilterSortModel::myInvalidate() diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h index 0c35f2612..98212979b 100644 --- a/qt-models/filtermodels.h +++ b/qt-models/filtermodels.h @@ -97,6 +97,7 @@ public: bool filterAcceptsRow(int source_row, const QModelIndex &source_parent) const; void addFilterModel(FilterModelBase *model); void removeFilterModel(FilterModelBase *model); + bool showDive(const struct dive *d) const; int divesDisplayed; public slots: -- cgit v1.2.3-70-g09d2 From 979f81f4090de1a4e828d2f6c1e5bc810d5e8b58 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 14 Aug 2018 18:30:49 -0400 Subject: Filter: separate backend from frontend logic The filter code was an unholy intermixture of backend and frontend logic, which made it hard to access it from outside of the UI. Notably, it expected that Qt would call filterAcceptsRow on all rows. For trip-view, apparently the filter functions were called twice (once for filtering the trip, then for filtering the individual dives). Make the filtering explicit, by calling showDive() for all dives in MultiFilterSortModel::myInvalidate(), setting the hidden_by_filter flags accordingly and ultimately invalidating the filter. The UI code only accesses the hidden_by_filter flag set previously. The "justCleared" flag can then be removed, since accessing the filter does not have side effects. Moreover, there is no noticeable performance gain by returning out early. Signed-off-by: Berthold Stoeger --- qt-models/filtermodels.cpp | 63 ++++++++++++++++------------------------------ qt-models/filtermodels.h | 1 - 2 files changed, 22 insertions(+), 42 deletions(-) diff --git a/qt-models/filtermodels.cpp b/qt-models/filtermodels.cpp index 91c524c68..1943056f7 100644 --- a/qt-models/filtermodels.cpp +++ b/qt-models/filtermodels.cpp @@ -362,7 +362,6 @@ void LocationFilterModel::addName(const QString &newName) MultiFilterSortModel::MultiFilterSortModel(QObject *parent) : QSortFilterProxyModel(parent), divesDisplayed(0), - justCleared(false), curr_dive_site(NULL) { } @@ -376,7 +375,7 @@ bool MultiFilterSortModel::showDive(const struct dive *d) const return same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid; } - if (justCleared || models.isEmpty()) + if (models.isEmpty()) return true; for (const FilterModelBase *model: models) { @@ -393,42 +392,23 @@ bool MultiFilterSortModel::filterAcceptsRow(int source_row, const QModelIndex &s QVariant diveVariant = sourceModel()->data(index0, DiveTripModel::DIVE_ROLE); struct dive *d = (struct dive *)diveVariant.value(); - if (d) { - // Current row is a dive - bool show = showDive(d); - filter_dive(d, show); - return show; - } + // For dives, simply check the hidden_by_filter flag + if (d) + return !d->hidden_by_filter; - // From here on, the current row is a trip - if (curr_dive_site) { - bool showTrip = false; - for (int i = 0; i < sourceModel()->rowCount(index0); i++) { - QModelIndex child = sourceModel()->index(i, 0, index0); - d = (struct dive *)sourceModel()->data(child, DiveTripModel::DIVE_ROLE).value(); - dive_site *ds = get_dive_site_by_uuid(d->dive_site_uuid); - if (!ds) - continue; - if (same_string(ds->name, curr_dive_site->name) || ds->uuid == curr_dive_site->uuid) { - if (ds->uuid != curr_dive_site->uuid) { - qWarning() << "Warning, two different dive sites with same name have a different id" - << ds->uuid << "and" << curr_dive_site->uuid; - } - showTrip = true; // do not shortcircuit the loop or the counts will be wrong - } - } - return showTrip; - } + // Since this is not a dive, it must be a trip + QVariant tripVariant = sourceModel()->data(index0, DiveTripModel::TRIP_ROLE); + dive_trip *trip = (dive_trip *)tripVariant.value(); - if (justCleared || models.isEmpty()) - return true; + if (!trip) + return false; // Oops. Neither dive nor trip, something is seriously wrong. - bool showTrip = false; - for (int i = 0; i < sourceModel()->rowCount(index0); i++) { - if (filterAcceptsRow(i, index0)) - showTrip = true; // do not shortcircuit the loop or the counts will be wrong + // Show the trip if any dive is visible + for (d = trip->dives; d; d = d->next) { + if (!d->hidden_by_filter) + return true; } - return showTrip; + return false; } void MultiFilterSortModel::myInvalidate() @@ -440,6 +420,14 @@ void MultiFilterSortModel::myInvalidate() divesDisplayed = 0; + // Apply filter for each dive + for_each_dive (i, d) { + bool show = showDive(d); + filter_dive(d, show); + if (show) + divesDisplayed++; + } + invalidateFilter(); // first make sure the trips are no longer shown as selected @@ -464,11 +452,6 @@ void MultiFilterSortModel::myInvalidate() dlv->selectDives(curSelectedDives); } - for_each_dive (i, d) { - if (!d->hidden_by_filter) - divesDisplayed++; - } - emit filterFinished(); if (curr_dive_site) { @@ -491,11 +474,9 @@ void MultiFilterSortModel::removeFilterModel(FilterModelBase *model) void MultiFilterSortModel::clearFilter() { - justCleared = true; Q_FOREACH (FilterModelBase *iface, models) { iface->clearFilter(); } - justCleared = false; myInvalidate(); } diff --git a/qt-models/filtermodels.h b/qt-models/filtermodels.h index 98212979b..bcbbf000a 100644 --- a/qt-models/filtermodels.h +++ b/qt-models/filtermodels.h @@ -111,7 +111,6 @@ signals: private: MultiFilterSortModel(QObject *parent = 0); QList models; - bool justCleared; struct dive_site *curr_dive_site; }; -- cgit v1.2.3-70-g09d2