Dive pictures: don't update all images on drag&drop to profile

Gracefully handle drag & drop to the profile, which changes the
offset of the pictures. To do this, keep the pictures in the
DivePictureModel and the ProfileWidget2 sorted by offset and
re-arrange if needed to keep the list sorted. This needs some
code reshuffling.

Introduce a helper-function that moves ranges in arrays.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

core/qthelper.h

@@ -88,6 +88,32 @@ QString getUserAgent();
 #define TITLE_OR_TEXT(_t, _m) _t, _m
 #endif
 
+// Move a range in a vector to a different position.
+// The parameters are given according to the usual STL-semantics:
+//	v: a container with STL-like random access iterator via std::begin(...)
+//	rangeBegin: index of first element
+//	rangeEnd: index one *past* last element
+//	destination: index to element before which the range will be moved
+// Owing to std::begin() magic, this function works with STL-like containers:
+//	QVector<int> v{ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+//	moveInVector(v, 1, 4, 6);
+// as well as with C-style arrays:
+//	int array[10] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
+//	moveInVector(array, 1, 4, 6);
+// Both calls will have the following effect:
+//	Before: 0 1 2 3 4 5 6 7 8 9
+//	After:  0 4 5 1 2 3 6 7 8 9
+// No sanitizing of the input arguments is performed.
+template <typename Vector>
+void moveInVector(Vector &v, int rangeBegin, int rangeEnd, int destination)
+{
+	auto it = std::begin(v);
+	if (destination > rangeEnd)
+		std::rotate(it + rangeBegin, it + rangeEnd, it + destination);
+	else if (destination < rangeBegin)
+		std::rotate(it + destination, it + rangeBegin, it + rangeEnd);
+}
+
 #endif
 
 // 3) Functions visible to C and C++

profile-widget/profilewidget2.cpp

@@ -2081,10 +2081,17 @@ void ProfileWidget2::updateThumbnail(QString filename, QImage thumbnail)
 	}
 }
 
-ProfileWidget2::PictureEntry::PictureEntry (offset_t offsetIn, const QString &filenameIn) : offset(offsetIn),
+// Create a PictureEntry object and add its thumbnail to the scene if profile pictures are shown.
+ProfileWidget2::PictureEntry::PictureEntry(offset_t offsetIn, const QString &filenameIn, QGraphicsScene *scene) : offset(offsetIn),
 	filename(filenameIn),
 	thumbnail(new DivePictureItem)
 {
+	int size = Thumbnailer::defaultThumbnailSize();
+	scene->addItem(thumbnail.get());
+	thumbnail->setVisible(prefs.show_pictures_in_profile);
+	QImage img = Thumbnailer::instance()->fetchThumbnail(filename).scaled(size, size, Qt::KeepAspectRatio);
+	thumbnail->setPixmap(QPixmap::fromImage(img));
+	thumbnail->setFileUrl(filename);
 }
 
 // Define a default sort order for picture-entries: sort lexicographically by timestamp and filename.
@@ -2094,9 +2101,13 @@ bool ProfileWidget2::PictureEntry::operator< (const PictureEntry &e) const
 	return std::tie(offset.seconds, filename) < std::tie(e.offset.seconds, e.filename);
 }
 
+// Calculate the y-coordinates of the thumbnails, which are supposed to be sorted by x-coordinate.
+// This will also change the order in which the thumbnails are painted, to avoid weird effects,
+// when items are added later to the scene. This is simply done by increasing the Z-value.
 void ProfileWidget2::calculatePictureYPositions()
 {
 	double lastX = -1.0, lastY;
+	double z = 0.0;
 	for (PictureEntry &e: pictures) {
 		if (!e.thumbnail)
 			continue;
@@ -2111,9 +2122,19 @@ void ProfileWidget2::calculatePictureYPositions()
 		lastX = x;
 		lastY = y;
 		e.thumbnail->setY(y);
+		e.thumbnail->setZValue(z);
+		z += 1.0;
 	}
 }
 
+void ProfileWidget2::updateThumbnailXPos(PictureEntry &e)
+{
+	// Here, we only set the x-coordinate of the picture. The y-coordinate
+	// will be set later in calculatePictureYPositions().
+	double x = timeAxis->posAtValue(e.offset.seconds);
+	e.thumbnail->setX(x);
+}
+
 // This function resets the picture thumbnails of the current dive.
 void ProfileWidget2::plotPictures()
 {
@@ -2124,9 +2145,10 @@ void ProfileWidget2::plotPictures()
 	// 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.
+	// Note that FOR_EACH_PICTURE handles current_dive being null gracefully.
 	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));
+			pictures.emplace_back(picture->offset, QString(picture->filename), scene());
 	}
 	if (pictures.empty())
 		return;
@@ -2134,21 +2156,9 @@ void ProfileWidget2::plotPictures()
 	// 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 finally calculate their positions.
