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>

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)
 {

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);

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))) {
+	if (queuedPictures.contains(filename))
-		picture_free(picture);
 		return;
-	}
+	queuedPictures.insert(filename);
-	queuedPictures.insert(QString(picture->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)));
+	QUrl url = QUrl::fromUserInput(localFilePath(file));
-	if(url.isLocalFile())
+	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);
+		QString filenameLocal = localFilePath(qPrintable(file));
-		qDebug() << QStringLiteral("Translated filename: %1 -> %2").arg(picture->filename, filename);
+		qDebug() << QStringLiteral("Translated filename: %1 -> %2").arg(file, filenameLocal);
-		if (filename.isNull()) {
+		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, filenameLocal);
-				QtConcurrent::run(hashPicture, clone_picture(picture));
 			} else {
 				// Interpret filename as URL
-				qInfo() << "Failed loading picture from translated filename" << filename;
+				QtConcurrent::run(loadPicture, filenameLocal, false);
-				QtConcurrent::run(loadPicture, clone_picture(picture), 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;
 }

core/imagedownloader.h

@@ -9,16 +9,15 @@
 class ImageDownloader : public QObject {
 	Q_OBJECT
 public:
-	ImageDownloader(struct picture *picture);
+	ImageDownloader(const QString &filename);
-	~ImageDownloader();
 	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

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!
+// This works on a copy of the string, because it runs in asynchronous context
-void hashPicture(struct picture *picture)
+void hashPicture(QString filename)
 {
-	if (!picture)
+	QByteArray oldHash = getHash(filename);
-		return;
+	QByteArray hash = hashFile(localFilePath(filename));
-	QByteArray oldHash = getHash(QString(picture->filename));
-	QByteArray hash = hashFile(localFilePath(picture->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() {

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);

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);
 	}