Simplify DivePictureModel

The code of DivePictureModel used a QHash to keep track of thumbnails.
Not only was the code rather complex - it also had the consequence that
pictures are sorted according to the hash function, i.e. seemingly
random.

This commit replaces the QHash by a simple QList which keeps track
of thumbnails and some meta-data.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

3 files changed, 35 insertions, 59 deletions

diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp
index e5b40b748..d135e0faa 100644
--- a/qt-models/divepicturemodel.cpp
+++ b/qt-models/divepicturemodel.cpp
@@ -9,22 +9,19 @@
 
 extern QHash <QString, QImage> thumbnailCache;
 
-SPixmap scaleImages(picturepointer picture)
+void scaleImages(PictureEntry &entry)
 {
-	SPixmap ret;
-	ret.first = picture;
-	if (thumbnailCache.contains(picture->filename) && !thumbnailCache.value(picture->filename).isNull()) {
-		ret.second = thumbnailCache.value(picture->filename);
+	if (thumbnailCache.contains(entry.filename) && !thumbnailCache.value(entry.filename).isNull()) {
+		entry.image = thumbnailCache.value(entry.filename);
 	} else {
 		int dim = defaultIconMetrics().sz_pic;
-		QImage p = SHashedImage(picture);
+		QImage p = SHashedImage(entry.picture);
 		if(!p.isNull()) {
 			p = p.scaled(dim, dim, Qt::KeepAspectRatio);
-			thumbnailCache.insert(picture->filename, p);
+			thumbnailCache.insert(entry.filename, p);
 		}
-		ret.second = p;
+		entry.image = p;
 	}
-	return ret;
 }
 
 DivePictureModel *DivePictureModel::instance()
@@ -33,7 +30,7 @@ DivePictureModel *DivePictureModel::instance()
 	return self;
 }
 
-DivePictureModel::DivePictureModel() : numberOfPictures(0)
+DivePictureModel::DivePictureModel()
 {
 }
 
