diff --git a/commands/command.cpp b/commands/command.cpp index 8195fbd22..d67009978 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -367,4 +367,9 @@ void setPictureOffset(dive *d, const QString &filename, offset_t offset) execute(new SetPictureOffset(d, filename, offset)); } +void removePictures(const std::vector &pictures) +{ + execute(new RemovePictures(pictures)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index 5d92eec03..e4c3b05d2 100644 --- a/commands/command.h +++ b/commands/command.h @@ -3,6 +3,7 @@ #define COMMAND_H #include "core/dive.h" +#include "core/pictureobj.h" #include #include #include @@ -120,7 +121,16 @@ void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank); // 7) Picture (media) commands +struct PictureListForDeletion { + dive *d; + std::vector filenames; +}; +struct PictureListForAddition { + dive *d; + std::vector pics; +}; void setPictureOffset(dive *d, const QString &filename, offset_t offset); +void removePictures(const std::vector &pictures); } // namespace Command diff --git a/commands/command_pictures.cpp b/commands/command_pictures.cpp index b7f03b193..2e40c0362 100644 --- a/commands/command_pictures.cpp +++ b/commands/command_pictures.cpp @@ -47,4 +47,110 @@ bool SetPictureOffset::workToBeDone() return !!d; } +// Filter out pictures that don't exist and sort them according to the backend +static PictureListForDeletion filterPictureListForDeletion(const PictureListForDeletion &p) +{ + PictureListForDeletion res; + res.d = p.d; + res.filenames.reserve(p.filenames.size()); + for (int i = 0; i < p.d->pictures.nr; ++i) { + std::string fn = p.d->pictures.pictures[i].filename; + if (std::find(p.filenames.begin(), p.filenames.end(), fn) != p.filenames.end()) + res.filenames.push_back(fn); + } + return res; +} + +// Helper function to remove pictures. Clears the passed-in vector. +static std::vector removePictures(std::vector &picturesToRemove) +{ + std::vector res; + for (const PictureListForDeletion &list: picturesToRemove) { + QVector filenames; + PictureListForAddition toAdd; + toAdd.d = list.d; + for (const std::string &fn: list.filenames) { + int idx = get_picture_idx(&list.d->pictures, fn.c_str()); + if (idx < 0) { + fprintf(stderr, "removePictures(): picture disappeared!"); + continue; // Huh? We made sure that this can't happen by filtering out non-existant pictures. + } + filenames.push_back(QString::fromStdString(fn)); + toAdd.pics.emplace_back(list.d->pictures.pictures[idx]); + remove_from_picture_table(&list.d->pictures, idx); + } + if (!toAdd.pics.empty()) + res.push_back(toAdd); + invalidate_dive_cache(list.d); + emit diveListNotifier.picturesRemoved(list.d, filenames); + } + picturesToRemove.clear(); + return res; +} + +// Helper function to add pictures. Clears the passed-in vector. +static std::vector addPictures(std::vector &picturesToAdd) +{ + // We play it extra safe here and again filter out those pictures that + // are already added to the dive. There should be no way for this to + // happen, as we checked that before. + std::vector res; + for (const PictureListForAddition &list: picturesToAdd) { + QVector picsForSignal; + PictureListForDeletion toRemove; + toRemove.d = list.d; + for (const PictureObj &pic: list.pics) { + int idx = get_picture_idx(&list.d->pictures, pic.filename.c_str()); // This should *not* already exist! + if (idx >= 0) { + fprintf(stderr, "addPictures(): picture disappeared!"); + continue; // Huh? We made sure that this can't happen by filtering out existing pictures. + } + picsForSignal.push_back(pic); + add_picture(&list.d->pictures, pic.toCore()); + toRemove.filenames.push_back(pic.filename); + } + if (!toRemove.filenames.empty()) + res.push_back(toRemove); + invalidate_dive_cache(list.d); + emit diveListNotifier.picturesAdded(list.d, picsForSignal); + } + picturesToAdd.clear(); + return res; +} + +RemovePictures::RemovePictures(const std::vector &pictures) : picturesToRemove(pictures) +{ + // Filter out the pictures that don't actually exist. In principle this shouldn't be necessary. + // Nevertheless, let's play it save. This has the additional benefit of sorting the pictures + // according to the backend if, for some reason, the caller passed the pictures in in random order. + picturesToRemove.reserve(pictures.size()); + size_t count = 0; + for (const PictureListForDeletion &p: pictures) { + PictureListForDeletion filtered = filterPictureListForDeletion(p); + if (filtered.filenames.size()) + picturesToRemove.push_back(p); + count += filtered.filenames.size(); + } + if (count == 0) { + picturesToRemove.clear(); // This signals that nothing is to be done + return; + } + setText(Command::Base::tr("remove %n pictures(s)", "", count)); +} + +bool RemovePictures::workToBeDone() +{ + return !picturesToRemove.empty(); +} + +void RemovePictures::undo() +{ + picturesToRemove = addPictures(picturesToAdd); +} + +void RemovePictures::redo() +{ + picturesToAdd = removePictures(picturesToRemove); +} + } // namespace Command diff --git a/commands/command_pictures.h b/commands/command_pictures.h index f2381d111..802c63c6c 100644 --- a/commands/command_pictures.h +++ b/commands/command_pictures.h @@ -5,6 +5,7 @@ #define COMMAND_PICTURES_H #include "command_base.h" +#include "command.h" // for PictureListForDeletion/Addition // We put everything in a namespace, so that we can shorten names without polluting the global namespace namespace Command { @@ -22,5 +23,17 @@ private: bool workToBeDone() override; }; +class RemovePictures final : public Base { +public: + RemovePictures(const std::vector &pictures); +private: + std::vector picturesToRemove; // for redo + std::vector picturesToAdd; // for undo + + void undo() override; + void redo() override; + bool workToBeDone() override; +}; + } // namespace Command #endif diff --git a/core/picture.c b/core/picture.c index d7ba597f6..36953b8b1 100644 --- a/core/picture.c +++ b/core/picture.c @@ -30,7 +30,7 @@ static bool picture_less_than(struct picture a, struct picture b) static MAKE_GROW_TABLE(picture_table, struct picture, pictures) static MAKE_GET_INSERTION_INDEX(picture_table, struct picture, pictures, picture_less_than) MAKE_ADD_TO(picture_table, struct picture, pictures) -static MAKE_REMOVE_FROM(picture_table, pictures) +MAKE_REMOVE_FROM(picture_table, pictures) MAKE_SORT(picture_table, struct picture, pictures, comp_pictures) //MAKE_REMOVE(picture_table, struct picture, picture) MAKE_CLEAR_TABLE(picture_table, pictures, picture) @@ -66,13 +66,3 @@ int get_picture_idx(const struct picture_table *t, const char *filename) } return -1; } - -// Return true if picture was found and deleted -bool remove_picture(struct picture_table *t, const char *filename) -{ - int idx = get_picture_idx(t, filename); - if (idx < 0) - return false; - remove_from_picture_table(t, idx); - return true; -} diff --git a/core/picture.h b/core/picture.h index 999ef9681..61492fffc 100644 --- a/core/picture.h +++ b/core/picture.h @@ -33,7 +33,7 @@ extern void clear_picture_table(struct picture_table *); extern void add_to_picture_table(struct picture_table *, int idx, struct picture pic); extern void copy_pictures(const struct picture_table *s, struct picture_table *d); extern void add_picture(struct picture_table *, struct picture newpic); -extern bool remove_picture(struct picture_table *, const char *filename); +extern void remove_from_picture_table(struct picture_table *, int idx); extern int get_picture_idx(const struct picture_table *, const char *filename); /* Return -1 if not found */ extern void sort_picture_table(struct picture_table *); diff --git a/core/subsurface-qt/divelistnotifier.h b/core/subsurface-qt/divelistnotifier.h index 0e85d6f26..2931e443e 100644 --- a/core/subsurface-qt/divelistnotifier.h +++ b/core/subsurface-qt/divelistnotifier.h @@ -6,6 +6,7 @@ #define DIVELISTNOTIFIER_H #include "core/dive.h" +#include "core/pictureobj.h" #include @@ -118,6 +119,8 @@ signals: // Picture (media) related signals void pictureOffsetChanged(dive *d, QString filename, offset_t offset); + void picturesRemoved(dive *d, QVector filenames); + void picturesAdded(dive *d, QVector pics); // This signal is emited every time a command is executed. // This is used to hide an old multi-dives-edited warning message. diff --git a/desktop-widgets/tab-widgets/TabDivePhotos.cpp b/desktop-widgets/tab-widgets/TabDivePhotos.cpp index 731e58388..db1b943e4 100644 --- a/desktop-widgets/tab-widgets/TabDivePhotos.cpp +++ b/desktop-widgets/tab-widgets/TabDivePhotos.cpp @@ -84,7 +84,8 @@ QVector TabDivePhotos::getSelectedFilenames() const void TabDivePhotos::removeSelectedPhotos() { - DivePictureModel::instance()->removePictures(getSelectedFilenames()); + QModelIndexList indices = ui->photosView->selectionModel()->selectedRows(); + DivePictureModel::instance()->removePictures(indices); } void TabDivePhotos::openFolderOfSelectedFiles() diff --git a/profile-widget/divepixmapitem.cpp b/profile-widget/divepixmapitem.cpp index 611f3f76a..7b2c9cc87 100644 --- a/profile-widget/divepixmapitem.cpp +++ b/profile-widget/divepixmapitem.cpp @@ -1,12 +1,13 @@ // SPDX-License-Identifier: GPL-2.0 #include "profile-widget/divepixmapitem.h" #include "profile-widget/animationfunctions.h" -#include "qt-models/divepicturemodel.h" #include "core/pref.h" #include "core/qthelper.h" #include "core/settings/qPrefDisplay.h" #ifndef SUBSURFACE_MOBILE #include "desktop-widgets/preferences/preferencesdialog.h" +#include "core/dive.h" // for displayed_dive +#include "commands/command.h" #endif #include @@ -123,6 +124,8 @@ void DivePictureItem::mousePressEvent(QGraphicsSceneMouseEvent *event) void DivePictureItem::removePicture() { #ifndef SUBSURFACE_MOBILE - DivePictureModel::instance()->removePictures({ fileUrl }); + struct dive *d = get_dive_by_uniq_id(displayed_dive.id); + if (d) + Command::removePictures({ { d, { fileUrl.toStdString() } } }); #endif } diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index cea5a50f0..e1a0415ea 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -169,9 +169,8 @@ ProfileWidget2::ProfileWidget2(QWidget *parent) : QGraphicsView(parent), addActionShortcut(Qt::Key_Right, &ProfileWidget2::keyRightAction); connect(Thumbnailer::instance(), &Thumbnailer::thumbnailChanged, this, &ProfileWidget2::updateThumbnail, Qt::QueuedConnection); - connect(DivePictureModel::instance(), &DivePictureModel::rowsInserted, this, &ProfileWidget2::plotPictures); - connect(DivePictureModel::instance(), &DivePictureModel::picturesRemoved, this, &ProfileWidget2::removePictures); - connect(DivePictureModel::instance(), &DivePictureModel::modelReset, this, &ProfileWidget2::plotPictures); + connect(&diveListNotifier, &DiveListNotifier::picturesRemoved, this, &ProfileWidget2::picturesRemoved); + connect(&diveListNotifier, &DiveListNotifier::picturesAdded, this, &ProfileWidget2::picturesAdded); connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &ProfileWidget2::profileChanged); connect(&diveListNotifier, &DiveListNotifier::eventsChanged, this, &ProfileWidget2::profileChanged); connect(&diveListNotifier, &DiveListNotifier::pictureOffsetChanged, this, &ProfileWidget2::pictureOffsetChanged); @@ -2147,11 +2146,10 @@ void ProfileWidget2::plotPicturesInternal(const struct dive *d, bool synchronous } // 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 &fileUrls) +void ProfileWidget2::picturesRemoved(dive *d, QVector fileUrls) { + if (d->id != displayed_dive.id) + return; // 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 @@ -2165,6 +2163,25 @@ void ProfileWidget2::removePictures(const QVector &fileUrls) calculatePictureYPositions(); } +void ProfileWidget2::picturesAdded(dive *d, QVector pics) +{ + if (d->id != displayed_dive.id) + return; + + for (const PictureObj &pic: pics) { + if (pic.offset.seconds > 0 && pic.offset.seconds <= d->duration.seconds) { + pictures.emplace_back(pic.offset, QString::fromStdString(pic.filename), scene(), false); + updateThumbnailXPos(pictures.back()); + } + } + + // 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()); + + calculatePictureYPositions(); +} + void ProfileWidget2::profileChanged(dive *d) { if (!d || d->id != displayed_dive.id) diff --git a/profile-widget/profilewidget2.h b/profile-widget/profilewidget2.h index 039424d87..e6f06a7b2 100644 --- a/profile-widget/profilewidget2.h +++ b/profile-widget/profilewidget2.h @@ -20,6 +20,7 @@ #include "profile-widget/diveprofileitem.h" #include "core/display.h" #include "core/color.h" +#include "core/pictureobj.h" #include "core/units.h" class RulerItem2; @@ -110,7 +111,8 @@ slots: // Necessary to call from QAction's signals. void setProfileState(); #ifndef SUBSURFACE_MOBILE void plotPictures(); - void removePictures(const QVector &fileUrls); + void picturesRemoved(dive *d, QVector filenames); + void picturesAdded(dive *d, QVector pics); void setPlanState(); void setAddState(); void pointInserted(const QModelIndex &parent, int start, int end); diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index 351396e5f..7c9fb18c2 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -1,16 +1,42 @@ // SPDX-License-Identifier: GPL-2.0 #include "qt-models/divepicturemodel.h" +#include "core/divelist.h" // for comp_dives #include "core/metrics.h" -#include "core/divelist.h" // for mark_divelist_changed() -#include "core/dive.h" #include "core/imagedownloader.h" #include "core/picture.h" #include "core/qthelper.h" #include "core/subsurface-qt/divelistnotifier.h" +#include "commands/command.h" #include #include +PictureEntry::PictureEntry(dive *dIn, const PictureObj &p) : d(dIn), + filename(p.filename), + offsetSeconds(p.offset.seconds), + length({ 0 }) +{ +} + +PictureEntry::PictureEntry(dive *dIn, const picture &p) : d(dIn), + filename(p.filename), + offsetSeconds(p.offset.seconds), + length({ 0 }) +{ +} + +// Note: it is crucial that this uses the same sorting as the core. +// Therefore, we use the C strcmp functions [std::string::operator<() +// should give the same result]. +bool PictureEntry::operator<(const PictureEntry &p2) const +{ + if (int cmp = comp_dives(d, p2.d)) + return cmp < 0; + if (offsetSeconds != p2.offsetSeconds) + return offsetSeconds < p2.offsetSeconds; + return strcmp(filename.c_str(), p2.filename.c_str()) < 0; +} + DivePictureModel *DivePictureModel::instance() { static DivePictureModel *self = new DivePictureModel(); @@ -23,6 +49,10 @@ DivePictureModel::DivePictureModel() : zoomLevel(0.0) this, &DivePictureModel::updateThumbnail, Qt::QueuedConnection); connect(&diveListNotifier, &DiveListNotifier::pictureOffsetChanged, this, &DivePictureModel::pictureOffsetChanged); + connect(&diveListNotifier, &DiveListNotifier::picturesRemoved, + this, &DivePictureModel::picturesRemoved); + connect(&diveListNotifier, &DiveListNotifier::picturesAdded, + this, &DivePictureModel::picturesAdded); } void DivePictureModel::setZoomLevel(int level) @@ -63,7 +93,7 @@ void DivePictureModel::updateDivePictures() if (dive->selected) { size_t first = pictures.size(); FOR_EACH_PICTURE(dive) - pictures.push_back({ dive, picture->filename, {}, picture->offset.seconds, {.seconds = 0}}); + pictures.push_back(PictureEntry(dive, *picture)); // Sort pictures of this dive by offset. // Thus, the list will be sorted by (dive, offset). @@ -111,43 +141,51 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const return QVariant(); } -// Return true if we actually removed a picture -static bool removePictureFromSelectedDive(const char *fileUrl) +void DivePictureModel::removePictures(const QModelIndexList &indices) { - int i; - struct dive *dive; - for_each_dive (i, dive) { - if (dive->selected && remove_picture(&dive->pictures, fileUrl)) { - invalidate_dive_cache(dive); - return true; - } + // Collect pictures to remove by dive + std::vector pics; + for (const QModelIndex &idx: indices) { + if (!idx.isValid()) + continue; + const PictureEntry &item = pictures[idx.row()]; + // Check if we already have pictures for that dive. + auto it = find_if(pics.begin(), pics.end(), + [&item](const Command::PictureListForDeletion &list) + { return list.d == item.d; }); + // If not found, add a new list + if (it == pics.end()) + pics.push_back({ item.d, { item.filename }}); + else + it->filenames.push_back(item.filename); } - return false; + Command::removePictures(pics); } -void DivePictureModel::removePictures(const QVector &fileUrlsIn) +void DivePictureModel::picturesRemoved(dive *d, QVector filenamesIn) { // Transform vector of QStrings into vector of std::strings - std::vector fileUrls; - fileUrls.reserve(fileUrlsIn.size()); - std::transform(fileUrlsIn.begin(), fileUrlsIn.end(), std::back_inserter(fileUrls), + std::vector filenames; + filenames.reserve(filenamesIn.size()); + std::transform(filenamesIn.begin(), filenamesIn.end(), std::back_inserter(filenames), [] (const QString &s) { return s.toStdString(); }); - bool removed = false; - for (const std::string &fileUrl: fileUrls) - removed |= removePictureFromSelectedDive(fileUrl.c_str()); - if (!removed) + // Get range of pictures of the given dive. + // Note: we could be more efficient by either using a binary search or a two-level data structure. + auto from = std::find_if(pictures.begin(), pictures.end(), [d](const PictureEntry &e) { return e.d == d; }); + auto to = std::find_if(from, pictures.end(), [d](const PictureEntry &e) { return e.d != d; }); + if (from == pictures.end()) return; - copy_dive(current_dive, &displayed_dive); - mark_divelist_changed(true); - for (size_t i = 0; i < pictures.size(); ++i) { + size_t fromIdx = from - pictures.begin(); + size_t toIdx = to - pictures.begin(); + for (size_t i = fromIdx; i < toIdx; ++i) { // Find range [i j) of pictures to remove - if (std::find(fileUrls.begin(), fileUrls.end(), pictures[i].filename) == fileUrls.end()) + if (std::find(filenames.begin(), filenames.end(), pictures[i].filename) == filenames.end()) continue; size_t j; - for (j = i + 1; j < pictures.size(); ++j) { - if (std::find(fileUrls.begin(), fileUrls.end(), pictures[j].filename) == fileUrls.end()) + for (j = i + 1; j < toIdx; ++j) { + if (std::find(filenames.begin(), filenames.end(), pictures[j].filename) == filenames.end()) break; } @@ -156,8 +194,48 @@ void DivePictureModel::removePictures(const QVector &fileUrlsIn) beginRemoveRows(QModelIndex(), i, j - 1); pictures.erase(pictures.begin() + i, pictures.begin() + j); endRemoveRows(); + toIdx -= j - i; + } + copy_dive(current_dive, &displayed_dive); // TODO: Remove once displayed_dive is moved to the planner +} + +// Assumes that pics is sorted! +void DivePictureModel::picturesAdded(dive *d, QVector picsIn) +{ + // We only display pictures of selected dives + if (!d->selected || picsIn.empty()) + return; + + // Convert the picture-data into our own format + std::vector pics; + pics.reserve(picsIn.size()); + for (int i = 0; i < picsIn.size(); ++i) + pics.push_back(PictureEntry(d, picsIn[i])); + + // Insert batch-wise to avoid too many reloads + pictures.reserve(pictures.size() + pics.size()); + auto from = pics.begin(); + int dest = 0; + while (from != pics.end()) { + // Search for the insertion index. This supposes a lexicographical sort for the [dive, offset, filename] triple. + // TODO: currently this works, because all undo commands that manipulate the dive list also reset the selection + // and thus the model is rebuilt. However, we might catch the respective signals here and not rely on being + // called by the tab-widgets. + auto dest_it = std::lower_bound(pictures.begin() + dest, pictures.end(), *from); + int dest = dest_it - pictures.begin(); + auto to = dest_it == pictures.end() ? pics.end() : from + 1; // If at the end - just add the rest + while (to != pics.end() && *to < *dest_it) + ++to; + int batch_size = to - from; + beginInsertRows(QModelIndex(), dest, dest + batch_size - 1); + pictures.insert(pictures.begin() + dest, from, to); + // Get thumbnails of inserted pictures + for (auto it = pictures.begin() + dest; it < pictures.begin() + dest + batch_size; ++it) + it->image = Thumbnailer::instance()->fetchThumbnail(QString::fromStdString(it->filename), false); + endInsertRows(); + from = to; + dest += batch_size; } - emit picturesRemoved(fileUrlsIn); } int DivePictureModel::rowCount(const QModelIndex&) const diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h index 395dbedd1..d7b043d44 100644 --- a/qt-models/divepicturemodel.h +++ b/qt-models/divepicturemodel.h @@ -3,6 +3,7 @@ #define DIVEPICTUREMODEL_H #include "core/units.h" +#include "core/pictureobj.h" #include #include @@ -16,6 +17,9 @@ struct PictureEntry { QImage image; int offsetSeconds; duration_t length; + PictureEntry(dive *, const PictureObj &); + PictureEntry(dive *, const picture &); + bool operator<(const PictureEntry &) const; }; class DivePictureModel : public QAbstractTableModel { @@ -26,13 +30,13 @@ public: QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override; int rowCount(const QModelIndex &parent = QModelIndex()) const override; void updateDivePictures(); - void removePictures(const QVector &fileUrls); -signals: - void picturesRemoved(const QVector &fileUrls); + void removePictures(const QModelIndexList &); public slots: void setZoomLevel(int level); void updateThumbnail(QString filename, QImage thumbnail, duration_t duration); void pictureOffsetChanged(dive *d, const QString filename, offset_t offset); + void picturesRemoved(dive *d, QVector filenames); + void picturesAdded(dive *d, QVector pics); private: DivePictureModel(); std::vector pictures;