diff options
author | Berthold Stoeger <bstoeger@mail.tuwien.ac.at> | 2018-08-03 11:35:43 +0200 |
---|---|---|
committer | Dirk Hohndel <dirk@hohndel.org> | 2018-10-11 16:22:27 -0700 |
commit | 7067e335964de97d51381ca9e9b09f7236ffc619 (patch) | |
tree | 88587c3ed4664b81c605866aa126bd41af3073db | |
parent | 23db0ba68df5f785a8a2db5ff5b76bbbee31d69f (diff) | |
download | subsurface-7067e335964de97d51381ca9e9b09f7236ffc619.tar.gz |
Undo: select dives after add, remove, merge, split dive commands
Select the proper dives after the add, remove, split and merge
dives commands on undo *and* redo. Generally, select the added
dives. For undo of add, remember the pre-addition selection.
For redo of remove, select the closest dive to the first removed
dive.
The biggest part of the commit is the signal-interface between
the dive commands and the dive-list model and dive-list view.
This is done in two steps:
1) To the DiveTripModel in batches of trips. The dive trip model
transforms the dives into indices.
2) To the DiveListView. The DiveListView has to translate the
DiveTripModel indexes to actual indexes via its QSortFilterProxy-
model.
For code-reuse, derive all divelist-changing commands from a new base-class,
which has a flag that describes whether the divelist changed. The helper
functions which add and remove dives are made members of the base class and
set the flag is a selected dive is added or removed.
To properly detect when the current dive was deleted it
became necessary to turn the current dive from an index
to a pointer, because indices are not stable.
Unfortunately, in some cases an index was expected and these
places now have to transform the dive into an index. These
should be converted in due course.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
-rw-r--r-- | core/dive.h | 3 | ||||
-rw-r--r-- | core/divelist.c | 82 | ||||
-rw-r--r-- | core/divelist.h | 5 | ||||
-rw-r--r-- | core/profile.c | 2 | ||||
-rw-r--r-- | core/subsurface-qt/DiveListNotifier.h | 19 | ||||
-rw-r--r-- | desktop-widgets/command_divelist.cpp | 271 | ||||
-rw-r--r-- | desktop-widgets/command_divelist.h | 86 | ||||
-rw-r--r-- | desktop-widgets/divelistview.cpp | 49 | ||||
-rw-r--r-- | desktop-widgets/divelistview.h | 1 | ||||
-rw-r--r-- | desktop-widgets/mainwindow.cpp | 8 | ||||
-rw-r--r-- | desktop-widgets/tab-widgets/maintab.cpp | 4 | ||||
-rw-r--r-- | profile-widget/profilewidget2.cpp | 4 | ||||
-rw-r--r-- | qt-models/divetripmodel.cpp | 72 | ||||
-rw-r--r-- | qt-models/divetripmodel.h | 14 |
14 files changed, 489 insertions, 131 deletions
diff --git a/core/dive.h b/core/dive.h index 3077c5bce..48cbaa740 100644 --- a/core/dive.h +++ b/core/dive.h @@ -440,9 +440,8 @@ struct dive_table { extern struct dive_table dive_table, downloadTable; extern struct dive displayed_dive; extern struct dive_site displayed_dive_site; -extern int selected_dive; extern unsigned int dc_number; -#define current_dive (get_dive(selected_dive)) +extern struct dive *current_dive; #define current_dc (get_dive_dc(current_dive, dc_number)) #define displayed_dc (get_dive_dc(&displayed_dive, dc_number)) diff --git a/core/divelist.c b/core/divelist.c index 1c6ad9fc0..72685b40f 100644 --- a/core/divelist.c +++ b/core/divelist.c @@ -32,14 +32,18 @@ * void delete_single_dive(int idx) * void add_single_dive(int idx, struct dive *dive) * struct dive *merge_two_dives(struct dive *a, struct dive *b) - * void select_dive(int idx) - * void deselect_dive(int idx) + * void select_dive(struct dive *dive) + * void deselect_dive(struct dive *dive) * void mark_divelist_changed(int changed) * int unsaved_changes() * void remove_autogen_trips() * void sort_table(struct dive_table *table) * bool is_trip_before_after(const struct dive *dive, bool before) +<<<<<<< HEAD * void delete_dive_from_table(struct dive_table *table, int idx) +======= + * int find_next_visible_dive(timestamp_t when); +>>>>>>> Undo: select dives after add, remove, merge, split dive commands */ #include <unistd.h> #include <stdio.h> @@ -1058,7 +1062,8 @@ void delete_dive_from_table(struct dive_table *table, int idx) /* this removes a dive from the dive table and trip-list but doesn't * free the resources associated with the dive. It returns a pointer - * to the unregistered dive. */ + * to the unregistered dive. The returned dive has the selection- + * and hidden-flags cleared. */ struct dive *unregister_dive(int idx) { struct dive *dive = get_dive(idx); @@ -1068,6 +1073,7 @@ struct dive *unregister_dive(int idx) unregister_dive_from_table(&dive_table, idx); if (dive->selected) amount_selected--; + dive->selected = false; return dive; } @@ -1079,7 +1085,7 @@ void delete_single_dive(int idx) if (!dive) return; /* this should never happen */ if (dive->selected) - deselect_dive(idx); + deselect_dive(dive); dive = unregister_dive(idx); free_dive(dive); } @@ -1104,6 +1110,7 @@ struct dive **grow_dive_table(struct dive_table *table) * ordered reverse-chronologically */ int dive_get_insertion_index(struct dive *dive) { + /* we might want to use binary search here */ for (int i = 0; i < dive_table.nr; i++) { if (dive->when <= dive_table.dives[i]->when) return i; @@ -1235,42 +1242,42 @@ struct dive *merge_two_dives(struct dive *a, struct dive *b) return res; } -void select_dive(int idx) +void select_dive(struct dive *dive) { - struct dive *dive = get_dive(idx); - if (dive) { - /* never select an invalid dive that isn't displayed */ - if (!dive->selected) { - dive->selected = 1; - amount_selected++; - } - selected_dive = idx; + if (dive && !dive->selected) { + dive->selected = 1; + amount_selected++; + current_dive = dive; } } -void deselect_dive(int idx) +void deselect_dive(struct dive *dive) { - struct dive *dive = get_dive(idx); + int idx; if (dive && dive->selected) { dive->selected = 0; if (amount_selected) amount_selected--; - if (selected_dive == idx && amount_selected > 0) { + if (current_dive == dive && amount_selected > 0) { /* pick a different dive as selected */ + int selected_dive = idx = get_divenr(dive); while (--selected_dive >= 0) { dive = get_dive(selected_dive); - if (dive && dive->selected) + if (dive && dive->selected) { + current_dive = dive; return; + } } selected_dive = idx; while (++selected_dive < dive_table.nr) { dive = get_dive(selected_dive); - if (dive && dive->selected) + if (dive && dive->selected) { + current_dive = dive; return; + } } } - if (amount_selected == 0) - selected_dive = -1; + current_dive = NULL; } } @@ -1280,7 +1287,7 @@ void deselect_dives_in_trip(struct dive_trip *trip) if (!trip) return; for (dive = trip->dives; dive; dive = dive->next) - deselect_dive(get_divenr(dive)); + deselect_dive(dive); } void select_dives_in_trip(struct dive_trip *trip) @@ -1290,7 +1297,7 @@ void select_dives_in_trip(struct dive_trip *trip) return; for (dive = trip->dives; dive; dive = dive->next) if (!dive->hidden_by_filter) - select_dive(get_divenr(dive)); + select_dive(dive); } void filter_dive(struct dive *d, bool shown) @@ -1299,7 +1306,7 @@ void filter_dive(struct dive *d, bool shown) return; d->hidden_by_filter = !shown; if (!shown && d->selected) - deselect_dive(get_divenr(d)); + deselect_dive(d); } @@ -1610,6 +1617,7 @@ int get_dive_nr_at_idx(int idx) void set_dive_nr_for_current_dive() { + int selected_dive = get_divenr(current_dive); if (dive_table.nr == 1) current_dive->number = 1; else if (selected_dive == dive_table.nr - 1 && get_dive(dive_table.nr - 2)->number) @@ -1719,3 +1727,31 @@ timestamp_t get_surface_interval(timestamp_t when) return 0; return when - prev_end; } + +/* Find visible dive close to given date. First search towards older, + * then newer dives. */ +struct dive *find_next_visible_dive(timestamp_t when) +{ + int i, j; + + if (!dive_table.nr) + return NULL; + + /* we might want to use binary search here */ + for (i = 0; i < dive_table.nr; i++) { + if (when <= get_dive(i)->when) + break; + } + + for (j = i - 1; j > 0; j--) { + if (!get_dive(j)->hidden_by_filter) + return get_dive(j); + } + + for (j = i; j < dive_table.nr; j++) { + if (!get_dive(j)->hidden_by_filter) + return get_dive(j); + } + + return NULL; +} diff --git a/core/divelist.h b/core/divelist.h index 9449e36e2..a99394d5c 100644 --- a/core/divelist.h +++ b/core/divelist.h @@ -36,8 +36,8 @@ extern dive_trip_t *get_trip_for_new_dive(struct dive *new_dive, bool *allocated extern void autogroup_dives(void); extern struct dive *merge_two_dives(struct dive *a, struct dive *b); extern bool consecutive_selected(); -extern void select_dive(int idx); -extern void deselect_dive(int idx); +extern void select_dive(struct dive *dive); +extern void deselect_dive(struct dive *dive); extern void select_dives_in_trip(struct dive_trip *trip); extern void deselect_dives_in_trip(struct dive_trip *trip); extern void filter_dive(struct dive *d, bool shown); @@ -51,6 +51,7 @@ extern int get_dive_nr_at_idx(int idx); extern void set_dive_nr_for_current_dive(); extern timestamp_t get_surface_interval(timestamp_t when); extern void delete_dive_from_table(struct dive_table *table, int idx); +extern struct dive *find_next_visible_dive(timestamp_t when); int get_min_datafile_version(); void reset_min_datafile_version(); diff --git a/core/profile.c b/core/profile.c index af7ebe271..84c77d68b 100644 --- a/core/profile.c +++ b/core/profile.c @@ -27,7 +27,7 @@ #define MAX_PROFILE_DECO 7200 -int selected_dive = -1; /* careful: 0 is a valid value */ +struct dive *current_dive = NULL; unsigned int dc_number = 0; static struct plot_data *last_pi_entry_new = NULL; diff --git a/core/subsurface-qt/DiveListNotifier.h b/core/subsurface-qt/DiveListNotifier.h index fe5ea0e22..35ecf108d 100644 --- a/core/subsurface-qt/DiveListNotifier.h +++ b/core/subsurface-qt/DiveListNotifier.h @@ -17,10 +17,10 @@ signals: // Note that there are no signals for trips being added / created / time-shifted, // because these events never happen without a dive being added / created / time-shifted. - // We send one divesAdded, divesDeleted, divesChanged and divesTimeChanged signal per trip - // (non-associated dives being considered part of the null trip). This is ideal for the - // tree-view, but might be not-so-perfect for the list view, if trips intermingle or - // the deletion spans multiple trips. But most of the time only dives of a single trip + // We send one divesAdded, divesDeleted, divesChanged and divesTimeChanged, divesSelected + // signal per trip (non-associated dives being considered part of the null trip). This is + // ideal for the tree-view, but might be not-so-perfect for the list view, if trips intermingle + // or the deletion spans multiple trips. But most of the time only dives of a single trip // will be affected and trips don't overlap, so these considerations are moot. // Notes: // - The dives are always sorted by start-time. @@ -31,7 +31,16 @@ signals: void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives); void divesTimeChanged(dive_trip *trip, timestamp_t delta, const QVector<dive *> &dives); - // This signal is sent if the selection of dives and/or the current dive changed + // Selection-signals come in two kinds: + // - divesSelected, divesDeselected and currentDiveChanged are finer grained and are + // called batch-wise per trip (except currentDiveChanged, of course). These signals + // are used by the dive-list model and view to correctly highlight the correct dives. + // - selectionChanged() is called once at the end of commands if either the selection + // or the current dive changed. It is used by the main-window / profile to update + // their data. + void divesSelected(dive_trip *trip, const QVector<dive *> &dives); + void divesDeselected(dive_trip *trip, const QVector<dive *> &dives); + void currentDiveChanged(); void selectionChanged(); public: // Desktop uses the QTreeView class to present the list of dives. The layout diff --git a/desktop-widgets/command_divelist.cpp b/desktop-widgets/command_divelist.cpp index a75004bea..9cf61eff9 100644 --- a/desktop-widgets/command_divelist.cpp +++ b/desktop-widgets/command_divelist.cpp @@ -4,6 +4,7 @@ #include "desktop-widgets/mainwindow.h" #include "desktop-widgets/divelistview.h" #include "core/divelist.h" +#include "core/display.h" // for amount_selected #include "core/subsurface-qt/DiveListNotifier.h" namespace Command { @@ -42,8 +43,20 @@ void processByTrip(std::vector<std::pair<dive_trip *, dive *>> &dives, Function // This helper function removes a dive, takes ownership of the dive and adds it to a DiveToAdd structure. // It is crucial that dives are added in reverse order of deletion, so the the indices are correctly // set and that the trips are added before they are used! -static DiveToAdd removeDive(struct dive *d) -{ +DiveToAdd DiveListBase::removeDive(struct dive *d) +{ + // If the dive to be removed is selected, we will inform the frontend + // later via a signal that the dive changed. + if (d->selected) + selectionChanged = true; + + // If the dive was the current dive, reset the current dive. The calling + // command is responsible of finding a new dive. + if (d == current_dive) { + selectionChanged = true; // Should have been set above, as current dive is always selected. + current_dive = nullptr; + } + DiveToAdd res; res.idx = get_divenr(d); if (res.idx < 0) @@ -65,7 +78,7 @@ static DiveToAdd removeDive(struct dive *d) // This helper function adds a dive and returns ownership to the backend. It may also add a dive trip. // It is crucial that dives are added in reverse order of deletion (see comment above)! // Returns pointer to added dive (which is owned by the backend!) -static dive *addDive(DiveToAdd &d) +dive *DiveListBase::addDive(DiveToAdd &d) { if (d.tripToAdd) insert_trip_dont_merge(d.tripToAdd.release()); // Return ownership to backend @@ -74,13 +87,18 @@ static dive *addDive(DiveToAdd &d) dive *res = d.dive.release(); // Give up ownership of dive add_single_dive(d.idx, res); // Return ownership to backend + // If the dive to be removed is selected, we will inform the frontend + // later via a signal that the dive changed. + if (res->selected) + selectionChanged = true; + return res; } // This helper function calls removeDive() on a list of dives to be removed and // returns a vector of corresponding DiveToAdd objects, which can later be readded. // The passed in vector is cleared. -static std::vector<DiveToAdd> removeDives(std::vector<dive *> &divesToDelete) +std::vector<DiveToAdd> DiveListBase::removeDives(std::vector<dive *> &divesToDelete) { std::vector<DiveToAdd> res; res.reserve(divesToDelete.size()); @@ -90,7 +108,7 @@ static std::vector<DiveToAdd> removeDives(std::vector<dive *> &divesToDelete) divesToDelete.clear(); // We send one dives-deleted signal per trip (see comments in DiveListNotifier.h). - // Therefore, collect all dives in a array and sort by trip. + // Therefore, collect all dives in an array and sort by trip. std::vector<std::pair<dive_trip *, dive *>> dives; dives.reserve(res.size()); for (const DiveToAdd &entry: res) @@ -112,7 +130,7 @@ static std::vector<DiveToAdd> removeDives(std::vector<dive *> &divesToDelete) // of dives to be (re)added and returns a vector of the added dives. It does this in reverse // order, so that trips are created appropriately and indexing is correct. // The passed in vector is cleared. -static std::vector<dive *> addDives(std::vector<DiveToAdd> &divesToAdd) +std::vector<dive *> DiveListBase::addDives(std::vector<DiveToAdd> &divesToAdd) { std::vector<dive *> res; res.reserve(divesToAdd.size()); @@ -289,6 +307,154 @@ static void moveDivesBetweenTrips(DivesToTrip &dives) std::reverse(dives.divesToMove.begin(), dives.divesToMove.end()); } +// When we initialize the command we don't have to roll-back any selection change +DiveListBase::DiveListBase() : firstExecution(true) +{ +} + +// Turn current selection into a vector. +// TODO: This could be made much more efficient if we kept a sorted list of selected dives! +static std::vector<dive *> getDiveSelection() +{ + std::vector<dive *> res; + res.reserve(amount_selected); + + int i; + dive *d; + for_each_dive(i, d) { + if (d->selected) + res.push_back(d); + } + return res; +} + +void DiveListBase::initWork() +{ + selectionChanged = false; +} + +void DiveListBase::finishWork() +{ + if (selectionChanged) // If the selection changed -> tell the frontend + emit diveListNotifier.selectionChanged(); +} + +// Set the current dive either from a list of selected dives, +// or a newly selected dive. In both cases, try to select the +// dive that is newer that is newer than the given date. +// This mimics the old behavior when the current dive changed. +static void setClosestCurrentDive(timestamp_t when, const std::vector<dive *> &selection) +{ + // Start from back until we get the first dive that is before + // the supposed-to-be selected dive. (Note: this mimics the + // old behavior when the current dive changed). + for (auto it = selection.rbegin(); it < selection.rend(); ++it) { + if ((*it)->when > when && !(*it)->hidden_by_filter) { + current_dive = *it; + return; + } + } + + // We didn't find a more recent selected dive -> try to + // find *any* visible selected dive. + for (dive *d: selection) { + if (!d->hidden_by_filter) { + current_dive = d; + return; + } + } + + // No selected dive is visible! Take the closest dive. Note, this might + // return null, but that just means unsetting the current dive (as no + // dive is visible anyway). + current_dive = find_next_visible_dive(when); +} + +// Rese the selection to the dives of the "selection" vector and send the appropriate signals. +// Set the current dive to "currentDive". "currentDive" must be an element of "selection" (or +// null if "seletion" is empty). +void DiveListBase::restoreSelection(const std::vector<dive *> &selection, dive *currentDive) +{ + // To do so, generate vectors of dives to be selected and deselected. + // We send signals batched by trip, so keep track of trip/dive pairs. + std::vector<std::pair<dive_trip *, dive *>> divesToSelect; + std::vector<std::pair<dive_trip *, dive *>> divesToDeselect; + + // TODO: We might want to keep track of selected dives in a more efficient way! + int i; + dive *d; + amount_selected = 0; // We recalculate amount_selected + for_each_dive(i, d) { + // We only modify dives that are currently visible. + if (d->hidden_by_filter) { + d->selected = false; // Note, not necessary, just to be sure + // that we get amount_selected right + continue; + } + + // Search the dive in the list of selected dives. + // TODO: By sorting the list in the same way as the backend, this could be made more efficient. + bool newState = std::find(selection.begin(), selection.end(), d) != selection.end(); + + // TODO: Instead of using select_dive() and deselect_dive(), we set selected directly. + // The reason is that deselect() automatically sets a new current dive, which we + // don't want, as we set it later anyway. + // There is other parts of the C++ code that touches the innards directly, but + // ultimately this should be pushed down to C. + if (newState && !d->selected) { + d->selected = true; + ++amount_selected; + divesToSelect.push_back({ d->divetrip, d }); + } else if (!newState && d->selected) { + d->selected = false; + divesToDeselect.push_back({ d->divetrip, d }); + } + } + + // Send the select and deselect signals + processByTrip(divesToSelect, [&](dive_trip *trip, const QVector<dive *> &divesInTrip) { + emit diveListNotifier.divesSelected(trip, divesInTrip); + }); + processByTrip(divesToDeselect, [&](dive_trip *trip, const QVector<dive *> &divesInTrip) { + emit diveListNotifier.divesDeselected(trip, divesInTrip); + }); + + bool currentDiveChanged = false; + if (current_dive != currentDive) { + currentDiveChanged = true; + + // We cannot simply change the currentd dive to the given dive. + // It might be hidden by a filter and thus not be selected. + if (currentDive->selected) + // Current dive is visible and selected. Excellent. + current_dive = currentDive; + else + // Current not visible -> find a different dive. + setClosestCurrentDive(currentDive->when, selection); + emit diveListNotifier.currentDiveChanged(); + } + + // If anything changed (selection or current dive), send a final signal. + if (!divesToSelect.empty() || !divesToDeselect.empty() || currentDiveChanged) + selectionChanged = true; +} + +void DiveListBase::undo() +{ + auto marker = diveListNotifier.enterCommand(); + initWork(); + undoit(); + finishWork(); +} + +void DiveListBase::redo() +{ + auto marker = diveListNotifier.enterCommand(); + initWork(); + redoit(); + finishWork(); +} + AddDive::AddDive(dive *d, bool autogroup) { setText(tr("add dive")); @@ -321,30 +487,31 @@ bool AddDive::workToBeDone() return true; } -void AddDive::redo() +void AddDive::redoit() { - auto marker = diveListNotifier.enterCommand(); + // Remember selection so that we can undo it + selection = getDiveSelection(); + currentDive = current_dive; - int idx = divesToAdd[0].idx; divesToRemove = addDives(divesToAdd); mark_divelist_changed(true); - // Finally, do the UI stuff: - MainWindow::instance()->dive_list()->unselectDives(); - MainWindow::instance()->dive_list()->selectDive(idx, true); + // Select the newly added dive + restoreSelection(divesToRemove, divesToRemove[0]); // Exit from edit mode, but don't recalculate dive list // TODO: Remove edit mode MainWindow::instance()->refreshDisplay(false); } -void AddDive::undo() +void AddDive::undoit() { - auto marker = diveListNotifier.enterCommand(); - - // Simply remove the dive that was previously added + // Simply remove the dive that was previously added... divesToAdd = removeDives(divesToRemove); + // ...and restore the selection + restoreSelection(selection, currentDive); + // Exit from edit mode, but don't recalculate dive list // TODO: Remove edit mode MainWindow::instance()->refreshDisplay(false); @@ -360,18 +527,31 @@ bool DeleteDive::workToBeDone() return !divesToDelete.empty(); } -void DeleteDive::undo() +void DeleteDive::undoit() { - auto marker = diveListNotifier.enterCommand(); divesToDelete = addDives(divesToAdd); mark_divelist_changed(true); + + // Select all re-added dives and make the first one current + dive *currentDive = !divesToDelete.empty() ? divesToDelete[0] : nullptr; + restoreSelection(divesToDelete, currentDive); } -void DeleteDive::redo() +void DeleteDive::redoit() { - auto marker = diveListNotifier.enterCommand(); divesToAdd = removeDives(divesToDelete); mark_divelist_changed(true); + + // Deselect all dives and select dive that was close to the first deleted dive + dive *newCurrent = nullptr; + if (!divesToAdd.empty()) { + timestamp_t when = divesToAdd[0].dive->when; + newCurrent = find_next_visible_dive(when); + } + if (newCurrent) + restoreSelection(std::vector<dive *>{ newCurrent }, newCurrent); + else + restoreSelection(std::vector<dive *>(), nullptr); } @@ -381,9 +561,8 @@ ShiftTime::ShiftTime(const QVector<dive *> &changedDives, int amount) setText(tr("shift time of %n dives", "", changedDives.count())); } -void ShiftTime::redo() +void ShiftTime::redoit() { - auto marker = diveListNotifier.enterCommand(); for (dive *d: diveList) d->when -= timeChanged; @@ -413,10 +592,10 @@ bool ShiftTime::workToBeDone() return !diveList.isEmpty(); } -void ShiftTime::undo() +void ShiftTime::undoit() { - // Same as redo(), since after redo() we reversed the timeOffset - redo(); + // Same as redoit(), since after redoit() we reversed the timeOffset + redoit(); } @@ -425,9 +604,8 @@ RenumberDives::RenumberDives(const QVector<QPair<dive *, int>> &divesToRenumberI setText(tr("renumber %n dive(s)", "", divesToRenumber.count())); } -void RenumberDives::undo() +void RenumberDives::undoit() { - auto marker = diveListNotifier.enterCommand(); renumberDives(divesToRenumber); mark_divelist_changed(true); } @@ -437,10 +615,10 @@ bool RenumberDives::workToBeDone() return !divesToRenumber.isEmpty(); } -void RenumberDives::redo() +void RenumberDives::redoit() { // Redo and undo do the same thing! - undo(); + undoit(); } bool TripBase::workToBeDone() @@ -448,18 +626,17 @@ bool TripBase::workToBeDone() return !divesToMove.divesToMove.empty(); } -void TripBase::redo() +void TripBase::redoit() { - auto marker = diveListNotifier.enterCommand(); moveDivesBetweenTrips(divesToMove); mark_divelist_changed(true); } -void TripBase::undo() +void TripBase::undoit() { // Redo and undo do the same thing! - redo(); + redoit(); } RemoveDivesFromTrip::RemoveDivesFromTrip(const QVector<dive *> &divesToRemove) @@ -540,7 +717,7 @@ SplitDives::SplitDives(dive *d, duration_t time) split_dive_dont_insert(d, &new1, &new2) : split_dive_at_time_dont_insert(d, time, &new1, &new2); - // If this didn't work, reset pointers so that redo() and undo() do nothing + // If this didn't work, reset pointers as a mark that nothing is to be done. if (idx < 0) { diveToSplit = nullptr; divesToUnsplit[0] = divesToUnsplit[1]; @@ -561,23 +738,27 @@ bool SplitDives::workToBeDone() return !!diveToSplit; } -void SplitDives::redo() +void SplitDives::redoit() { - auto marker = diveListNotifier.enterCommand(); divesToUnsplit[0] = addDive(splitDives[0]); divesToUnsplit[1] = addDive(splitDives[1]); unsplitDive = removeDive(diveToSplit); mark_divelist_changed(true); + + // Select split dives and make first dive current + restoreSelection(std::vector<dive *>{ divesToUnsplit[0], divesToUnsplit[1] }, divesToUnsplit[0]); } -void SplitDives::undo() +void SplitDives::undoit() { - // Note: reverse order with respect to redo() - auto marker = diveListNotifier.enterCommand(); + // Note: reverse order with respect to redoit() diveToSplit = addDive(unsplitDive); splitDives[1] = removeDive(divesToUnsplit[1]); splitDives[0] = removeDive(divesToUnsplit[0]); mark_divelist_changed(true); + + // Select unsplit dive and make it current + restoreSelection(std::vector<dive *>{ diveToSplit }, diveToSplit); } MergeDives::MergeDives(const QVector <dive *> &dives) @@ -664,20 +845,24 @@ bool MergeDives::workToBeDone() return !!mergedDive.dive; } -void MergeDives::redo() +void MergeDives::redoit() { - auto marker = diveListNotifier.enterCommand(); renumberDives(divesToRenumber); diveToUnmerge = addDive(mergedDive); unmergedDives = removeDives(divesToMerge); + + // Select merged dive and make it current + restoreSelection(std::vector<dive *>{ diveToUnmerge }, diveToUnmerge); } -void MergeDives::undo() +void MergeDives::undoit() { - auto marker = diveListNotifier.enterCommand(); divesToMerge = addDives(unmergedDives); mergedDive = removeDive(diveToUnmerge); renumberDives(divesToRenumber); + + // Select unmerged dives and make first one current + restoreSelection(divesToMerge, divesToMerge[0]); } } // namespace Command diff --git a/desktop-widgets/command_divelist.h b/desktop-widgets/command_divelist.h index e679369af..01dd4db77 100644 --- a/desktop-widgets/command_divelist.h +++ b/desktop-widgets/command_divelist.h @@ -38,12 +38,54 @@ struct DivesToTrip std::vector<OwningTripPtr> tripsToAdd; }; -class AddDive : public Base { -public: - AddDive(dive *dive, bool autogroup); +// All divelist commands derive from a common base class, which has a flag +// for when then selection changed. In such a case, in the redo() and undo() +// methods a signal will be sent. The base-class implements redo() and undo(), +// which resets the flag and sends a signal. Derived classes implement the +// virtual methods redoit() and undoit() [Yes, the names could be more expressive]. +// Moreover, the class implements helper methods, which set the selectionChanged +// flag accordingly. +class DiveListBase : public Base { +protected: + DiveListBase(); + + // These are helper functions to add / remove dive from the C-core structures, + // which set the selectionChanged flag if the added / removed dive was selected. + DiveToAdd removeDive(struct dive *d); + dive *addDive(DiveToAdd &d); + std::vector<DiveToAdd> removeDives(std::vector<dive *> &divesToDelete); + std::vector<dive *> addDives(std::vector<DiveToAdd> &divesToAdd); + + // Set the selection to a given state. Set the selectionChanged flag if anything changed. + void restoreSelection(const std::vector<dive *> &selection, dive *currentDive); + + // On first execution, the selections before and after execution will + // be remembered. On all further executions, the selection will be reset to + // the remembered values. + // Note: Therefore, commands should manipulate the selection and send the + // corresponding signals only on first execution! + bool firstExecution; + + // Commands set this flag if the selection changed on first execution. + // Only then, a new the divelist will be scanned again after the command. + // If this flag is set on first execution, a selectionChanged signal will + // be sent. + bool selectionChanged; private: + void initWork(); // reset selectionChanged flag + void finishWork(); // emit signal if selection changed void undo() override; void redo() override; + virtual void redoit() = 0; + virtual void undoit() = 0; +}; + +class AddDive : public DiveListBase { +public: + AddDive(dive *dive, bool autogroup); +private: + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo @@ -53,14 +95,16 @@ private: // For undo std::vector<dive *> divesToRemove; + std::vector<dive *> selection; + dive * currentDive; }; -class DeleteDive : public Base { +class DeleteDive : public DiveListBase { public: DeleteDive(const QVector<dive *> &divesToDelete); private: - void undo() override; - void redo() override; + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo @@ -70,12 +114,12 @@ private: std::vector<DiveToAdd> divesToAdd; }; -class ShiftTime : public Base { +class ShiftTime : public DiveListBase { public: ShiftTime(const QVector<dive *> &changedDives, int amount); private: - void undo() override; - void redo() override; + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo and undo @@ -83,12 +127,12 @@ private: int timeChanged; }; -class RenumberDives : public Base { +class RenumberDives : public DiveListBase { public: RenumberDives(const QVector<QPair<dive *, int>> &divesToRenumber); private: - void undo() override; - void redo() override; + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo and undo: pairs of dive-id / new number @@ -99,10 +143,10 @@ private: // and MergeTrips all do the same thing, just the intialization differs. // Therefore, define a base class with the proper data-structures, redo() // and undo() functions and derive to specialize the initialization. -class TripBase : public Base { +class TripBase : public DiveListBase { protected: - void undo() override; - void redo() override; + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo and undo @@ -127,13 +171,13 @@ struct MergeTrips : public TripBase { MergeTrips(dive_trip *trip1, dive_trip *trip2); }; -class SplitDives : public Base { +class SplitDives : public DiveListBase { public: // If time is < 0, split at first surface interval SplitDives(dive *d, duration_t time); private: - void undo() override; - void redo() override; + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo @@ -147,12 +191,12 @@ private: dive *divesToUnsplit[2]; }; -class MergeDives : public Base { +class MergeDives : public DiveListBase { public: MergeDives(const QVector<dive *> &dives); private: - void undo() override; - void redo() override; + void undoit() override; + void redoit() override; bool workToBeDone() override; // For redo diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp index 2a0d7028c..8ed07b4db 100644 --- a/desktop-widgets/divelistview.cpp +++ b/desktop-widgets/divelistview.cpp @@ -44,6 +44,7 @@ DiveListView::DiveListView(QWidget *parent) : QTreeView(parent), mouseClickSelec setContextMenuPolicy(Qt::DefaultContextMenu); setSelectionMode(ExtendedSelection); header()->setContextMenuPolicy(Qt::ActionsContextMenu); + connect(DiveTripModel::instance(), &DiveTripModel::selectionChanged, this, &DiveListView::diveSelectionChanged); header()->setStretchLastSection(true); @@ -176,23 +177,37 @@ void DiveListView::reset() } } +// If items were selected, inform the selection model +void DiveListView::diveSelectionChanged(const QVector<QModelIndex> &indexes, bool select) +{ + MultiFilterSortModel *m = MultiFilterSortModel::instance(); + QItemSelectionModel *s = selectionModel(); + auto flags = select ? + QItemSelectionModel::Rows | QItemSelectionModel::Select : + QItemSelectionModel::Rows | QItemSelectionModel::Deselect; + for (const QModelIndex &index: indexes) { + // We have to transform the indices into local indices, since + // there might be sorting or filtering in effect. + QModelIndex localIndex = m->mapFromSource(index); + + // It might be possible that the item is not shown (filter is + // in effect). Then we get an invalid index and should ignore + // this selection. + if (!localIndex.isValid()) + continue; + + s->select(localIndex, flags); + } +} + // If rows are added, check which of these rows is a trip and expand the first column void DiveListView::rowsInserted(const QModelIndex &parent, int start, int end) { // First, let the QTreeView do its thing. QTreeView::rowsInserted(parent, start, end); + // Check for each inserted row whether this is a trip and expand the first column QAbstractItemModel *m = model(); - QItemSelectionModel *s = selectionModel(); - - // Check whether any of the items is selected - for (int i = start; i <= end; ++i) { - QModelIndex index = m->index(i, 0, parent); - if (m->data(index, DiveTripModel::SELECTED_ROLE).toBool()) - s->select(index, QItemSelectionModel::Select | QItemSelectionModel::Rows); - } - - // Now check for each inserted row whether this is a trip and expand the first column if (parent.isValid()) // Trips don't have a parent return; for (int i = start; i <= end; ++i) { @@ -284,7 +299,7 @@ void DiveListView::clearTripSelection() void DiveListView::unselectDives() { // make sure we don't try to redraw the dives during the selection change - selected_dive = -1; + current_dive = nullptr; amount_selected = 0; // clear the Qt selection selectionModel()->clearSelection(); @@ -353,7 +368,7 @@ void DiveListView::selectDives(const QList<int> &newDiveSelection) while (!sortedSelection.isEmpty()) selectDive(sortedSelection.takeLast()); - while (selected_dive == -1) { + while (!current_dive) { // that can happen if we restored a selection after edit // and the only selected dive is no longer visible because of a filter newSelection--; @@ -365,7 +380,7 @@ void DiveListView::selectDives(const QList<int> &newDiveSelection) selectDive(newSelection); } QSortFilterProxyModel *m = qobject_cast<QSortFilterProxyModel *>(model()); - QModelIndexList idxList = m->match(m->index(0, 0), DiveTripModel::DIVE_IDX, selected_dive, 2, Qt::MatchRecursive); + QModelIndexList idxList = m->match(m->index(0, 0), DiveTripModel::DIVE_IDX, get_divenr(current_dive), 2, Qt::MatchRecursive); if (!idxList.isEmpty()) { QModelIndex idx = idxList.first(); if (idx.parent().isValid()) @@ -441,7 +456,7 @@ void DiveListView::reload(DiveTripModel::Layout layout, bool forceSort) QSortFilterProxyModel *m = qobject_cast<QSortFilterProxyModel *>(model()); sortByColumn(sortColumn, currentOrder); if (amount_selected && current_dive != NULL) { - selectDive(selected_dive, true); + selectDive(get_divenr(current_dive), true); } else { QModelIndex firstDiveOrTrip = m->index(0, 0); if (firstDiveOrTrip.isValid()) { @@ -567,7 +582,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS if (!dive) // it's a trip! deselect_dives_in_trip((dive_trip_t *)model->data(index, DiveTripModel::TRIP_ROLE).value<void *>()); else - deselect_dive(get_divenr(dive)); + deselect_dive(dive); } Q_FOREACH (const QModelIndex &index, newSelected.indexes()) { if (index.column() != 0) @@ -586,7 +601,7 @@ void DiveListView::selectionChanged(const QItemSelection &selected, const QItemS expand(index); } } else { - select_dive(get_divenr(dive)); + select_dive(dive); } } if (!dontEmitDiveChangedSignal) @@ -889,7 +904,7 @@ void DiveListView::contextMenuEvent(QContextMenuEvent *event) QAction *actionTaken = popup.exec(event->globalPos()); if (actionTaken == collapseAction && collapseAction) { this->setAnimated(false); - selectDive(selected_dive, true); + selectDive(get_divenr(current_dive), true); scrollTo(selectedIndexes().first()); this->setAnimated(true); } diff --git a/desktop-widgets/divelistview.h b/desktop-widgets/divelistview.h index 9e6857312..effde2ece 100644 --- a/desktop-widgets/divelistview.h +++ b/desktop-widgets/divelistview.h @@ -58,6 +58,7 @@ slots: void shiftTimes(); void loadImages(); void loadWebImages(); + void diveSelectionChanged(const QVector<QModelIndex> &indexes, bool select); private: bool mouseClickSelection; QList<int> expandedRows; diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp index 5ec086195..72b4e4720 100644 --- a/desktop-widgets/mainwindow.cpp +++ b/desktop-widgets/mainwindow.cpp @@ -504,7 +504,7 @@ void MainWindow::recreateDiveList() } void MainWindow::configureToolbar() { - if (selected_dive > 0) { + if (current_dive) { bool freeDiveMode = current_dive->dc.divemode == FREEDIVE; ui.profCalcCeiling->setDisabled(freeDiveMode); ui.profCalcCeiling->setDisabled(freeDiveMode); @@ -870,7 +870,7 @@ void MainWindow::refreshProfile() { showProfile(); configureToolbar(); - graphics()->replot(get_dive(selected_dive)); + graphics()->replot(current_dive); DivePictureModel::instance()->updateDivePictures(); } @@ -889,8 +889,8 @@ void MainWindow::planCreated() if (displayed_dive.id == 0) { // we might have added a new dive (so displayed_dive was cleared out by clone_dive() dive_list()->unselectDives(); - select_dive(dive_table.nr - 1); - dive_list()->selectDive(selected_dive); + select_dive(get_dive(dive_table.nr - 1)); + dive_list()->selectDive(get_divenr(current_dive)); // TODO: don't convert dive->idx and back set_dive_nr_for_current_dive(); } // make sure our UI is in a consistent state diff --git a/desktop-widgets/tab-widgets/maintab.cpp b/desktop-widgets/tab-widgets/maintab.cpp index 5f740b969..92e55784f 100644 --- a/desktop-widgets/tab-widgets/maintab.cpp +++ b/desktop-widgets/tab-widgets/maintab.cpp @@ -1058,11 +1058,11 @@ void MainTab::rejectChanges() MainWindow::instance()->dive_list()->restoreSelection(); // now make sure that the correct dive is displayed - if (selected_dive >= 0) + if (current_dive) copy_dive(current_dive, &displayed_dive); else clear_dive(&displayed_dive); - updateDiveInfo(selected_dive < 0); + updateDiveInfo(!current_dive); for (auto widget : extraWidgets) { widget->updateData(); diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 9221ff293..3b932144b 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -559,7 +559,7 @@ void ProfileWidget2::plotDive(struct dive *d, bool force, bool doClearPictures) #endif if (currentState != ADD && currentState != PLAN) { if (!d) { - if (selected_dive == -1) + if (!current_dive) return; d = current_dive; // display the current dive } @@ -1413,7 +1413,7 @@ void ProfileWidget2::contextMenuEvent(QContextMenuEvent *event) } QMenu m; bool isDCName = false; - if (selected_dive == -1) + if (!current_dive) return; // figure out if we are ontop of the dive computer name in the profile QGraphicsItem *sceneItem = itemAt(mapFromGlobal(event->globalPos())); diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index f8b5e0deb..7c027e4b4 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -440,6 +440,8 @@ DiveTripModel::DiveTripModel(QObject *parent) : connect(&diveListNotifier, &DiveListNotifier::divesChanged, this, &DiveTripModel::divesChanged); connect(&diveListNotifier, &DiveListNotifier::divesMovedBetweenTrips, this, &DiveTripModel::divesMovedBetweenTrips); connect(&diveListNotifier, &DiveListNotifier::divesTimeChanged, this, &DiveTripModel::divesTimeChanged); + connect(&diveListNotifier, &DiveListNotifier::divesSelected, this, &DiveTripModel::divesSelected); + connect(&diveListNotifier, &DiveListNotifier::divesDeselected, this, &DiveTripModel::divesDeselected); } int DiveTripModel::columnCount(const QModelIndex&) const @@ -978,7 +980,7 @@ void DiveTripModel::divesAdded(dive_trip *trip, bool addTrip, const QVector<dive void DiveTripModel::divesDeleted(dive_trip *trip, bool deleteTrip, const QVector<dive *> &divesIn) { // TODO: dives comes sorted by ascending time, but the model is sorted by descending time. - // Instead of being smart, simple reverse the input array. + // Instead of being smart, simply reverse the input array. QVector<dive *> dives = divesIn; std::reverse(dives.begin(), dives.end()); @@ -1033,7 +1035,7 @@ void DiveTripModel::divesDeleted(dive_trip *trip, bool deleteTrip, const QVector void DiveTripModel::divesChanged(dive_trip *trip, const QVector<dive *> &divesIn) { // TODO: dives comes sorted by ascending time, but the model is sorted by descending time. - // Instead of being smart, simple reverse the input array. + // Instead of being smart, simply reverse the input array. QVector<dive *> dives = divesIn; std::reverse(dives.begin(), dives.end()); @@ -1100,15 +1102,67 @@ void DiveTripModel::divesTimeChanged(dive_trip *trip, timestamp_t delta, const Q // order of the dives don't change. This is indeed the case, as all starting-times where // moved by the same delta. - // Unfortunately, deleting of the dives clears current_dive, so we have to remember it. - // TODO: remove this hack! - dive *current = current_dive; - // Cheating! divesDeleted(trip, false, dives); divesAdded(trip, false, dives); +} + +void DiveTripModel::divesSelected(dive_trip *trip, const QVector<dive *> &dives) +{ + changeDiveSelection(trip, dives, true); +} + +void DiveTripModel::divesDeselected(dive_trip *trip, const QVector<dive *> &dives) +{ + changeDiveSelection(trip, dives, false); +} + +void DiveTripModel::changeDiveSelection(dive_trip *trip, const QVector<dive *> &divesIn, bool select) +{ + // TODO: dives comes sorted by ascending time, but the model is sorted by descending time. + // Instead of being smart, simply reverse the input array. + QVector<dive *> dives = divesIn; + std::reverse(dives.begin(), dives.end()); + + // We got a number of dives that have been selected. Turn this into QModelIndexes and + // emit a signal, so that views can change the selection. + QVector<QModelIndex> indexes; + indexes.reserve(dives.count()); + + if (!trip || currentLayout == LIST) { + // Either this is outside of a trip or we're in list mode. + // Since both lists are sorted, we can do this linearly. Perhaps a binary search + // would be better? + int j = 0; // Index in items array + for (int i = 0; i < dives.size(); ++i) { + while (j < (int)items.size() && !items[j].isDive(dives[i])) + ++j; + if (j >= (int)items.size()) + break; + indexes.append(createIndex(j, 0, noParent)); + } + } else { + // Find the trip. + int idx = findTripIdx(trip); + if (idx < 0) { + // We don't know the trip - this shouldn't happen. We seem to have + // missed some signals! + qWarning() << "DiveTripModel::divesSelected(): unknown trip"; + return; + } + // Locate the indices inside the trip. + // Since both lists are sorted, we can do this linearly. Perhaps a binary search + // would be better? + int j = 0; // Index in items array + const Item &entry = items[idx]; + for (int i = 0; i < dives.size(); ++i) { + while (j < (int)entry.dives.size() && entry.dives[j] != dives[i]) + ++j; + if (j >= (int)entry.dives.size()) + break; + indexes.append(createIndex(j, 0, idx)); + } + } - // Now, restore current_dive - // TODO: remove this hack! - selected_dive = get_divenr(current); + emit selectionChanged(indexes, select); } diff --git a/qt-models/divetripmodel.h b/qt-models/divetripmodel.h index aa2e046d7..5cd69852c 100644 --- a/qt-models/divetripmodel.h +++ b/qt-models/divetripmodel.h @@ -109,12 +109,23 @@ public: int rowCount(const QModelIndex &parent) const; QModelIndex index(int row, int column, const QModelIndex &parent) const; QModelIndex parent(const QModelIndex &index) const; +signals: + // The propagation of selection changes is complex. + // The control flow of dive-selection goes: + // Commands/DiveListNotifier ---(dive */dive_trip *)---> DiveTripModel ---(QModelIndex)---> DiveListView + // i.e. The command objects send changes in terms of pointer-to-dives, which the DiveTripModel transforms + // into QModelIndexes according to the current view (tree/list). Finally, the DiveListView transforms these + // indexes into local indexes according to current sorting/filtering and instructs the QSelectionModel to + // perform the appropriate actions. + void selectionChanged(const QVector<QModelIndex> &indexes, bool select); private slots: void divesAdded(dive_trip *trip, bool addTrip, const QVector<dive *> &dives); void divesDeleted(dive_trip *trip, bool deleteTrip, const QVector<dive *> &dives); void divesChanged(dive_trip *trip, const QVector<dive *> &dives); void divesTimeChanged(dive_trip *trip, timestamp_t delta, const QVector<dive *> &dives); void divesMovedBetweenTrips(dive_trip *from, dive_trip *to, bool deleteFrom, bool createTo, const QVector<dive *> &dives); + void divesSelected(dive_trip *trip, const QVector<dive *> &dives); + void divesDeselected(dive_trip *trip, const QVector<dive *> &dives); private: // The model has up to two levels. At the top level, we have either trips or dives // that do not belong to trips. Such a top-level item is represented by the "Item" @@ -142,6 +153,9 @@ private: int findDiveInTrip(int tripIdx, const dive *d) const; // Find dive inside trip. Second parameter is index of trip int findInsertionIndex(timestamp_t when) const; // Where to insert item with timestamp "when" + // Select or deselect dives + void changeDiveSelection(dive_trip *trip, const QVector<dive *> &dives, bool select); + // Addition and deletion of dives void addDivesToTrip(int idx, const QVector<dive *> &dives); |