From 3c0c1801cd869f7a98c23356ab2ae4e373f55f31 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Fri, 18 May 2018 21:57:18 +0200 Subject: [PATCH] Dive pictures: change removePicture() interface The function removePicture() had a flag "last", which would indicate that the called had finished removing pictures. Only then would the model be recalculated. This is a strange interface and, matter of fact, the caller was buggy: if the last picture to be removed didn't have a proper url, removePicture() was never called with "last" being set. Change the interface to take a list of pictures to be deleted. This will allow us to make picture deletion smarter in follow-up commits. Signed-off-by: Berthold Stoeger --- desktop-widgets/tab-widgets/TabDivePhotos.cpp | 16 +++++------- profile-widget/divepixmapitem.cpp | 2 +- qt-models/divepicturemodel.cpp | 26 +++++++++++++------ qt-models/divepicturemodel.h | 2 +- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/desktop-widgets/tab-widgets/TabDivePhotos.cpp b/desktop-widgets/tab-widgets/TabDivePhotos.cpp index a2a41398f..1a5d233e5 100644 --- a/desktop-widgets/tab-widgets/TabDivePhotos.cpp +++ b/desktop-widgets/tab-widgets/TabDivePhotos.cpp @@ -60,23 +60,21 @@ void TabDivePhotos::contextMenuEvent(QContextMenuEvent *event) void TabDivePhotos::removeSelectedPhotos() { - bool last = false; if (!ui->photosView->selectionModel()->hasSelection()) return; QModelIndexList indexes = ui->photosView->selectionModel()->selectedRows(); if (indexes.count() == 0) indexes = ui->photosView->selectionModel()->selectedIndexes(); - QModelIndex photo = indexes.first(); - do { - photo = indexes.first(); - last = indexes.count() == 1; + QVector photosToDelete; + photosToDelete.reserve(indexes.count()); + for (const auto &photo: indexes) { if (photo.isValid()) { QString fileUrl = photo.data(Qt::DisplayPropertyRole).toString(); - if (fileUrl.length() > 0) - DivePictureModel::instance()->removePicture(fileUrl, last); + if (!fileUrl.isEmpty()) + photosToDelete.push_back(fileUrl); } - indexes.removeFirst(); - } while(!indexes.isEmpty()); + } + DivePictureModel::instance()->removePictures(photosToDelete); } //TODO: This looks overly wrong. We shouldn't call MainWindow to retrieve the DiveList to add Images. diff --git a/profile-widget/divepixmapitem.cpp b/profile-widget/divepixmapitem.cpp index c81cd51e1..27fea611b 100644 --- a/profile-widget/divepixmapitem.cpp +++ b/profile-widget/divepixmapitem.cpp @@ -116,6 +116,6 @@ void DivePictureItem::mousePressEvent(QGraphicsSceneMouseEvent *event) void DivePictureItem::removePicture() { #ifndef SUBSURFACE_MOBILE - DivePictureModel::instance()->removePicture(fileUrl, true); + DivePictureModel::instance()->removePictures({ fileUrl }); #endif } diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index 9b993cb5a..3fa1a07f4 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -127,19 +127,29 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const return ret; } -void DivePictureModel::removePicture(const QString &fileUrl, bool last) +// Return true if we actually removed a picture +static bool removePictureFromSelectedDive(const char *fileUrl) { int i; struct dive *dive; for_each_dive (i, dive) { - if (dive->selected && dive_remove_picture(dive, qPrintable(fileUrl))) - break; - } - if (last) { - copy_dive(current_dive, &displayed_dive); - updateDivePictures(); - mark_divelist_changed(true); + if (dive->selected && dive_remove_picture(dive, fileUrl)) + return true; } + return false; +} + +void DivePictureModel::removePictures(const QVector &fileUrls) +{ + bool removed = false; + for (const QString &fileUrl: fileUrls) + removed |= removePictureFromSelectedDive(qPrintable(fileUrl)); + if (!removed) + return; + copy_dive(current_dive, &displayed_dive); + mark_divelist_changed(true); + + updateDivePictures(); } int DivePictureModel::rowCount(const QModelIndex &parent) const diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h index 7f2cde6d0..61eb1d984 100644 --- a/qt-models/divepicturemodel.h +++ b/qt-models/divepicturemodel.h @@ -22,7 +22,7 @@ public: virtual int rowCount(const QModelIndex &parent = QModelIndex()) const; virtual void updateDivePictures(); void updateDivePicturesWhenDone(QList>); - void removePicture(const QString& fileUrl, bool last); + void removePictures(const QVector &fileUrls); int rowDDStart, rowDDEnd; void updateDivePictureOffset(const QString &filename, int offsetSeconds); public slots: