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>

7 files changed, 29 insertions, 58 deletions

diff --git a/core/dive.c b/core/dive.c
index 3d7c0f486..00c232215 100644
--- a/core/dive.c
+++ b/core/dive.c
@@ -3843,18 +3843,6 @@ void picture_free(struct picture *picture)
 	free(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;
-}
-
 // Return true if picture was found and deleted
 bool dive_remove_picture(struct dive *d, const char *filename)
 {
diff --git a/core/dive.h b/core/dive.h
index 854ec82ba..3d7eb5979 100644
--- a/core/dive.h
+++ b/core/dive.h
@@ -429,7 +429,6 @@ 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, const char *filename, int shift_time, bool match_all);
 extern void dive_add_picture(struct dive *d, struct picture *newpic);
diff --git a/core/imagedownloader.cpp b/core/imagedownloader.cpp
index 772365d38..a1f98fc9c 100644
--- a/core/imagedownloader.cpp
+++ b/core/imagedownloader.cpp
@@ -16,23 +16,17 @@ static QUrl cloudImageURL(const char *filename)
 	return QUrl::fromUserInput(QString("https://cloud.subsurface-divelog.org/images/").append(hash));
 }
 
-ImageDownloader::ImageDownloader(struct picture *pic)
+ImageDownloader::ImageDownloader(const QString &filename_in) : filename(filename_in)
 {
-	picture = pic;
-}
-
-ImageDownloader::~ImageDownloader()
-{
-	picture_free(picture);
 }
 
 void ImageDownloader::load(bool fromHash)
 {
-	if (fromHash && loadFromUrl(cloudImageURL(picture->filename)))
+	if (fromHash && loadFromUrl(cloudImageURL(qPrintable(filename))))
 		return;
 
 	// If loading from hash failed, try to load from filename
-	loadFromUrl(QUrl::fromUserInput(QString(picture->filename)));
+	loadFromUrl(QUrl::fromUserInput(filename));
 }
 
 bool ImageDownloader::loadFromUrl(const QUrl &url)
@@ -75,7 +69,7 @@ void ImageDownloader::saveImage(QNetworkReply *reply, bool &success)
 		stream.writeRawData(imageData.data(), imageData.length());
 		imageFile.waitForBytesWritten(-1);
 		imageFile.close();
-		learnHash(QString(picture->filename), imageFile.fileName(), hash.result());
+		learnHash(filename, imageFile.fileName(), hash.result());
 	}
 	// This should be called to make the picture actually show.
 	// Problem is DivePictureModel is not in core.
@@ -84,22 +78,18 @@ void ImageDownloader::saveImage(QNetworkReply *reply, bool &success)
 
 }
 
-static void loadPicture(struct picture *picture, bool fromHash)
+static void loadPicture(QString filename, bool fromHash)
 {
 	static QSet<QString> queuedPictures;
 	static QMutex pictureQueueMutex;
 
-	if (!picture)
-		return;
 	QMutexLocker locker(&pictureQueueMutex);
-	if (queuedPictures.contains(QString(picture->filename))) {
-		picture_free(picture);
+	if (queuedPictures.contains(filename))
 		return;
-	}
-	queuedPictures.insert(QString(picture->filename));
+	queuedPictures.insert(filename);
 	locker.unlock();
 
-	ImageDownloader download(picture);
+	ImageDownloader download(filename);
 	download.load(fromHash);
 }
 
@@ -113,38 +103,36 @@ static QImage loadImage(const QString &fileName, const char *format = nullptr)
 	return res;
 }
 
