Commit graph

27 commits

Author SHA1 Message Date
Berthold Stoeger
afe8843509 Dive pictures: don't attempt reading the same image twice
If loading of an image failed, we tried to see if we find a
canonical filename in the cache. There's no point in rereading
the picture if the canonical and the original filename are
the same.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-16 15:20:30 +03:00
Berthold Stoeger
c3c2c2f606 Dive pictures: Don't enter infinity loop on invalid pictures
The recently committed refactoring of the dive-picture code introduced
a severe bug:

If an image couldn't be loaded from disk owing to an invalid file, the
filename was interpreted as an url and loaded in the background. This
succeeded, because the file actually exists. After download, the file
would then still be invalid and the whole thing restarted, leading to
an infinity loop.

To fix this, do two things:

1) Don't even try to download local files.
2) If interpreting a downloaded file fails, don't try the downloading
business again.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-16 02:01:26 +03:00
Berthold Stoeger
d33e3b22fc Dive pictures: refactor image downloader
Instead of generating one ImageDownloader object per image to be
downloaded and running every image download in a separate worker
thread, use one global ImageDownloader object owned by the UI thread.

The images are downloaded using event based IO (as probably was the
intention of the QNetworkManager class).

User-visible change: after download from the internet, the thumbnail
is shown without having to change dives.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-13 13:52:35 -07:00
Berthold Stoeger
19fa2966ef Dive pictures: Import camera icon
Import a camera icon from the KDE breeze theme, which is licensed
under the LGPL. Use this icon to display not-yet-loaded images
in the photos tab and the profile.

Source: https://github.com/KDE/breeze-icons

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-13 13:52:35 -07:00
Berthold Stoeger
b450c155fd Dive pictures: Move thumbnail-size to Thumbnailer class
The size of the to-be-created thumbnails was passed from DivePictureModel
to Thumbnailer. This became more and more bothersome, because the size
had to be stored with the request. Calling from Thumbnailer into
DivePictureModel was not an option, since this is not linked to all tests.
Therefore, move these functions to the Thumbnailer class.

Since the maximum thumbnail size is now known to the thumbnailer, the
dummy and failure images can be precalculated, which makes switching
between dives faster.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-13 13:52:35 -07:00
Berthold Stoeger
3967b1fd4d Dive pictures: Introduce thumbnailer class
Create a new class, which performs all thumbnailing code.
This is mostly code reshuffling. Thumbnails are extracted
either from a cache or thumbnail calculation is started in
a worker thread.

Since getHashedImage() is called from a worker thread it
makes no sense to call subfunctions in yet another worker
thread. Remove these calls.

In contrast to the previous code, on error the background
thread produces a failure image, but it is not yet shown.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-13 13:52:35 -07:00
Berthold Stoeger
f60343eebb Dive pictures: replace picture struct by QString
In imagedownloader.cpp the only thing we need from the picture struct
is the filename. Therefore, use QStrings instead of the picture struct.
This simplifies memory management.

Remove the clone_picture() function, which is not needed anymore.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-13 13:52:35 -07:00
Berthold Stoeger
5d372cfda3 Dive pictures: turn SHashedImage class into getHashedImage() function
SHashedImage was a subclass of QImage, which fetched the image according
to the filename hashes. Turn this into a function, as this is much more
idiomatic and flexible.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-05-13 13:52:35 -07:00
Berthold Stoeger
d9df8c3f47 Debug: Error reporting for SHashedImage loading
Overwrite QImage::load() in SHashedImage so that we can perform better
error reporting.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-04-01 15:59:46 +03:00
Berthold Stoeger
376b737891 Debug: sprinkle debug messages in thumbnailing code
To ease trouble-shooting of the picture thumbnailer add a number
of debug- and info-messages.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
2018-04-01 15:59:46 +03:00
Berthold Stoeger
bdc470a80e Cleanup: Remove hash field from picture-structure
The hash field in the picture-structure was in principle non-operational.
It was set on loading, but never actually changed. The authoritative
hash comes from the filename->hash map.

Therefore, make this explicit by removing the hash field from the
picture structure.

Instead of filling the picture structure on loading, add the
hash directly to the filename->hash map. This is done in the
register_hash() function, which does not overwrite old entries.
I.e. the local hash has priority over the save-file. This
policy might be refined in the future.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
f5eafe2a71 Cleanup: Fold add_hash() call into learnHash() function
learnHash() was always called in conjunction with add_hash(). The
pattern was that a local filename and a hash were connected in
the hash-to-filename and the filename-to-hash maps. Then, the
original picture-filename or url were registered in the filename-to-hash
map.

