DiveImportedModel::recordDives() called add_imported_dives(). But that
actually consumes the dive and dive-site tables. Which in turn will
lead to an inconsistent model.
Properly reset the model by using the consumeTables() function.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The DiveImportedModel and DownloadThread used the same table
of dives and dive sites. This made it very hard to keep the
model consistent: Every modification of the download thread
would make the model inconsistent and could lead to memory
corruption owing to dangling pointers.
Therefore, keep a copy in the model. When updating the model,
use move-semantics, i.e. move the data and reset the tables
of the thread to zero elements.
Since the DiveImportedModel and the DownloadThread are very
tightly integrated, remove the accessor-functions of the
dive and dive-site tables. They fulfilled no purpose
whatsoever as they gave the same access-rights as a public
field.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The plan is to make the model the authoritative source of
the imported dives. Therefore, access the number of
downloaded dives from there.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When importing dives, consume the tables from DiveImportedModel
and not the DownloadThread. This appears more logical and avoids
an inconsistent state of the DiveImportedModel: On import the
tables would be reset, but the DiveImportedModel wasn't
informed of that.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In DiveImportedModel::deleteDeselected(), unselected dives were
deleted from the dive-table. But this left the model in an
inconsistent state and the frontend was not informed of the
missing dives.
Fix this by invoking the appropriate beginRemoveRows()/
endRemoveRows() pairs. Move the functionality into its
own function so that it can be reused by the desktop version.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Currently, desktop and mobile are accessing the DownloadThread
and the DiveImportedModel concurrently. This makes a big data
flow mess. To achieve a more hierarchical data flow, start
by making the DownloadThread a subobject of DiveImportedModel.
Start the download by calling a function in DiveImportedModel.
Route the finished signal through DiveImportedModel. Thus,
the model can reload itself with the new data.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Owing to apparent QML breakage, a model-reset leads to the DiveDetail
page being reloaded for every dive in the list(!). Therefore, add
rows instead.
This leads to extremely subtle code, as it is now imperative that
the model has been properly cleared beforehand. Nevertheless, for
now we have to do this to fix a severe performance regression.
Fixes#2295
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Clearing the dive data directly in the core leaves us with an
inconsistent model. Therefore, clear via the model.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of using the GpsLocation singleton in GpsListModel::update()
to extract the gpsTrackers, pass the gpsTrackers as function argument.
The caller has direct access to the GpsLocation object anyway and this
make things less entangled.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The way we handle singletons in QML, QML insists on allocating the
objects. This leads to a very idiosyncratic way of handling
singletons: The global instance pointer is set in the constructor.
Unify all these by implementing a "SillySingleton" template. All
of the weird singleton-classes can derive from this template and
don't have to bother with reimplementing the instance() function
with all the safety-checks, etc.
This serves firstly as documentation but also improves debugging
as we will now see wanted and unwanted creation and destruction
of these weird singletons.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of converting the section-heading string to a trip-pointer
in QML and pass that to the tripTitle() and tripShortDate()
functions, pass the string and convert in C++ code.
Hopefully, this makes the code more robust.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The section heading in the QtQuick ListView has to be a string.
Therefore, we passed a pointer formatted using hexadecimal notation.
Later, that was converted back without being checked.
A very scary proposition, so let's pass unique integer trip-id instead.
This means that on converting back we have to scan the trip table,
but that is a very minor cost comsidering to the gained robustness.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In MultiFilterSortModel::startFilterDiveSites(), the setting of the
dive sites to be filtered is done later in the code. Therefore,
remove the assignment in the first line of the function. Under
some circumstances, this would prevent a needed map reload!
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of looping over all dives and search the dive with the given
id, let the source model determine the index and map that. Thus,
we do only one mapping and don't generate a ton of DiveObjectHelpers.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Accesses were via DiveObjectHelpers. Provide a direct access to
struct dive *. Use this for the filter - there is no point in
mass generating DiveHelperObjects in the filter code.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
In DiveListModel::data() a DiveObjectHelper was created for any
data-access. Create it only when a DiveObjectHelper is actually
returned.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since DiveListModel does not keep its own list of dives anymore,
insertDive() doesn't use the DiveObjectHelper argument. Remove it.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The clear()/addAllDives() pair was bogus as the former didn't
clear the model (this is not possible anymore - the model
represents the core dive list) and the latter readded all
dives again.
Replace this by a reload() function.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Instead of keeping track of a list of DiveObjectHelpers, generate
them on-the-fly in DiveListModel. Thus, there is less danger of
model and core getting out of sync. On the flip-side, now the
DiveListModel and the DiveListSortModel might get out of sync.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
DiveObjectHelper is a tiny wrapper around dive * to allow access
to dive data from QML and grantlee. It doesn't have to be a
full-fledged QObject with support for signals, etc. Therefore,
turn it into a Q_GADGET based object. This allows us passing the
object around as object, not as pointer to DiveObjectHelper.
This makes memory-management distinctly easier.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We don't want to generate a DiveObjectHelper numerous times for
every item in the dive list. Therefore, return this data directly
from the model. In this case, don't remove from DiveObjectHelper,
as these data might be used by grantlee templates.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We don't want to generate a DiveObjectHelper numerous times for
every item in the dive list. Therefore, return this datum directly
from the model. In this case, don't remove from DiveObjectHelper,
as this datum might be used by grantlee templates.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We don't want to generate a DiveObjectHelper numerous times for
every item in the dive list. Therefore, return this datum directly
from the model. In this case, don't remove from DiveObjectHelper,
as this datum might be used by grantlee templates.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We don't want to generate a DiveObjectHelper numerous times for
every item in the dive list. Therefore, return this datum directly
from the model. In this case, don't remove from DiveObjectHelper,
as this datum might be used by grantlee templates.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We don't want to generate a DiveObjectHelper numerous times for
every item in the dive list. Therefore, return this data directly
from the model. In this case, don't remove from DiveObjectHelper,
as these data might be used by grantlee templates.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We don't want to generate a DiveObjectHelper numerous times for
every item in the dive list. Therefore, return this datum directly
from the model.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
The canonical way of displaying lists in Qt is via models.
Thus, return the tripId directly from the DiveListModel instead
of going indirectly via a DiveObjectHelper. In the future, this
will allow us to make the DiveObjectHelper value-based, as it
is not generated numerous times for every list item.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
These properties are not needed anymore, because the full text search
was decoupled from the DiveObjectHelper.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
1) The full text search was looping over the DiveListModel when
it could simply loop over the core model. Do that instead.
2) Don't generate a DiveObjectHelper to do a full text search.
Currently this is harmless as the DiveObjectHelper is only
a disguised "dive *". But from a conceptual point of view,
it represents the full representation of a dive and we don't
want to generate that in a tight loop.
This will help in
1) Making the DiveObjectHelper a non-reference object.
2) Moving fulltext search to the core and thus making it available
to desktop and more performant.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
There used to be code to remove the old planner notes when replanning
a dive. It used a global variable and seemed rather brittle. Moreover,
the place that set the global variable was inadvertently removed.
Therefore has been effectively dead code.
Reimplement the functionality, but be more robust by considering
that the deco-type may have changed: Split the translated disclaimer
string in two parts, before and after the "%s" place-holder.
Search for these two parts. Remove the disclaimer and everything
after the disclaimer.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
"Roles" is a C-style enum (i.e. not C++-style enum class). Since that
means that the names spill into the outer namespace, the names
themselves are prefixed with "Role". Nevertheless the code qualified
the names with "Roles::". This is redundant and unnecessary.
Remove this redundancy to show that we understand how the language
works.
Note: we could also transform the enum into an enum class and remove
the "Role" prefix from the names. That would arguably be "cleaner",
but then the enum doesn't auto-convert to/from int, but Qt uses int
to pass the roles to functions. So let's go the simple way that
avoids casting.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since this is no longer a Q_METATYPE, nobody will try to
default construct this object. Remove the default constructor
and guarantee that there will be no null divesite.
Of course, the lack of default constructor means that the
default argument to the "selected" member variable should
be removed.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Let's face it: this is a value type. No point in having Java-style
getters and setters. Replace by plain old and boring member variables.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Nobody was listening for that signal. Remove it. This, quite
obviously, makse the setCoordinateNoEmit() function redundant.
Merge with setCoordinateNoEmit().
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
We never dish out an object of this type to QML. It is unclear how
Q_PROPERTIEs could be of any use.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Recently we changed the MapLocationModel-items to store whether
they are selected. Thus, we can directly export an isSelected
flag instead of calling a function taking a dive-site argument.
1) This makes the QML easier to read.
2) This avoids passing pointers through QML which has caused
us lots of pain.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
To connect a model to QML, one is supposed to provide a
QHash<int, QByteArray> MapLocationModel::roleNames()
function that returns a role -> attribute-name hash.
That was realized by filling the hash in the constructor,
storing it as a member variable, using static strings that
were declared in the class-definition and defined in the
translation unit.
Adding a new role was a pain and the whole thing was totally
pointless as the attribute names were used nowhere else and
the roleNames() function is called only once.
Simply do, what we do everywhere else: initialize the hash
in the roleNames() function and use normal string literals.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When editing the dive site, for certain fields a divesChanged signal
was emitted so that the dive-list can be updated.
Arguably it is wrong to decide which fields are relevant to the
dive list in the undo-command code. Therefore, let the list
catch the dive-site-edited signal and decide itself.
But the actual reason for this commit is that if the dive-site
field of a dive changes, we might have to reload the dive-location-model
because suddenly a new dive site appears. Now if this is done
in QML context on some Qt version (notably 5.9) we get crashes
later on. But that can happen if the user moves a flag. So in that
case only send a diveSiteChanged signal.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since changing the highlighting to use the selected dive, dive
sites with no dive were never highlighted in dive site mode.
Obviously, because there was no dive to be selected.
Therefore special-case all dive-site selection code to recognize
when we are in dive site mode.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since selection change doesn't to a full map reload, we have to
reload the map on filter changes, since the shown dive sites change.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Since not fully reloading the map on selection change,
the selected sites were not moved to the top. Not calculating
the z-value in QML, but making it a simple model property
helps.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
When clicking a dive site on the map, the QML code would set
the selected dive site, but then all dives of dive sites in
the vicinity were set. But still only the clicked-on dive site
was shown.
Therefore, don't set the list of selected dive sites in QML,
but later in DiveListView::selectDives(), where we know all
the dives that were selected.
This, again, gives nasty entanglement of diverse widgets and
models.
Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
