The DiveTripModels are sorted in *reverse* chronological order.
Therefore, when comparing a dive against a trip, the dive has
to be inserted if the dive has a *later* date. Change the
comparison accordingly.
Reported-by: Jan Mulder <jlmulder@xs4all.nl>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The generic addInBatches() function is used to add batches of
contiguous sets of dives to the dive-list models. The loop
searching for the end of the batch used the wrong index and
would therefore not properly cut the batches.
Fix this.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The dive list was sorted using the default-sorter of
QSortFilterProxy model. This is mighty inflexible as it
considers only one column. This has the funky effect that
for rows with identical elements, the sort order depends
on the previous sorting.
Implement a lessThan() function in the MultiFilterSortModel,
which simply hands the sorting down to the actual model.
This might be considered a layering violation, but it makes
things so much easier.
Sadly, it seems like the column-to-be-sorted is transported
in the provided indices. Therefore, the comparison is chosen
using a switch for *every* comparison. It would seem much
more logical to set a function pointer once and use that.
Further investigations are necessary.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The data-flow from C-core to list-view is as follows:
C-core --> DiveTripModel --> MultiSortFilterModel --> DiveListView
The control-flow, on the other hand, differs as DiveListView
accesses both MultiSortFilterModel and DiveTripModel, whereas
MultiSortFilterModel is mostly unaware of its source model.
This is in principle legitimate, as the MultiSortFilterModel might
be used for different sources. In our particular case, this is
not so. MultiSortFilterModel is written for a particular use case.
Therefore, model control-flow follow after data-flow: Let MultiSortFilterModel
set its own source model and DiveListView access the MultiSortFilterModel,
which then manages its source model.
This is not bike-shedding, but will enable a more flexible and
higher-performance sorting.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Now that struct dive_site * is a proper Q_METATYPE it is not
necessary anymore to pass dive-sites as opaque uintptr_t types.
Simply pass a QVariants or directly via dive_site *.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
There was this ugly pattern of passing pointers-to-dive_site via
a QVariant of void * type. This is of course inherently unsafe.
Pass these pointers using their proper types instead. This makes
it necessary to register them in Qt's meta-type system. Doing so,
fixes a bug: QML couldn't call into updateDiveSiteCoordinates()
because it didn't know the type and thus the coordinates of
the moved flag were not reflected in the divesite-dialog.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
To test whether to show a dive, the UUIDs of the filtered-by
location and the dive-site of a dive were compared. Since UUIDs
are unique (as the name implies), directly compare pointers.
Note: this code comes from a time when the filtered-by location
was not a pointer, but a copy.
Moreover, the if tested first for the same name, then (logical-or)
for the same uuid. This makes no sense, as the same dive-site
implies the same name. This code likewise can be explained by
historic reasons: the filtered-by location may have contained
a different name. Swap the order of the conditions: first test
for the same object and only of the objects differ, test for
the same same.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Replace the UUID reference of struct dive by a pointer to dive_site.
This commit is rather large in lines, but nevertheless quite simple
since most of the UUID->pointer work was done in previous commits.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This is another case of a weird pattern where an object would
connect it's own signal to the slot of a different object.
There seems to be no reason why the former couldn't simply
call the latter.
Remove the [start|stop]FilterDiveSite signals of LocationInformationWidget
and call the corresponding functions of MultiFilterSortModel directly.
While doing so, replace the UUID argument by a pointer-to-divesite.
It will be converted anyway right at the beginning of the function.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Replace UUIDs from LocationInformationModel and fix the fallout.
Notably, replace the UUID "column" by a DIVESITE "column".
Getting pointers through Qt's QVariant is horrible, we'll have
to think about a better solution.
RECENTLY_ADDED_DIVESITE now defines to a special pointer to
struct dive_site (defined as ~0).
This fixes an interesting logic bug:
The old code checked the uuid of the LocationInformationModel (currUuid)
for the value "1", which corresponded to RECENTLY_ADDED_DIVESITE.
If equal, currType would be set to NEW_DIVE_SITE. Later, _currType_
was compared against _RECENTLY_ADDED_DIVESITE_. This would only work
because NEW_DIVE_SITE and RECENTLY_ADDED_DIVESITE both were defined
as 1.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Access to dive-sites in the LocationInformationModel was via UUID.
Replace this by a direct access to the struct dive_site pointer.
Accordingly, rename the UUID_ROLE to DIVESITE_ROLE.
This is a small step in replacing dive-site UUIDs by pointers
throughout the code base.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of passing a uuid, pass a pointer to the dive site.
This is small step in an effort to remove uuids.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This accessor was never used. This is a small step in splitting
the DiveTripModel in two (list & tree), which means that the
layout is moved up to the view.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Whenever the filter changes, simply walk the filtered dive list and ensure
that we have the correct count for dives that match this filter.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
The regular expression based generic filtering made things very slow on a cell
phone or other, slower device. With this the results seem more reasonable.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
[Dirk Hohndel: this is the starting point of my following commits, I decided to
leave it in place to give Jan credit for the work he did on
figuring out some of the plumbing needed to get things to work]
Signed-off-by: Jan Mulder <jlmulder@xs4all.nl>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
After invalidating the filter, the dive-selection was modified to
ensure that at least one dive is selected. This was done in the
filter code, but it seems preferrable to do this in the dive-list
code, which has direct access to the selection-model.
Therefore, move the code from MultiFilterSortModel to DiveListView.
While doing so, split the code in DiveListView into more functions to:
1) Get the index of the first dive (if any).
2) Select the first dive (if any).
This allows a distinct size reduction of conditional compilation
in MultiFilterSortModel (accesses to MainWindow are not possible
in mobile code).
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
On change of the filter, the headers of non-extended trips were not
updated. Therefore, on filter-finish-event loop over all trips
in DiveTripModel and signal data-changed.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of having people treat latitude and longitude as separate
things, just add a 'location_t' data structure that contains both.
Almost all cases want to always act on them together.
This is really just prep-work for adding a few more locations that we
track: I want to add a entry/exit location to each dive (independent of
the dive site) because of how the Garmin Descent gives us the
information (and hopefully, some day, other dive computers too).
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
The DiveItem and TripItem classes were wrappers around dive * and
dive_trip * used to extract tabular data. With the rework of
DiveTripModel they lost all their state besides the pointer itself.
The usage was:
DiveItem item(d);
item.data(...);
This can now be simplified to the much more idiomatic
diveData(d, ...);
and analoguously for TripItem.
While adapting the data() function to be part of DiveTripModel, change
the
QVariant ret
switch(...) {
...
case ...:
ret = ...;
break;
...
}
return ret;
style to
switch(...) {
...
case ...:
return ...;
}
Not only is this shorter and easier to reason about, it generally also
improves the generated code. The compiler can directly construct the
return value in the buffer provided by the caller. Though modern
compilers start to be very good at avoiding unnecessary copies.
In total this cleanup results in a net-reduction of 190 lines of code.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Comits f427226b3b and 43c3885249 of the undo series introduced 2 calls
of autogroup_dives() without checking the autogroup global boolean.
This is a bug. An import from DC (for example) then triggers an
autogrouping, the divelist is autogrouped, and the UI button
is off.
This commit solves this. I've chosen for a guard in the autogroup_dives()
that now is a no-op when called when the user did not select autogrouping.
In additon, simplified the other calls to this function, as we do
not need to check before calling any more.
Signed-off-by: Jan Mulder <jlmulder@xs4all.nl>
Instead of the weirdly named "information" and the inconsistent
"dive_list" use the logical "mainTab" and the camel-cased
"diveList", respectively.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The keeps track of different sub widgets needed by other parts
of the code, notably:
MainTab
PlannerDetails
PlannerSettingsWidget
ProfileWidget2
DivePlannerWidget
DiveListView
Access to these widgets was provided with accessor functions.
Now these functions were very weird: instead of simply returning
pointers that were stored in the class, they accessed a data
structure which describes the different application states.
But this data structure was "duck-typed", so there was an
implicit agreement at which position the pointers to the
widgets were put inside. The widgets were then down-cast by
the accessor functions. This might make sense if the individual
widgets could for some reason be replaced by other widgets
[dynamic plugins?], but even then it would be strange, as one
would expect to get a pointer to some base class.
Therefore, directly store the properly typed pointers to the
widgets and simply remove the accessor functions. Why bother?
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
SsrfFilterSortProxyModel was a thin wrapper around QFilterSortProxyModel,
which was intended as a convenience class to avoid deriving from the
latter. The filter and sort functions were replaced by simple function
pointers.
Unfortunately, by using function-pointers, the whole thing was rather
weak as these functions do not have state. The last user was removed
in ac8dcd7f65b78958587ba025280ed4c529b0b519. Therefore, remove the
whole class.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The location information shows a list of dive sites at the
same location as the edited dive site. This was done by passing
a function to an "SsrfSortFilterProxyModel". Unfortunately,
the latter does only support function pointers without state
and therefore had to access the global "displayed_dive_site"
object.
Replace the SsrfSortFilterProxyModel by a proper subclass of
QSortFilterProxyModel that contains information on the position
and id of the currently edited dive site.
Update the filter model if the location of the dive site changes.
This introduces a behavioral change: editing the GPS location
will lead to an updated list.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Planned dives were still added by directly calling core code.
This could confuse the undo-machinery, leading to crashes.
Instead, use the proper undo-command. The problem is that as
opposed to the other AddDive-commands, planned dives may
belong to a trip. Thus, the interface to the AddDive command
was changed to respect the divetrip field. Make sure that
the other callers reset that field (actually, it should never
be set). Add a comment describing the perhaps surprising
interface (the passed-in dive, usually displayed dive, is
reset).
Moreover, a dive cloned in the planner is not assigned a
new number. Thus, add an argument to the AddDive-command,
which expresses whether a new number should be generated
for the to-be-added dive.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Currently, the filter is recalculated if a filter-entry is changed.
This also happens if the counts of a filter-entry changes. This
is to be avoided, as it causes unnecessary churn.
Therefore, send the proper role with the dataChanged() signal
and add a new slot, which invalidates only if a field with the
Qt::CheckStateRole is changed.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of reloading all the filter, only increment / decrement the
count of the entries of added / removed dives.
Originally, this was planned to be done via the signals from the
divelist, but it turned out that this was suboptimal, because
if the filter decides that the new item is selected, this has to
be done *before* adding the dive. Otherwise, it wouldn't be shown.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Ultimately, we want to use a single dive-list and not replicate
it in the Qt-model code. To this goal, let's start with using
the same sort function.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The current code cheats when it comes to move dives inside
a trip or move dives between trips: Instead of using the
*MoveRows() functionality, the dives are removed from and
re-added to the respective trips. This loses the selection.
Therefore, remember which of the moved dives are selected
and select them manually after they are re-added.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The command-objects select a current item, but this selection
was not propagated to the front-end. The current item is the
base for keyboard-navigation through the dive-list and therefore
should be set correctly.
It took some experimentation to get the flags right:
QItemSelectionModel::Current
Hopefully, these are the correct flags across all supported
Qt versions!
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
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>
Don't delesect dives, when unregistering them from the backend.
If a previously selected dive is added, select it in the dive-list.
For this purpose introduce a SELECTED_ROLE to query the DiveTripModel
for selected dives.
Unfortunately, when adding multiple selected dives, current_dive_changed
is called for each of them, making this very slow. This will have
to be fixed in subsequent commits.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Previously, each dive-list modifying function would lead to a
full model reset. Instead, implement proper Qt-model semantics
using beginInsertRows()/endInsertRows(), beginRemoveRows()/
endRemoveRows(), dataChange().
To do so, a DiveListNotifer singleton is generatated, which
broadcasts all changes to the dive-list. Signals are sent by
the commands and received by the DiveTripModel. Signals are
batched by dive-trip. This seems to be an adequate compromise
for the two kinds of list-views (tree and list). In the common
usecase mostly dives of a single trip are affected.
Thus, batching of dives is performed in two positions:
- At command-level to batch by trip
- In DiveTripModel to feed batches of contiguous elements
to Qt's begin*/end*-functions.
This is conceptually simple, but rather complex code. To avoid
repetition of complex loops, the batching is implemented in
templated-functions, which are passed lambda-functions, which
are called for each batch.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
AddDivesToTrip, CreateTrip, AutogroupDives, RemoveAutogenTrips
and MergeTrips basically all did the same thing as RemoveDivesFromTrip,
which was already implemented. Thus, factor our the common functionality
and hook it up to make all these functions undo-able.
Don't do the autogroup-call everytime the dive-list is rebuilt
(that would create innumberable undo-actions), but only on dive-load /
import or if expressly asked by the user [by switching the autogroup
flag].
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The dive list is fed data by means of a sorted "DiveTripModel".
There are two modes: list and tree. This was implemented rather
elegantly with a general "TreeModel", which can represent trees
of arbitrary depths.
Nevertheless, we have at most two levels and on the second level
only dives can reside. Implementing proper model-semantics
(insert, delete, move) will be quite a challenge and implementing
it under the umbrella of a very general model will not make it
easier.
Therefore, for now, hardcode the model:
At the top-level there are items which may either be a trip
(can contain multiple dives) or a dive (contains exactly one dive).
Thus, we can completely de-virutalize the DiveItem and TripItem
classes, which are now trivial wrappers around dive * and dive_trip *.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The undo-system now guarantees that pointers to dives are stable
throughout their lifetime. Therefore, replace the unique index by
pointers. This is a small performance improvement, but much more
importantly, it will make it more natural to transport a pointer
to the dive inside QModelIndex's private pointer.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
For increased maintainability, use the same columns, roles and
the same accessor function for both dive-site models.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The LocationInformationModel added two dummy sites to the front
of the list (add new dive site). This was never used - desktop
uses its own model, mobile only extracts the list of dive site
names with a custom function. Remove this functionality.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Editing of dive sites does not work via this model and the function
was broken anyway (it didn't subtract 2 from the index).
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
If only selected dives were exported into HTML, the statistics would
nevertheless cover all dives. A counter-intuitive behavior. Fix by
adding a selected_only flag to calculate_stats_summary().
Reported-by: Jan Mulder <jlmulder@xs4all.nl>
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Statistics were calculated into global variables every time the
current dive was changed.
Calculate statistics only when needed and into a structure
provided by the caller.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
process_imported_dives() is more efficient for downloaded than for
imported (from a file) dives, because it checks only the divecomputer
of the first dive.
This condition is checked via the "downloaded" flag of the first
dive. Instead, pass an argument to process_imported_dives().
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Dives are now in all cases imported via distinct dive_tables.
Therefore the "preexisting" marker is useless. Remove.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Dives were directly imported into the global dive table and then
merged in process_imported_dives(). Make this interface more flexible,
by passing an independent dive table.
The dive table of the to-be-imported dives will be sorted and merged.
Then each dive is inserted in a one-by-one manner to into the global
dive table.
This actually introduces (at least) two functional changes:
1) If a new dive spans two old dives, it will only be merged to the
first dive. But this seems like a pathological case, which is of
dubious value anyway.
2) Dives unrelated to the import will not be merged. The old code
would happily merge dives that were not even close to the
newly imported dives. A surprising behavior.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