-	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);
-
-		// Here, we only set the x-coordinate of the picture. The y-coordinate
-		// will be set later in calculatePictureYPositions().
-		double x = timeAxis->posAtValue(e.offset.seconds);
-		e.thumbnail->setX(x);
-	}
+	// Calculate thumbnail positions. First the x-coordinates and and then the y-coordinates.
+	for (PictureEntry &e: pictures)
+		updateThumbnailXPos(e);
 	calculatePictureYPositions();
 }
 
@@ -2180,23 +2190,87 @@ void ProfileWidget2::dropEvent(QDropEvent *event)
 		QDataStream dataStream(&itemData, QIODevice::ReadOnly);
 
 		QString filename;
-		QPoint offset;
-		dataStream >> filename >> offset;
+		QPoint pos;
+		dataStream >> filename >> pos;
 
-		QPointF mappedPos = mapToScene(event->pos());
-
-		FOR_EACH_PICTURE(current_dive) {
-			if (QString(picture->filename) == filename) {
-				picture->offset.seconds = lrint(timeAxis->valueAt(mappedPos));
-				mark_divelist_changed(true);
 #ifndef SUBSURFACE_MOBILE
-				DivePictureModel::instance()->updateDivePictureOffset(filename, picture->offset.seconds);
-				plotPictures();
-#endif
+		// Calculate time in dive where picture was dropped and whether the new position is during the dive.
+		QPointF mappedPos = mapToScene(event->pos());
+		offset_t offset { (int32_t)lrint(timeAxis->valueAt(mappedPos)) };
+		bool duringDive = current_dive && offset.seconds > 0 && offset.seconds < current_dive->duration.seconds;
+
+		// Flag which states whether the drag&dropped picture actually belongs to this dive.
+		// If this is not the case, the calculated offset makes no sense whatsoever and we must ignore the event.
+		bool belongsToDive = true;
+
+		// A picture was drag&dropped onto the profile: We have four cases to consider:
+		//	1a) The image was already shown on the profile and is moved to a different position on the profile.
+		//	    Calculate the new position and move the picture.
+		//	1b) The image was on the profile and is moved outside of the dive time.
+		//	    Remove the picture.
+		//	2a) The image was not on the profile, but belongs to the current dive.
+		//	    Add the picture to the profile if it is during the dive.
+		//	2b) The picture does not belong to the current dive.
+		//	    For now, do nothing. We may think about adding the picture to the dive.
+		auto oldPos = std::find_if(pictures.begin(), pictures.end(), [filename](const PictureEntry &e)
+					   { return e.filename == filename; });
+		if (oldPos != pictures.end()) {
+			// Cases 1a) and 1b): picture is on profile
+			if (duringDive) {
+				// Case 1a): move to new position
+				// First, find new position. Note that we also have to compare filenames,
+				// because it is quite easy to generate equal offsets.
+				auto newPos = std::find_if(pictures.begin(), pictures.end(), [offset, &filename](const PictureEntry &e)
+							   { return std::tie(e.offset.seconds, e.filename) > std::tie(offset.seconds, filename); });
+				// Set new offset
+				oldPos->offset.seconds = offset.seconds;
+				updateThumbnailXPos(*oldPos);
+
+				// Move image from old to new position
+				int oldIndex = oldPos - pictures.begin();
+				int newIndex = newPos - pictures.begin();
+				moveInVector(pictures, oldIndex, oldIndex + 1, newIndex);
+			} else {
+				// Case 1b): remove picture
+				pictures.erase(oldPos);
+			}
+
+			// In both cases the picture list changed, therefore we must recalculate the y-coordinatesA.
+			calculatePictureYPositions();
+		} else {
+			// Cases 2a) and 2b): picture not on profile. Check if it belongs to current dive.
+			// Note that FOR_EACH_PICTURE handles current_dive being null gracefully.
+			bool found = false;
+			FOR_EACH_PICTURE(current_dive) {
+				if (picture->filename == filename) {
+					found = true;
 					break;
 				}
 			}
-		copy_dive(current_dive, &displayed_dive);
+			if (found && duringDive) {
+				// Case 2a): add the picture at the appropriate position.
+				// The case move from outside-to-outside of the profile plot was handled by
+				// the "&& duringDive" condition in the if above.
+				// As for case 1a), we have to also consider filenames in the case of equal offsets.
+				auto newPos = std::find_if(pictures.begin(), pictures.end(), [offset, &filename](const PictureEntry &e)
+							   { return std::tie(e.offset.seconds, e.filename) > std::tie(offset.seconds, filename); });
+				// emplace() constructs the element at the given position in the vector.
+				// The parameters are passed directly to the contructor.
+				// The call returns an iterator to the new element (which might differ from
+				// the old iterator, since the buffer might have been reallocated).
+				newPos = pictures.emplace(newPos, offset, filename, scene());
+				updateThumbnailXPos(*newPos);
+				calculatePictureYPositions();
+			} else if (!found) {
+				// Case 2b): Unknown picture. Ignore.
+				belongsToDive = false;
+			}
+		}
+
+		// Only signal the drag&drop action if the picture actually belongs to the dive.
+		if (belongsToDive)
+			DivePictureModel::instance()->updateDivePictureOffset(displayed_dive.id, filename, offset.seconds);
+#endif
 
 		if (event->source() == this) {
 			event->setDropAction(Qt::MoveAction);

profile-widget/profilewidget2.h

@@ -236,9 +236,10 @@ private:
 		offset_t offset;
 		QString filename;
 		std::unique_ptr<DivePictureItem> thumbnail;
-		PictureEntry (offset_t offsetIn, const QString &filenameIn);
+		PictureEntry (offset_t offsetIn, const QString &filenameIn, QGraphicsScene *scene);
 		bool operator< (const PictureEntry &e) const;
 	};
+	void updateThumbnailXPos(PictureEntry &e);
 	std::vector<PictureEntry> pictures;
 	void calculatePictureYPositions();
 

qt-models/divepicturemodel.cpp

@@ -56,8 +56,14 @@ void DivePictureModel::updateDivePictures()
 	struct dive *dive;
 	for_each_dive (i, dive) {
 		if (dive->selected) {
+			int first = pictures.count();
 			FOR_EACH_PICTURE(dive)
-				pictures.push_back({picture, picture->filename, {}, picture->offset.seconds});
+				pictures.push_back({ dive->id, picture, picture->filename, {}, picture->offset.seconds });
+
+			// Sort pictures of this dive by offset.
+			// Thus, the list will be sorted by (diveId, offset).
+			std::sort(pictures.begin() + first, pictures.end(),
+				  [](const PictureEntry &a, const PictureEntry &b) { return a.offsetSeconds < b.offsetSeconds; });
 		}
 	}
 
@@ -166,11 +172,39 @@ void DivePictureModel::updateThumbnail(QString filename, QImage thumbnail)
 	}
 }
 
