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>

core/imagedownloader.cpp

@@ -4,9 +4,12 @@
 #include "divelist.h"
 #include "qthelper.h"
 #include "imagedownloader.h"
+#include "qt-models/divepicturemodel.h"
+#include "metadata.h"
 #include <unistd.h>
 #include <QString>
 #include <QImageReader>
+#include <QDataStream>
 
 #include <QtConcurrent>
 
@@ -118,21 +121,122 @@ QImage getHashedImage(const QString &file)
 			// That didn't produce a local filename.
 			// Try the cloud server
 			// TODO: This is dead code at the moment.
-			QtConcurrent::run(loadPicture, file, true);
+			loadPicture(file, true);
 		} else {
 			// Load locally from translated file name
 			res = loadImage(filenameLocal);
 			if (!res.isNull()) {
 				// Make sure the hash still matches the image file
-				QtConcurrent::run(hashPicture, filenameLocal);
+				hashPicture(filenameLocal);
 			} else {
 				// Interpret filename as URL
-				QtConcurrent::run(loadPicture, filenameLocal, false);
+				loadPicture(filenameLocal, false);
 			}
 		}
 	} else {
 		// We loaded successfully. Now, make sure hash is up to date.
-		QtConcurrent::run(hashPicture, file);
+		hashPicture(file);
 	}
 	return res;
 }
+
+Thumbnailer::Thumbnailer()
+{
+	// Currently, we only process one image at a time. Stefan Fuchs reported problems when
+	// calculating multiple thumbnails at once and this hopefully helps.
+	pool.setMaxThreadCount(1);
+}
+
+Thumbnailer *Thumbnailer::instance()
+{
+	static Thumbnailer self;
+	return &self;
+}
+
+static QImage getThumbnailFromCache(const QString &picture_filename)
+{
+	// First, check if we know a hash for this filename
+	QString filename = thumbnailFileName(picture_filename);
+	if (filename.isEmpty())
+		return QImage();
+
+	QFile file(filename);
+	if (!file.open(QIODevice::ReadOnly))
+		return QImage();
+	QDataStream stream(&file);
+
+	// Each thumbnail file is composed of a media-type and an image file.
+	// Currently, the type is ignored. This will be used to mark videos.
+	quint32 type;
+	QImage res;
+	stream >> type;
+	stream >> res;
+	return res;
+}
+
+static void addThumbnailToCache(const QImage &thumbnail, const QString &picture_filename)
+{
+	if (thumbnail.isNull())
+		return;
+
+	QString filename = thumbnailFileName(picture_filename);
+
+	// If we got a thumbnail, we are guaranteed to have its hash and therefore
+	// thumbnailFileName() should return a filename.
+	if (filename.isEmpty()) {
+		qWarning() << "Internal error: can't get filename of recently created thumbnail";
+		return;
+	}
+
+	QSaveFile file(filename);
+	if (!file.open(QIODevice::WriteOnly))
+		return;
+	QDataStream stream(&file);
+
+	// For format of the file, see comments in getThumnailForCache
+	quint32 type = MEDIATYPE_PICTURE;
+	stream << type;
+	stream << thumbnail;
+	file.commit();
+}
+
+void Thumbnailer::processItem(QString filename, int size)
+{
+	QImage thumbnail = getThumbnailFromCache(filename);
+
+	if (thumbnail.isNull()) {
+		thumbnail = getHashedImage(filename);
+		if (thumbnail.isNull()) {
+			// TODO: Don't misuse filter close icon
+			thumbnail = QImage(":filter-close").scaled(size, size, Qt::KeepAspectRatio);
+		} else {
+			thumbnail = thumbnail.scaled(size, size, Qt::KeepAspectRatio);
+			addThumbnailToCache(thumbnail, filename);
+		}
+	}
+
+	QMutexLocker l(&lock);
+	emit thumbnailChanged(filename, thumbnail);
+	workingOn.remove(filename);
+}
+
+QImage Thumbnailer::fetchThumbnail(PictureEntry &entry, int size)
+{
+	QMutexLocker l(&lock);
+
+	// We are not currently fetching this thumbnail - add it to the list.
+	const QString &filename = entry.filename;
+	if (!workingOn.contains(filename)) {
+		workingOn.insert(filename,
+				 QtConcurrent::run(&pool, [this, filename, size]() { processItem(filename, size); }));
+	}
+	return QImage(":photo-icon").scaled(size, size, Qt::KeepAspectRatio);
+}
+
+void Thumbnailer::clearWorkQueue()
+{
+	QMutexLocker l(&lock);
+	for (auto it = workingOn.begin(); it != workingOn.end(); ++it)
+		it->cancel();
+	workingOn.clear();
+}

