Dive pictures: detach ProfileWidget2 from DivePictureModel

As long as ProfileWidget2 and DivePictureModel showed the same set of
pictures and any change would lead to a full recalculation of the set,
it made sense to let ProfileWidget2 use DivePictureModel's data.

Recently, keeping the two lists in sync become more and more of a
burden. Therefore, disconnect ProfileWidget2 and DivePictureModel. This
will lead to some code-duplication and perhaps a temporary drop in
UI-performance, but in the end the code is distinctly simpler and also
more flexible.

Thus, for example the DivePhotoTab could be changed to support headings
without having to touch ProfileWidget2 at all.

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

core/imagedownloader.cpp

@@ -269,12 +269,11 @@ void Thumbnailer::imageDownloadFailed(QString filename)
 	workingOn.remove(filename);
 }
 
-QImage Thumbnailer::fetchThumbnail(PictureEntry &entry)
+QImage Thumbnailer::fetchThumbnail(const QString &filename)
 {
 	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]() { processItem(filename, true); }));

core/imagedownloader.h

@@ -31,9 +31,9 @@ public:
 	static Thumbnailer *instance();
 
 	// Schedule a thumbnail for fetching or calculation.
-	// Returns a placehlder thumbnail. The actual thumbnail will be sent
+	// Returns a placeholder thumbnail. The actual thumbnail will be sent
 	// via a signal later.
-	QImage fetchThumbnail(PictureEntry &entry);
+	QImage fetchThumbnail(const QString &filename);
 
 	// Schedule multiple thumbnails for forced recalculation
 	void calculateThumbnails(const QVector<QString> &filenames);

profile-widget/profilewidget2.cpp

@@ -24,6 +24,7 @@
 #include "desktop-widgets/mainwindow.h"
 #include "core/qthelper.h"
 #include "core/gettextfromc.h"
+#include "core/imagedownloader.h"
 #endif
 
 #include <libdivecomputer/parser.h>
@@ -155,9 +156,9 @@ ProfileWidget2::ProfileWidget2(QWidget *parent) : QGraphicsView(parent),
 	addActionShortcut(Qt::Key_Left, &ProfileWidget2::keyLeftAction);
 	addActionShortcut(Qt::Key_Right, &ProfileWidget2::keyRightAction);
 
-	connect(DivePictureModel::instance(), &DivePictureModel::dataChanged, this, &ProfileWidget2::updatePictures);
+	connect(Thumbnailer::instance(), &Thumbnailer::thumbnailChanged, this, &ProfileWidget2::updateThumbnail, Qt::QueuedConnection);
 	connect(DivePictureModel::instance(), SIGNAL(rowsInserted(const QModelIndex &, int, int)), this, SLOT(plotPictures()));
-	connect(DivePictureModel::instance(), &DivePictureModel::rowsRemoved, this, &ProfileWidget2::removePictures);
+	connect(DivePictureModel::instance(), &DivePictureModel::picturesRemoved, this, &ProfileWidget2::removePictures);
 	connect(DivePictureModel::instance(), &DivePictureModel::modelReset, this, &ProfileWidget2::plotPictures);
 #endif // SUBSURFACE_MOBILE
 
@@ -2063,49 +2064,70 @@ void ProfileWidget2::clearPictures()
 	pictures.clear();
 }
 
