Dirk,

On 15.03.2016, at 23:30, Dirk Hohndel <[email protected]> wrote:

imagedownloader.cpp doesn't exist in v4.5-branch... that code is still in
the divepicturewidget there... I would appreciate if you could create a
seperate commit against the v4.5-branch, just so I don't mess things up
when trying to merge things...

Any chance you could do that?

Here is your patch.

From 0b513871ec15ac11b4c91a770f2a916efab83930 Mon Sep 17 00:00:00 2001
From: "Robert C. Helling" <[email protected]>
Date: Wed, 16 Mar 2016 00:29:28 +0100
Subject: [PATCH] Copy picture struct for worker thread
To: [email protected]

This copies the picture struct when delegating image handling
to a worker thread to prevent a crashe when main thread
frees the picture upon selecting a different dive.

Signed-off-by: Robert C. Helling <[email protected]>
---
 dive.c                      | 16 +++++++++++++++-
 dive.h                      |  2 ++
 qt-ui/divepicturewidget.cpp | 13 ++++++++++---
 qt-ui/divepicturewidget.h   |  1 +
 qthelper.cpp                | 12 ++++++++----
 5 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/dive.c b/dive.c
index 83ec161..8dc4322 100644
--- a/dive.c
+++ b/dive.c
@@ -2357,6 +2357,20 @@ static void free_pic(struct picture *picture)
        }
 }
 
+// When handling pictures in different threads, we need to copy them so we 
don't
+// run into problems when the main thread frees the picture.
+
+struct picture *clone_picture(struct picture *src)
+{
+       struct picture *dst;
+
+       dst = alloc_picture();
+       copy_pl(src, dst);
+       return dst;
+}
+
+
+
 static int same_sample(struct sample *a, struct sample *b)
 {
        if (a->time.seconds != b->time.seconds)
@@ -3387,7 +3401,7 @@ void dive_set_geodata_from_picture(struct dive *dive, 
struct picture *picture)
        }
 }
 
-static void picture_free(struct picture *picture)
+void picture_free(struct picture *picture)
 {
        if (!picture)
                return;
diff --git a/dive.h b/dive.h
index cf9b19f..0eec8fc 100644
--- a/dive.h
+++ b/dive.h
@@ -376,6 +376,7 @@ struct picture {
        for (struct picture *picture = (_divestruct).picture_list; picture; 
picture = picture->next)
 
 extern struct picture *alloc_picture();
+extern struct picture *clone_picture(struct picture *src);
 extern bool dive_check_picture_time(struct dive *d, int shift_time, 
timestamp_t timestamp);
 extern void dive_create_picture(struct dive *d, char *filename, int 
shift_time, bool match_all);
 extern void dive_add_picture(struct dive *d, struct picture *newpic);
@@ -385,6 +386,7 @@ extern bool picture_check_valid(char *filename, int 
shift_time);
 extern void picture_load_exif_data(struct picture *p);
 extern timestamp_t picture_get_timestamp(char *filename);
 extern void dive_set_geodata_from_picture(struct dive *d, struct picture *pic);
+extern void picture_free(struct picture *picture);
 
 extern int explicit_first_cylinder(struct dive *dive, struct divecomputer *dc);
 extern int get_depth_at_time(struct divecomputer *dc, int time);
diff --git a/qt-ui/divepicturewidget.cpp b/qt-ui/divepicturewidget.cpp
index bed3d3b..5e37110 100644
--- a/qt-ui/divepicturewidget.cpp
+++ b/qt-ui/divepicturewidget.cpp
@@ -23,16 +23,17 @@ void loadPicture(struct picture *picture)
 
 SHashedImage::SHashedImage(struct picture *picture) : QImage()
 {
-       QUrl url = QUrl::fromUserInput(QString(picture->filename));
+       QUrl url = 
QUrl::fromUserInput(localFilePath(QString(picture->filename)));
+
        if(url.isLocalFile())
                load(url.toLocalFile());
        if (isNull()) {
                // Hash lookup.
                load(fileFromHash(picture->hash));
                if (!isNull()) {
-                       QtConcurrent::run(updateHash, picture);
+                       QtConcurrent::run(updateHash, clone_picture(picture));
                } else {
-                       QtConcurrent::run(loadPicture, picture);
+                       QtConcurrent::run(loadPicture, clone_picture(picture));
                }
        } else {
                QByteArray hash = hashFile(url.toLocalFile());
@@ -46,6 +47,11 @@ ImageDownloader::ImageDownloader(struct picture *pic)
        picture = pic;
 }
 
+ImageDownloader::~ImageDownloader()
+{
+       picture_free(picture);
+}
+
 void ImageDownloader::load(){
        QUrl url = QUrl::fromUserInput(QString(picture->filename));
        if (url.isValid()) {
@@ -64,6 +70,7 @@ void ImageDownloader::load(){
 void ImageDownloader::saveImage(QNetworkReply *reply)
 {
        QByteArray imageData = reply->readAll();
+       qDebug() << "downloaded ";
        QImage image = QImage();
        image.loadFromData(imageData);
        if (image.isNull())
diff --git a/qt-ui/divepicturewidget.h b/qt-ui/divepicturewidget.h
index 54f5bb8..ff1240e 100644
--- a/qt-ui/divepicturewidget.h
+++ b/qt-ui/divepicturewidget.h
@@ -11,6 +11,7 @@ class ImageDownloader : public QObject {
        Q_OBJECT;
 public:
        ImageDownloader(struct picture *picture);
+       ~ImageDownloader();
        void load();
 private:
        struct picture *picture;
diff --git a/qthelper.cpp b/qthelper.cpp
index 7e4d64e..c78454b 100644
--- a/qthelper.cpp
+++ b/qthelper.cpp
@@ -1179,9 +1179,11 @@ void learnHash(struct picture *picture, QByteArray hash)
 
 QString localFilePath(const QString originalFilename)
 {
-       if (hashOf.contains(originalFilename) && 
localFilenameOf.contains(hashOf[originalFilename]))
-               return localFilenameOf[hashOf[originalFilename]];
-       else
+       if (hashOf.contains(originalFilename))
+               if (localFilenameOf.contains(hashOf[originalFilename]))
+                       if (localFilenameOf[hashOf[originalFilename]] != "")
+                               return 
localFilenameOf[hashOf[originalFilename]];
+
                return originalFilename;
 }
 
@@ -1197,6 +1199,7 @@ void updateHash(struct picture *picture) {
        char *old = picture->hash;
        picture->hash = strdup(hash.toHex());
        free(old);
+       picture_free(picture);
 }
 
 void hashPicture(struct picture *picture)
@@ -1206,13 +1209,14 @@ void hashPicture(struct picture *picture)
        if (!same_string(picture->hash, "") && !same_string(picture->hash, 
oldHash))
                mark_divelist_changed((true));
        free(oldHash);
+       picture_free(picture);
 }
 
 extern "C" void cache_picture(struct picture *picture)
 {
        QString filename = picture->filename;
        if (!hashOf.contains(filename))
-               QtConcurrent::run(hashPicture, picture);
+               QtConcurrent::run(hashPicture, clone_picture(picture));
 }
 
 void learnImages(const QDir dir, int max_recursions, bool recursed)
-- 
2.5.4 (Apple Git-61)


I just realized (upon testing this patch), that in your cherry pick you did not include  

commit 2075038de1d3a181eb964020c635099ebf52b4ed
Author: Robert C. Helling <helling@atdotde.de>
Date:   Mon Nov 9 16:48:12 2015 +0100

    Store Thumbnails with image hashes

If you still have the chance, that one improves loading speed significantly.

Best
Robert

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to