core/imagedownloader.h

@@ -5,6 +5,7 @@
 #include <QImage>
 #include <QFuture>
 #include <QNetworkReply>
+#include <QThreadPool>
 
 class ImageDownloader : public QObject {
 	Q_OBJECT
@@ -18,6 +19,31 @@ private:
 	QString filename;
 };
 
+class PictureEntry;
+class Thumbnailer : public QObject {
+	Q_OBJECT
+public:
+	static Thumbnailer *instance();
+
+	// Schedule a thumbnail for fetching or calculation.
+	// Returns a placehlder thumbnail. The actual thumbnail will be sent
+	// via a signal later.
+	QImage fetchThumbnail(PictureEntry &entry, int size);
+
+	// If we change dive, clear all unfinished thumbnail creations
+	void clearWorkQueue();
+signals:
+	void thumbnailChanged(QString filename, QImage thumbnail);
+private:
+	Thumbnailer();
+	void processItem(QString filename, int size);
+
+	mutable QMutex lock;
+	QThreadPool pool;
+
+	QMap<QString,QFuture<void>> workingOn;
+};
+
 QImage getHashedImage(const QString &filename);
 
 #endif // IMAGEDOWNLOADER_H

core/qthelper.cpp

@@ -11,6 +11,9 @@
 #include "gettextfromc.h"
 #include "metadata.h"
 #include <sys/time.h>
+#include "exif.h"
+#include "file.h"
+#include "imagedownloader.h"
 #include "prefs-macros.h"
 #include <QFile>
 #include <QRegExp>

qt-models/divepicturemodel.cpp

@@ -5,73 +5,11 @@
 #include "core/divelist.h"
 #include "core/imagedownloader.h"
 #include "core/qthelper.h"
-#include "core/metadata.h"
 
-#include <QtConcurrent>
+#include <QFileInfo>
 
 static const int maxZoom = 3;	// Maximum zoom: thrice of standard size
 
-static QImage getThumbnailFromCache(const PictureEntry &entry)
-{
-	// First, check if we know a hash for this filename
-	QString filename = thumbnailFileName(entry.filename);
-	if (filename.isEmpty())
-		return QImage();
-
-	QFile file(filename);
-	if (!file.open(QIODevice::ReadOnly))
-		return QImage();
-	QDataStream stream(&file);
-
-	// Each thumbnail file is composed of a media-type and an image file.
-	// Currently, the type is ignored. This will be used to mark videos.
-	quint32 type;
-	QImage res;
-	stream >> type;
-	stream >> res;
-	return res;
-}
-
-static void addThumbnailToCache(const QImage &thumbnail, const PictureEntry &entry)
-{
-	if (thumbnail.isNull())
-		return;
-
-	QString filename = thumbnailFileName(entry.filename);
-
-	// If we got a thumbnail, we are guaranteed to have its hash and therefore
-	// thumbnailFileName() should return a filename.
-	if (filename.isEmpty()) {
-		qWarning() << "Internal error: can't get filename of recently created thumbnail";
-		return;
-	}
-
-	QSaveFile file(filename);
-	if (!file.open(QIODevice::WriteOnly))
-		return;
-	QDataStream stream(&file);
-
-	// For format of the file, see comments in getThumnailForCache
-	quint32 type = MEDIATYPE_PICTURE;
-	stream << type;
-	stream << thumbnail;
-	file.commit();
-}
-
-static void scaleImages(PictureEntry &entry, int maxSize)
-{
-	QImage thumbnail = getThumbnailFromCache(entry);
-	// If thumbnails were written by an earlier version, they might be smaller than needed.
-	// 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(QString(entry.picture->filename));
-		addThumbnailToCache(thumbnail, entry);
-	}
-
-	entry.image = thumbnail;
-}
-
 DivePictureModel *DivePictureModel::instance()
 {
 	static DivePictureModel *self = new DivePictureModel();
@@ -121,7 +59,8 @@ void DivePictureModel::updateThumbnails()
 {
 	int maxSize = defaultSize * maxZoom;
 	updateZoom();
-	QtConcurrent::blockingMap(pictures, [maxSize](PictureEntry &entry){scaleImages(entry, maxSize);});
+	for (PictureEntry &entry: pictures)
+		entry.image = Thumbnailer::instance()->fetchThumbnail(entry, maxSize);
 }
 
 void DivePictureModel::updateDivePictures()
@@ -131,6 +70,7 @@ void DivePictureModel::updateDivePictures()
 		pictures.clear();
 		endRemoveRows();
 		rowDDStart = rowDDEnd = 0;
+		Thumbnailer::instance()->clearWorkQueue();
 	}
 
 	// if the dive_table is empty, quit