-void ProfileWidget2::updatePictures(const QModelIndex &from, const QModelIndex &to)
+// This function is called asynchronously by the thumbnailer if a thumbnail
+// was fetched from disk or freshly calculated.
+void ProfileWidget2::updateThumbnail(QString filename, QImage thumbnail)
 {
-	DivePictureModel *m = DivePictureModel::instance();
-	for (int picNr = from.row(); picNr <= to.row(); ++picNr) {
-		int picItemNr = picNr - m->rowDDStart;
-		if (picItemNr < 0 || (size_t)picItemNr >= pictures.size())
-			return;
-		if (!pictures[picItemNr])
-			return;
+	// Find the picture with the given filename
+	auto it = std::find_if(pictures.begin(), pictures.end(), [&filename](const PictureEntry &e)
+			       { return e.filename == filename; });
 
-		pictures[picItemNr]->setPixmap(m->index(picNr, 0).data(Qt::UserRole).value<QPixmap>());
+	// If we didn't find a picture, it does either not belong to the current dive,
+	// or its timestamp is outside of the profile.
+	if (it != pictures.end()) {
+		// Replace the pixmap of the thumbnail with the newly calculated one.
+		int size = Thumbnailer::defaultThumbnailSize();
+		it->thumbnail->setPixmap(QPixmap::fromImage(thumbnail.scaled(size, size, Qt::KeepAspectRatio)));
 	}
 }
 
+ProfileWidget2::PictureEntry::PictureEntry (offset_t offsetIn, const QString &filenameIn) : offset(offsetIn),
+	filename(filenameIn),
+	thumbnail(new DivePictureItem)
+{
+}
+
+// Define a default sort order for picture-entries: sort lexicographically by timestamp and filename.
+bool ProfileWidget2::PictureEntry::operator< (const PictureEntry &e) const
+{
+	// Use std::tie() for lexicographical sorting.
+	return std::tie(offset.seconds, filename) < std::tie(e.offset.seconds, e.filename);
+}
+
+// This function resets the picture thumbnails of the current dive.
 void ProfileWidget2::plotPictures()
 {
-	DivePictureModel *m = DivePictureModel::instance();
-	pictures.resize(m->rowDDEnd - m->rowDDStart);
+	pictures.clear();
+	if (currentState == ADD || currentState == PLAN)
+		return;
 
+	// Fetch all pictures of the current dive, but consider only those that are within the dive time.
+	// For each picture, create a PictureEntry object in the pictures-vector.
+	// emplace_back() constructs an object at the end of the vector. The parameters are passed directly to the constructor.
+	FOR_EACH_PICTURE(current_dive) {
+		if (picture->offset.seconds > 0 && picture->offset.seconds <= current_dive->duration.seconds)
+			pictures.emplace_back(picture->offset, QString(picture->filename));
+	}
+	if (pictures.empty())
+		return;
+	// Sort pictures by timestamp (and filename if equal timestamps).
+	// This will allow for proper location of the pictures on the profile plot.
+	std::sort(pictures.begin(), pictures.end());
+
+	// Add the DivePictureItems to the scene, set their pixmaps and filenames
+	// and finaly calculate their positions.
 	double x, y, lastX = -1.0, lastY = -1.0;
-	for (int i = m->rowDDStart; i < m->rowDDEnd; i++) {
-		int picItemNr = i - m->rowDDStart;
-		int offsetSeconds = m->index(i, 1).data(Qt::UserRole).value<int>();
-		// it's a correct picture, but doesn't have a timestamp: only show on the widget near the
-		// information area. A null pointer in the pictures array indicates that this picture is not
-		// shown.
-		if (!offsetSeconds) {
-			pictures[picItemNr].reset();
-			continue;
-		}
-		DivePictureItem *item = pictures[picItemNr].get();
-		if (!item) {
-			item = new DivePictureItem;
-			pictures[picItemNr].reset(item);
-			scene()->addItem(item);
-		}
-		item->setVisible(prefs.show_pictures_in_profile);
-		item->setPixmap(m->index(i, 0).data(Qt::UserRole).value<QPixmap>());
-		item->setFileUrl(m->index(i, 1).data().toString());
+	int size = Thumbnailer::defaultThumbnailSize();
+	for (PictureEntry &e: pictures) {
+		scene()->addItem(e.thumbnail.get());
+		e.thumbnail->setVisible(prefs.show_pictures_in_profile);
+		QImage thumbnail = Thumbnailer::instance()->fetchThumbnail(e.filename).scaled(size, size, Qt::KeepAspectRatio);
+		e.thumbnail->setPixmap(QPixmap::fromImage(thumbnail));
+		e.thumbnail->setFileUrl(e.filename);
 		// let's put the picture at the correct time, but at a fixed "depth" on the profile
 		// not sure this is ideal, but it seems to look right.
-		x = timeAxis->posAtValue(offsetSeconds);
-		if (i == 0)
+		x = timeAxis->posAtValue(e.offset.seconds);
+		if (lastX < 0.0)
 			y = 10;
 		else if (fabs(x - lastX) < 3 && lastY <= (10 + 14 * 3))
 			y = lastY + 3;
@@ -2113,20 +2135,26 @@ void ProfileWidget2::plotPictures()
 			y = 10;
 		lastX = x;
 		lastY = y;
-		item->setPos(x, y);
+		e.thumbnail->setPos(x, y);
 	}
 }
 
-void ProfileWidget2::removePictures(const QModelIndex &, int first, int last)
+// Remove the pictures with the given filenames from the profile plot.
+// TODO: This does not check for the fact that the same image may be attributed
+// to different dives! Deleting the picture from one dive may therefore remove
+// it from the profile of a different dive.
+void ProfileWidget2::removePictures(const QVector<QString> &fileUrls)
 {
-	DivePictureModel *m = DivePictureModel::instance();
-	first = std::max(0, first - m->rowDDStart);
-	// Note that last points *to* the last item and not *past* the last item,
-	// therefore we add 1 to achieve conventional C++ semantics.
-	last = std::min((int)pictures.size(), last + 1 - m->rowDDStart);
-	if (first >= (int)pictures.size() || last <= first)
-		return;
-	pictures.erase(pictures.begin() + first, pictures.begin() + last);
+	// To remove the pictures, we use the std::remove_if() algorithm.
+	// std::remove_if() does not actually delete the elements, but moves
+	// them to the end of the given range. It returns an iterator to the
+	// end of the new range of non-deleted elements. A subsequent call to
+	// std::erase on the range of deleted elements then ultimately shrinks the vector.
+	// (c.f. erase-remove idiom: https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom)
+	auto it = std::remove_if(pictures.begin(), pictures.end(), [&fileUrls](const PictureEntry &e)
+			// Check whether filename of entry is in list of provided filenames
+			{ return std::find(fileUrls.begin(), fileUrls.end(), e.filename) != fileUrls.end(); });
+	pictures.erase(it, pictures.end());
 }
 
 #endif

profile-widget/profilewidget2.h

@@ -20,6 +20,7 @@
 #include "profile-widget/diveprofileitem.h"
 #include "core/display.h"
 #include "core/color.h"
+#include "core/units.h"
 
 class RulerItem2;
 struct dive;
@@ -110,7 +111,7 @@ slots: // Necessary to call from QAction's signals.
 	void replot(dive *d = 0);
 #ifndef SUBSURFACE_MOBILE
 	void plotPictures();
-	void removePictures(const QModelIndex &, int first, int last);
+	void removePictures(const QVector<QString> &fileUrls);
 	void setPlanState();
 	void setAddState();
 	void changeGas();
@@ -126,7 +127,7 @@ slots: // Necessary to call from QAction's signals.
 	void deleteCurrentDC();
 	void pointInserted(const QModelIndex &parent, int start, int end);
 	void pointsRemoved(const QModelIndex &, int start, int end);
-	void updatePictures(const QModelIndex &from, const QModelIndex &to);
+	void updateThumbnail(QString filename, QImage thumbnail);
 
 	/* this is called for every move on the handlers. maybe we can speed up this a bit? */
 	void recreatePlannedDive();
@@ -228,8 +229,17 @@ private:
 
 	//specifics for ADD and PLAN
 #ifndef SUBSURFACE_MOBILE
-	// Use std::vector<> and std::unique_ptr<>, because QVector<QScopedPointer<...>> is unsupported.
-	std::vector<std::unique_ptr<DivePictureItem>> pictures;
+	// The list of pictures in this plot. The pictures are sorted by offset in seconds.
+	// For the same offset, sort by filename.
+	// Pictures that are outside of the dive time are not shown.
+	struct PictureEntry {
+		offset_t offset;
+		QString filename;
+		std::unique_ptr<DivePictureItem> thumbnail;
+		PictureEntry (offset_t offsetIn, const QString &filenameIn);
+		bool operator< (const PictureEntry &e) const;
+	};
+	std::vector<PictureEntry> pictures;
 
 	QList<DiveHandler *> handles;
 	void repositionDiveHandlers();

qt-models/divepicturemodel.cpp

@@ -14,10 +14,7 @@ DivePictureModel *DivePictureModel::instance()
 	return self;
 }
 
-DivePictureModel::DivePictureModel() : rowDDStart(0),
-				       rowDDEnd(0),
-				       zoomLevel(0.0),
-				       defaultSize(Thumbnailer::defaultThumbnailSize())
+DivePictureModel::DivePictureModel() : zoomLevel(0.0)
 {
 	connect(Thumbnailer::instance(), &Thumbnailer::thumbnailChanged,
 		this, &DivePictureModel::updateThumbnail, Qt::QueuedConnection);
@@ -44,7 +41,7 @@ void DivePictureModel::updateThumbnails()
 {
 	updateZoom();
 	for (PictureEntry &entry: pictures)
-		entry.image = Thumbnailer::instance()->fetchThumbnail(entry);
+		entry.image = Thumbnailer::instance()->fetchThumbnail(entry.filename);
 }
 
 void DivePictureModel::updateDivePictures()
@@ -52,7 +49,6 @@ void DivePictureModel::updateDivePictures()
 	beginResetModel();
 	if (!pictures.isEmpty()) {
 		pictures.clear();
-		rowDDStart = rowDDEnd = 0;
 		Thumbnailer::instance()->clearWorkQueue();
 	}
 
@@ -60,12 +56,8 @@ void DivePictureModel::updateDivePictures()
 	struct dive *dive;
 	for_each_dive (i, dive) {
 		if (dive->selected) {
-			if (dive->id == displayed_dive.id)
-				rowDDStart = pictures.count();
 			FOR_EACH_PICTURE(dive)
 				pictures.push_back({picture, picture->filename, {}, picture->offset.seconds});
-			if (dive->id == displayed_dive.id)
-				rowDDEnd = pictures.count();
 		}
 	}
 
@@ -93,9 +85,6 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const
 		case Qt::DecorationRole:
 			ret = entry.image.scaled(size, size, Qt::KeepAspectRatio);
 			break;
-		case Qt::UserRole:	// Used by profile widget to access bigger thumbnails
-			ret = entry.image.scaled(defaultSize, defaultSize, Qt::KeepAspectRatio);
-			break;
 		case Qt::DisplayRole:
 			ret = QFileInfo(entry.filename).fileName();
 			break;
@@ -126,14 +115,6 @@ static bool removePictureFromSelectedDive(const char *fileUrl)
 	return false;
 }
 
-// Calculate how many items of a range are before the given index
-static int rangeBefore(int rangeFrom, int rangeTo, int index)
-{
-	if (rangeTo <= rangeFrom)
-		return 0;
-	return std::min(rangeTo, index) - std::min(rangeFrom, index);
-}
-
 void DivePictureModel::removePictures(const QVector<QString> &fileUrls)
 {
 	bool removed = false;
@@ -159,13 +140,8 @@ void DivePictureModel::removePictures(const QVector<QString> &fileUrls)
 		beginRemoveRows(QModelIndex(), i, j - 1);
 		pictures.erase(pictures.begin() + i, pictures.begin() + j);
 		endRemoveRows();
-
-		// After removing pictures, we have to adjust rowDDStart and rowDDEnd.
-		// Calculate the part of the range that is before rowDDStart and rowDDEnd,
-		// respectively and subtract accordingly.
-		rowDDStart -= rangeBefore(i, j, rowDDStart);
-		rowDDEnd -= rangeBefore(i, j, rowDDEnd);
 	}
+	emit picturesRemoved(fileUrls);
 }
 
 int DivePictureModel::rowCount(const QModelIndex&) const

qt-models/divepicturemodel.h

@@ -22,8 +22,9 @@ public:
 	virtual int rowCount(const QModelIndex &parent = QModelIndex()) const;
 	virtual void updateDivePictures();
 	void removePictures(const QVector<QString> &fileUrls);
-	int rowDDStart, rowDDEnd;
 	void updateDivePictureOffset(const QString &filename, int offsetSeconds);
+signals:
+	void picturesRemoved(const QVector<QString> &fileUrls);
 public slots:
 	void setZoomLevel(int level);
 	void updateThumbnail(QString filename, QImage thumbnail);
@@ -33,7 +34,6 @@ private:
 	int findPictureId(const QString &filename);	// Return -1 if not found
 	double zoomLevel;	// -1.0: minimum, 0.0: standard, 1.0: maximum
 	int size;
-	int defaultSize;
 	void updateThumbnails();
 	void updateZoom();
 };