@@ -47,30 +44,22 @@ void DivePictureModel::updateDivePicturesWhenDone(QList<QFuture<void>> futures)
 
 void DivePictureModel::updateDivePictures()
 {
-	if (numberOfPictures != 0) {
-		beginRemoveRows(QModelIndex(), 0, numberOfPictures - 1);
-		numberOfPictures = 0;
+	if (!pictures.isEmpty()) {
+		beginRemoveRows(QModelIndex(), 0, pictures.count() - 1);
+		pictures.clear();
 		endRemoveRows();
 	}
 
 	// if the dive_table is empty, ignore the displayed_dive
-	numberOfPictures = dive_table.nr == 0 ? 0 : dive_get_picture_count(&displayed_dive);
-	if (numberOfPictures == 0) {
+	if (dive_table.nr == 0 || dive_get_picture_count(&displayed_dive) == 0)
 		return;
-	}
 
-	stringPixmapCache.clear();
-	SPictureList pictures;
-	FOR_EACH_PICTURE_NON_PTR(displayed_dive) {
-		stringPixmapCache[QString(picture->filename)].offsetSeconds = picture->offset.seconds;
-		pictures.push_back(picture);
-	}
+	FOR_EACH_PICTURE_NON_PTR(displayed_dive)
+		pictures.push_back({picture, picture->filename, QImage(), picture->offset.seconds});
 
-	QList<SPixmap> list = QtConcurrent::blockingMapped(pictures, scaleImages);
-	Q_FOREACH (const SPixmap &pixmap, list)
-		stringPixmapCache[pixmap.first->filename].image = pixmap.second;
+	QtConcurrent::blockingMap(pictures, scaleImages);
 
-	beginInsertRows(QModelIndex(), 0, numberOfPictures - 1);
+	beginInsertRows(QModelIndex(), 0, pictures.count() - 1);
 	endInsertRows();
 }
 
@@ -86,28 +75,28 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const
 	if (!index.isValid())
 		return ret;
 
-	QString key = stringPixmapCache.keys().at(index.row());
+	const PictureEntry &entry = pictures.at(index.row());
 	if (index.column() == 0) {
 		switch (role) {
 		case Qt::ToolTipRole:
-			ret = key;
+			ret = entry.filename;
 			break;
 		case Qt::DecorationRole:
-			ret = stringPixmapCache[key].image;
+			ret = entry.image;
 			break;
 		case Qt::DisplayRole:
-			ret = QFileInfo(key).fileName();
+			ret = QFileInfo(entry.filename).fileName();
 			break;
 		case Qt::DisplayPropertyRole:
-			ret = QFileInfo(key).filePath();
+			ret = QFileInfo(entry.filename).filePath();
 		}
 	} else if (index.column() == 1) {
 		switch (role) {
 		case Qt::UserRole:
-			ret = QVariant::fromValue((int)stringPixmapCache[key].offsetSeconds);
+			ret = QVariant::fromValue(entry.offsetSeconds);
 			break;
 		case Qt::DisplayRole:
-			ret = key;
+			ret = entry.filename;
 		}
 	}
 	return ret;
@@ -126,5 +115,5 @@ void DivePictureModel::removePicture(const QString &fileUrl, bool last)
 int DivePictureModel::rowCount(const QModelIndex &parent) const
 {
 	Q_UNUSED(parent);
-	return numberOfPictures;
+	return pictures.count();
 }
diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h
index b0494d355..bbb82293e 100644
--- a/qt-models/divepicturemodel.h
+++ b/qt-models/divepicturemodel.h
@@ -6,17 +6,15 @@
 #include <QImage>
 #include <QFuture>
 
-struct PhotoHelper {
+struct PictureEntry {
+	struct picture *picture;
+	QString filename;
 	QImage image;
 	int offsetSeconds;
 };
 
-typedef QList<struct picture *> SPictureList;
-typedef struct picture *picturepointer;
-typedef QPair<picturepointer, QImage> SPixmap;
-
 // function that will scale the pixmap, used inside the QtConcurrent thread.
-SPixmap scaleImages(picturepointer picture);
+void scaleImages(PictureEntry &entry);
 
 class DivePictureModel : public QAbstractTableModel {
 	Q_OBJECT
@@ -31,11 +29,10 @@ public:
 
 protected:
 	DivePictureModel();
-	int numberOfPictures;
 	// Currently, load the images on the fly
 	// Later, use a thread to load the images
 	// Later, save the thumbnails so we don't need to reopen every time.
-	QHash<QString, PhotoHelper> stringPixmapCache;
+	QList<PictureEntry> pictures;
 };
 
 #endif
diff --git a/qt-models/divesitepicturesmodel.cpp b/qt-models/divesitepicturesmodel.cpp
index 0cd444ca8..b83d616c0 100644
--- a/qt-models/divesitepicturesmodel.cpp
+++ b/qt-models/divesitepicturesmodel.cpp
@@ -17,30 +17,20 @@ DiveSitePicturesModel::DiveSitePicturesModel() {
 
 void DiveSitePicturesModel::updateDivePictures() {
 	beginResetModel();
-	numberOfPictures = 0;
+	pictures.clear();
 	endResetModel();
 
 	const uint32_t ds_uuid = displayed_dive_site.uuid;
 	struct dive *d;
 	int i;
 
-	stringPixmapCache.clear();
-	SPictureList pictures;
+	for_each_dive (i, d)
+		if (d->dive_site_uuid == ds_uuid && dive_get_picture_count(d))
+			FOR_EACH_PICTURE(d)
+				pictures.push_back({picture, picture->filename, QImage(), picture->offset.seconds});
 
-	for_each_dive (i, d) {
-		if (d->dive_site_uuid == ds_uuid && dive_get_picture_count(d)) {
-			FOR_EACH_PICTURE(d) {
-				stringPixmapCache[QString(picture->filename)].offsetSeconds = picture->offset.seconds;
-				pictures.push_back(picture);
-			}
-		}
-	}
+	QtConcurrent::blockingMap(pictures, scaleImages);
 
-	QList<SPixmap> list = QtConcurrent::blockingMapped(pictures, scaleImages);
-	Q_FOREACH (const SPixmap &pixmap, list)
-		stringPixmapCache[pixmap.first->filename].image = pixmap.second;
-
-	numberOfPictures = list.count();
-	beginInsertRows(QModelIndex(), 0, numberOfPictures - 1);
+	beginInsertRows(QModelIndex(), 0, pictures.count() - 1);
 	endInsertRows();
 }