-QImage getHashedImage(struct picture *picture)
+QImage getHashedImage(const QString &file)
 {
 	QImage res;
-	QUrl url = QUrl::fromUserInput(localFilePath(QString(picture->filename)));
-	if(url.isLocalFile())
+	QUrl url = QUrl::fromUserInput(localFilePath(file));
+	if (url.isLocalFile())
 		res = loadImage(url.toLocalFile());
 	if (res.isNull()) {
 		// This did not load anything. Let's try to get the image from other sources
 		// Let's try to load it locally via its hash
-		QString filename = localFilePath(picture->filename);
-		qDebug() << QStringLiteral("Translated filename: %1 -> %2").arg(picture->filename, filename);
-		if (filename.isNull()) {
+		QString filenameLocal = localFilePath(qPrintable(file));
+		qDebug() << QStringLiteral("Translated filename: %1 -> %2").arg(file, filenameLocal);
+		if (filenameLocal.isNull()) {
 			// That didn't produce a local filename.
 			// Try the cloud server
 			// TODO: This is dead code at the moment.
-			QtConcurrent::run(loadPicture, clone_picture(picture), true);
+			QtConcurrent::run(loadPicture, file, true);
 		} else {
 			// Load locally from translated file name
-			res = loadImage(filename);
+			res = loadImage(filenameLocal);
 			if (!res.isNull()) {
 				// Make sure the hash still matches the image file
-				qDebug() << "Loaded picture from translated filename" << filename;
-				QtConcurrent::run(hashPicture, clone_picture(picture));
+				QtConcurrent::run(hashPicture, filenameLocal);
 			} else {
 				// Interpret filename as URL
-				qInfo() << "Failed loading picture from translated filename" << filename;
-				QtConcurrent::run(loadPicture, clone_picture(picture), false);
+				QtConcurrent::run(loadPicture, filenameLocal, false);
 			}
 		}
 	} else {
 		// We loaded successfully. Now, make sure hash is up to date.
-		QtConcurrent::run(hashPicture, clone_picture(picture));
+		QtConcurrent::run(hashPicture, file);
 	}
 	return res;
 }
diff --git a/core/imagedownloader.h b/core/imagedownloader.h
index 2a3d95df4..337185869 100644
--- a/core/imagedownloader.h
+++ b/core/imagedownloader.h
@@ -9,16 +9,15 @@
 class ImageDownloader : public QObject {
 	Q_OBJECT
 public:
-	ImageDownloader(struct picture *picture);
-	~ImageDownloader();
+	ImageDownloader(const QString &filename);
 	void load(bool fromHash);
 
 private:
 	bool loadFromUrl(const QUrl &);	// return true on success
 	void saveImage(QNetworkReply *reply, bool &success);
-	struct picture *picture;
+	QString filename;
 };
 
-QImage getHashedImage(struct picture *picture);
+QImage getHashedImage(const QString &filename);
 
 #endif // IMAGEDOWNLOADER_H
diff --git a/core/qthelper.cpp b/core/qthelper.cpp
index 93c4c20f5..797dbf932 100644
--- a/core/qthelper.cpp
+++ b/core/qthelper.cpp
@@ -1240,23 +1240,20 @@ QString localFilePath(const QString &originalFilename)
 		return originalFilename;
 }
 
-// This needs to operate on a copy of picture as it frees it after finishing!
-void hashPicture(struct picture *picture)
+// This works on a copy of the string, because it runs in asynchronous context
+void hashPicture(QString filename)
 {
-	if (!picture)
-		return;
-	QByteArray oldHash = getHash(QString(picture->filename));
-	QByteArray hash = hashFile(localFilePath(picture->filename));
+	QByteArray oldHash = getHash(filename);
+	QByteArray hash = hashFile(localFilePath(filename));
 	if (!hash.isNull() && hash != oldHash)
 		mark_divelist_changed(true);
-	picture_free(picture);
 }
 
 extern "C" void cache_picture(struct picture *picture)
 {
 	QString filename = picture->filename;
 	if (!haveHash(filename))
-		QtConcurrent::run(hashPicture, clone_picture(picture));
+		QtConcurrent::run(hashPicture, filename);
 }
 
 QStringList imageExtensionFilters() {
diff --git a/core/qthelper.h b/core/qthelper.h
index ac1de4df8..41a76ad3b 100644
--- a/core/qthelper.h
+++ b/core/qthelper.h
@@ -33,7 +33,7 @@ QString hashString(const char *filename);
 QString thumbnailFileName(const QString &filename);
 void learnImages(const QDir dir, int max_recursions);
 void add_hash(const QString &filename, const QByteArray &hash);
-void hashPicture(struct picture *picture);
+void hashPicture(QString filename);
 extern "C" char *hashstring(const char *filename);
 QString localFilePath(const QString &originalFilename);
 void learnHash(const QString &originalName, const QString &localName, const QByteArray &hash);
diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp
index 6a90c1431..33ac7e886 100644
--- a/qt-models/divepicturemodel.cpp
+++ b/qt-models/divepicturemodel.cpp
@@ -65,7 +65,7 @@ static void scaleImages(PictureEntry &entry, int maxSize)
 	// Rescale in such a case to avoid resizing artifacts.
 	if (thumbnail.isNull() || (thumbnail.size().width() < maxSize && thumbnail.size().height() < maxSize)) {
 		qDebug() << "No thumbnail in cache for" << entry.filename;
-		thumbnail = getHashedImage(entry.picture).scaled(maxSize, maxSize, Qt::KeepAspectRatio);
+		thumbnail = getHashedImage(QString(entry.picture->filename));
 		addThumbnailToCache(thumbnail, entry);
 	}