This commit changes learnHash() to take three parameters (original-filename,
local-filename and hash) and do all of the above. The new code is
simpler because no dummy picture struct has to be generated in
DiveListView::loadImageFromURL().

The tests were extended to check for all hash<->filename associations.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
a0d02bacf3 Cleanup: remove updateHash() function
updateHash() and hashPicture() did the same thing, with the exception
that hashPicture() marked the dive list as changed if a hash changed.
This seems like a good idea in any case, therefore always use
hashPicture().

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
eaff0ddee7 core/imagedownloader.cpp: Use signal to exit eventloop
Don't do busy-waiting.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
907c07ce50 core/imagedownloader.cpp: Use implicit default constructor of QImage
No point in move-constructing from a different default constructed
QImage.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
117f4a0d7d core/imagedownloader.cpp: remove recursion
If loading from hash failed in the saveImage() slot(!) it would
recurse into loadFromUrl(), which would generate a new network
reply. Very scary and a (small) wonder that it worked.

Let's try to make this all more explicit.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
54d56a74aa core/imagedownloader.cpp: Make the manager object local to load()
The QNetworkAccessManager is only used in the load() function. No
point in it being a subobject.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
2803b7d385 core/imagedownloader.cpp: Simplify deletion of objects
1) Destroying the QNetworkManager seems like a bug: this was a
subobject of ImageDonwloader. It's mysterious how this didn't
crash.

2) Instead of calling deleteLater() on the reply object, simply
delete it after completion.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
c1cd2e0f02 core/imagedownloader.cpp: Fix memory leak
A cloned picture struct would not be freed if the filename was already
in the queued-for-download set.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-03-05 18:04:57 +02:00
Berthold Stoeger
fe133e2cca Cleanup: unserialize loading of pictures
The images to load are kept in a set to avoid multiple loading of
the same picture. The set was protected with a mutex.

Problem: the mutex was not released before loading the picture,
effectively serializing the loading of pictures.

Therefore unlock the mutex once the set is updated.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-02-07 09:44:43 +01:00
Berthold Stoeger
9986ce25d7 Cleanup: Make local helpers and variables of static linkage
In core/imagedownloader.cpp the helpers cloudImageUrl() and loadPicture()
are made of static linkage.

The global variables queuedPictures and pictureQueueMutex were moved
into the loadPicture() function, because they are used only there.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
2018-02-07 09:44:43 +01:00
Jan Mulder
13305e7745 cleanup: Uninitialized scalar field
CID 208303

Signed-off-by: Jan Mulder <jlmulder@xs4all.nl>
2017-12-29 09:14:20 -08:00
Dirk Hohndel
b368ecd5aa Add SPDX header to remaining core files
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
2017-04-29 13:32:55 -07:00
Robert C. Helling
10b8bda662 Fix picture hashing logic for pictures on the web.
Signed-off-by: Robert C. Helling <helling@atdotde.de>
2016-12-30 19:43:00 +01:00
Robert C. Helling
c2c0cb48d2 Only attempt to download each picture once
Otherwise we keep downloading the same image multiple times instead
of new images.

Signed-off-by: Robert C. Helling <helling@atdotde.de>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
2016-04-30 16:18:44 -07:00
Robert C. Helling
2e6588dc0e Protect access to image hash dictionaries with lock
Otherwise we step on our own feet when downloading several images,
like after import from divelogs.de with many linked images.

Signed-off-by: Robert C. Helling <helling@atdotde.de>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
2016-04-29 09:08:20 -07:00
Dirk Hohndel
7be962bfc2 Move subsurface-core to core and qt-mobile to mobile-widgets
Having subsurface-core as a directory name really messes with
autocomplete and is obviously redundant. Simmilarly, qt-mobile caused an
autocomplete conflict and also was inconsistent with the desktop-widget
name for the directory containing the "other" UI.

And while cleaning up the resulting change in the path name for include
files, I decided to clean up those even more to make them consistent
overall.

This could have been handled in more commits, but since this requires a
make clean before the build, it seemed more sensible to do it all in one.

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
2016-04-04 22:33:58 -07:00
Renamed from subsurface-core/imagedownloader.cpp (Browse further)