-void DivePictureModel::updateDivePictureOffset(const QString &filename, int offsetSeconds)
+void DivePictureModel::updateDivePictureOffset(int diveId, const QString &filename, int offsetSeconds)
 {
-	int i = findPictureId(filename);
-	if (i >= 0) {
-		pictures[i].offsetSeconds = offsetSeconds;
-		emit dataChanged(createIndex(i, 0), createIndex(i, 1));
+	// Find the pictures of the given dive.
+	auto from = std::find_if(pictures.begin(), pictures.end(), [diveId](const PictureEntry &e) { return e.diveId == diveId; });
+	auto to = std::find_if(from, pictures.end(), [diveId](const PictureEntry &e) { return e.diveId != diveId; });
+
+	// Find picture with the given filename
+	auto oldPos = std::find_if(from, to, [filename](const PictureEntry &e) { return e.filename == filename; });
+	if (oldPos == to)
+		return;
+
+	// Find new position
+	auto newPos = std::find_if(from, to, [offsetSeconds](const PictureEntry &e) { return e.offsetSeconds > offsetSeconds; });
+
+	// Update the offset here and in the backend
+	oldPos->offsetSeconds = offsetSeconds;
+	if (struct dive *dive = get_dive_by_uniq_id(diveId)) {
+		FOR_EACH_PICTURE(dive) {
+			if (picture->filename == filename) {
+				picture->offset.seconds = offsetSeconds;
+				mark_divelist_changed(true);
+				break;
 			}
 		}
+		copy_dive(current_dive, &displayed_dive);
+	}
+
+	// Henceforth we will work with indices instead of iterators
+	int oldIndex = oldPos - pictures.begin();
+	int newIndex = newPos - pictures.begin();
+	if (oldIndex == newIndex || oldIndex + 1 == newIndex)
+		return;
+	beginMoveRows(QModelIndex(), oldIndex, oldIndex, QModelIndex(), newIndex);
+	moveInVector(pictures, oldIndex, oldIndex + 1, newIndex);
+	endMoveRows();
+}

qt-models/divepicturemodel.h

@@ -7,6 +7,7 @@
 #include <QFuture>
 
 struct PictureEntry {
+	int diveId;
 	struct picture *picture;
 	QString filename;
 	QImage image;
@@ -22,7 +23,7 @@ public:
 	virtual int rowCount(const QModelIndex &parent = QModelIndex()) const;
 	virtual void updateDivePictures();
 	void removePictures(const QVector<QString> &fileUrls);
-	void updateDivePictureOffset(const QString &filename, int offsetSeconds);
+	void updateDivePictureOffset(int diveId, const QString &filename, int offsetSeconds);
 signals:
 	void picturesRemoved(const QVector<QString> &fileUrls);
 public slots: