From e61641c79cd57bfa55d2371615a7eef7c73b4eb7 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Tue, 14 Apr 2020 22:07:00 +0200 Subject: undo: implement undo of setting a picture time by drag&drop Even though the functionality is seemingly trivial, this is a bit invasive, as the code has to be split into two distinct parts: 1) Post undo command 2) React to changes to the divelist Don't compile that code on mobile. Signed-off-by: Berthold Stoeger --- commands/CMakeLists.txt | 2 + commands/command.cpp | 8 ++ commands/command.h | 4 + commands/command_pictures.cpp | 50 ++++++++++++ commands/command_pictures.h | 26 ++++++ core/subsurface-qt/divelistnotifier.h | 3 + packaging/ios/Subsurface-mobile.pro | 2 + profile-widget/profilewidget2.cpp | 145 +++++++++++++++++----------------- profile-widget/profilewidget2.h | 1 + qt-models/divepicturemodel.cpp | 24 +++--- qt-models/divepicturemodel.h | 3 +- 11 files changed, 178 insertions(+), 90 deletions(-) create mode 100644 commands/command_pictures.cpp create mode 100644 commands/command_pictures.h diff --git a/commands/CMakeLists.txt b/commands/CMakeLists.txt index ceb2740f0..aa51dd066 100644 --- a/commands/CMakeLists.txt +++ b/commands/CMakeLists.txt @@ -16,6 +16,8 @@ set(SUBSURFACE_GENERIC_COMMANDS_SRCS command_edit_trip.h command_event.cpp command_event.h + command_pictures.cpp + command_pictures.h ) add_library(subsurface_commands STATIC ${SUBSURFACE_GENERIC_COMMANDS_SRCS}) target_link_libraries(subsurface_commands ${QT_LIBRARIES}) diff --git a/commands/command.cpp b/commands/command.cpp index 33c866e72..8195fbd22 100644 --- a/commands/command.cpp +++ b/commands/command.cpp @@ -6,6 +6,7 @@ #include "command_edit.h" #include "command_edit_trip.h" #include "command_event.h" +#include "command_pictures.h" namespace Command { @@ -359,4 +360,11 @@ void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank) execute(new AddGasSwitch(d, dcNr, seconds, tank)); } +// Picture (media) commands + +void setPictureOffset(dive *d, const QString &filename, offset_t offset) +{ + execute(new SetPictureOffset(d, filename, offset)); +} + } // namespace Command diff --git a/commands/command.h b/commands/command.h index fc1ddf582..5d92eec03 100644 --- a/commands/command.h +++ b/commands/command.h @@ -118,6 +118,10 @@ void renameEvent(struct dive *d, int dcNr, struct event *ev, const char *name); void removeEvent(struct dive *d, int dcNr, struct event *ev); void addGasSwitch(struct dive *d, int dcNr, int seconds, int tank); +// 7) Picture (media) commands + +void setPictureOffset(dive *d, const QString &filename, offset_t offset); + } // namespace Command #endif // COMMAND_H diff --git a/commands/command_pictures.cpp b/commands/command_pictures.cpp new file mode 100644 index 000000000..b7f03b193 --- /dev/null +++ b/commands/command_pictures.cpp @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include "command_pictures.h" +#include "core/subsurface-qt/divelistnotifier.h" + +namespace Command { + +static picture *dive_get_picture(const dive *d, const QString &fn) +{ + int idx = get_picture_idx(&d->pictures, qPrintable(fn)); + return idx < 0 ? nullptr : &d->pictures.pictures[idx]; +} + +SetPictureOffset::SetPictureOffset(dive *dIn, const QString &filenameIn, offset_t offsetIn) : + d(dIn), filename(filenameIn), offset(offsetIn) +{ + if (!dive_get_picture(d, filename)) + d = nullptr; + setText(Command::Base::tr("Change media time")); +} + +void SetPictureOffset::redo() +{ + picture *pic = dive_get_picture(d, filename); + if (!pic) { + fprintf(stderr, "SetPictureOffset::redo(): picture disappeared!"); + return; + } + std::swap(pic->offset, offset); + offset_t newOffset = pic->offset; + + // Instead of trying to be smart, let's simply resort the picture table. + // If someone complains about speed, do our usual "smart" thing. + sort_picture_table(&d->pictures); + emit diveListNotifier.pictureOffsetChanged(d, filename, newOffset); + invalidate_dive_cache(d); +} + +// Undo and redo do the same thing +void SetPictureOffset::undo() +{ + redo(); +} + +bool SetPictureOffset::workToBeDone() +{ + return !!d; +} + +} // namespace Command diff --git a/commands/command_pictures.h b/commands/command_pictures.h new file mode 100644 index 000000000..f2381d111 --- /dev/null +++ b/commands/command_pictures.h @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +// Note: this header file is used by the undo-machinery and should not be included elsewhere. + +#ifndef COMMAND_PICTURES_H +#define COMMAND_PICTURES_H + +#include "command_base.h" + +// We put everything in a namespace, so that we can shorten names without polluting the global namespace +namespace Command { + +class SetPictureOffset final : public Base { +public: + SetPictureOffset(dive *d, const QString &filename, offset_t offset); // Pictures are identified by the unique (dive,filename) pair +private: + dive *d; // null means no work to be done + QString filename; + offset_t offset; + + void undo() override; + void redo() override; + bool workToBeDone() override; +}; + +} // namespace Command +#endif diff --git a/core/subsurface-qt/divelistnotifier.h b/core/subsurface-qt/divelistnotifier.h index b5f8106d5..0e85d6f26 100644 --- a/core/subsurface-qt/divelistnotifier.h +++ b/core/subsurface-qt/divelistnotifier.h @@ -116,6 +116,9 @@ signals: // Event-related signals. Currently, we're very blunt: only one signal for any changes to the events. void eventsChanged(dive *d); + // Picture (media) related signals + void pictureOffsetChanged(dive *d, QString filename, offset_t offset); + // This signal is emited every time a command is executed. // This is used to hide an old multi-dives-edited warning message. // This is necessary, so that the user can't click on the "undo" button and undo diff --git a/packaging/ios/Subsurface-mobile.pro b/packaging/ios/Subsurface-mobile.pro index 2e70662c5..a036ccae8 100644 --- a/packaging/ios/Subsurface-mobile.pro +++ b/packaging/ios/Subsurface-mobile.pro @@ -21,6 +21,7 @@ SOURCES += ../../subsurface-mobile-main.cpp \ ../../commands/command_divesite.cpp \ ../../commands/command_edit.cpp \ ../../commands/command_edit_trip.cpp \ + ../../commands/command_pictures.cpp \ ../../core/cloudstorage.cpp \ ../../core/configuredivecomputerthreads.cpp \ ../../core/devicedetails.cpp \ @@ -186,6 +187,7 @@ HEADERS += \ ../../commands/command_divesite.h \ ../../commands/command_edit.h \ ../../commands/command_edit_trip.h \ + ../../commands/command_pictures.h \ ../../core/libdivecomputer.h \ ../../core/cloudstorage.h \ ../../core/configuredivecomputerthreads.h \ diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index 71db3d66f..cea5a50f0 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -19,7 +19,7 @@ #include "core/pref.h" #include "qt-models/diveplannermodel.h" #include "qt-models/models.h" -#include "qt-models/divepicturemodel.h" +#include "qt-models/divepicturemodel.h" // TODO: remove once divepictures have been undo-ified #include "core/divelist.h" #include "core/errorhelper.h" #ifndef SUBSURFACE_MOBILE @@ -174,6 +174,7 @@ ProfileWidget2::ProfileWidget2(QWidget *parent) : QGraphicsView(parent), connect(DivePictureModel::instance(), &DivePictureModel::modelReset, this, &ProfileWidget2::plotPictures); connect(&diveListNotifier, &DiveListNotifier::cylinderEdited, this, &ProfileWidget2::profileChanged); connect(&diveListNotifier, &DiveListNotifier::eventsChanged, this, &ProfileWidget2::profileChanged); + connect(&diveListNotifier, &DiveListNotifier::pictureOffsetChanged, this, &ProfileWidget2::pictureOffsetChanged); #endif // SUBSURFACE_MOBILE #if !defined(QT_NO_DEBUG) && defined(SHOW_PLOT_INFO_TABLE) @@ -2175,86 +2176,16 @@ void ProfileWidget2::profileChanged(dive *d) void ProfileWidget2::dropEvent(QDropEvent *event) { +#ifndef SUBSURFACE_MOBILE if (event->mimeData()->hasFormat("application/x-subsurfaceimagedrop")) { QByteArray itemData = event->mimeData()->data("application/x-subsurfaceimagedrop"); QDataStream dataStream(&itemData, QIODevice::ReadOnly); QString filename; - int diveId; - dataStream >> filename >> diveId; - - // If the id of the drag & dropped picture belongs to a different dive, then - // the offset we determine makes no sense what so ever. Simply ignore such an event. - // In the future, we might think about duplicating the picture or moving the picture - // from one dive to the other. - if (!current_dive || displayed_dive.id != diveId) { - event->ignore(); - return; - } - -#ifndef SUBSURFACE_MOBILE - // Calculate time in dive where picture was dropped and whether the new position is during the dive. + dataStream >> filename; 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; - - // 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 and is moved into the dive time. - // Add the picture to the profile. - // 2b) The image was not on the profile and is moved outside of the dive time. - // Do nothing. - 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. We only have to take action for - // the first case: picture is moved into dive-time. - if (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(), false); - updateThumbnailXPos(*newPos); - calculatePictureYPositions(); - } - } - - // Only signal the drag&drop action if the picture actually belongs to the dive. - DivePictureModel::instance()->updateDivePictureOffset(displayed_dive.id, filename, offset.seconds); -#endif + Command::setPictureOffset(current_dive, filename, offset); if (event->source() == this) { event->setDropAction(Qt::MoveAction); @@ -2265,7 +2196,73 @@ void ProfileWidget2::dropEvent(QDropEvent *event) } else { event->ignore(); } +#endif +} + +#ifndef SUBSURFACE_MOBILE +void ProfileWidget2::pictureOffsetChanged(dive *d, QString filename, offset_t offset) +{ + if (d->id != displayed_dive.id) + return; // Picture of a different dive than the one shown changed. + + // Calculate time in dive where picture was dropped and whether the new position is during the dive. + bool duringDive = current_dive && offset.seconds > 0 && offset.seconds < current_dive->duration.seconds; + + // 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 and is moved into the dive time. + // Add the picture to the profile. + // 2b) The image was not on the profile and is moved outside of the dive time. + // Do nothing. + 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. We only have to take action for + // the first case: picture is moved into dive-time. + if (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(), false); + updateThumbnailXPos(*newPos); + calculatePictureYPositions(); + } + } } +#endif void ProfileWidget2::dragEnterEvent(QDragEnterEvent *event) { diff --git a/profile-widget/profilewidget2.h b/profile-widget/profilewidget2.h index 96191a1d6..039424d87 100644 --- a/profile-widget/profilewidget2.h +++ b/profile-widget/profilewidget2.h @@ -117,6 +117,7 @@ slots: // Necessary to call from QAction's signals. void pointsRemoved(const QModelIndex &, int start, int end); void updateThumbnail(QString filename, QImage thumbnail, duration_t duration); void profileChanged(dive *d); + void pictureOffsetChanged(dive *d, QString filename, offset_t offset); /* this is called for every move on the handlers. maybe we can speed up this a bit? */ void recreatePlannedDive(); diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index 306030c4e..e50c51744 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -6,6 +6,7 @@ #include "core/imagedownloader.h" #include "core/picture.h" #include "core/qthelper.h" +#include "core/subsurface-qt/divelistnotifier.h" #include #include @@ -20,6 +21,8 @@ DivePictureModel::DivePictureModel() : zoomLevel(0.0) { connect(Thumbnailer::instance(), &Thumbnailer::thumbnailChanged, this, &DivePictureModel::updateThumbnail, Qt::QueuedConnection); + connect(&diveListNotifier, &DiveListNotifier::pictureOffsetChanged, + this, &DivePictureModel::pictureOffsetChanged); } void DivePictureModel::setZoomLevel(int level) @@ -212,13 +215,13 @@ void DivePictureModel::updateThumbnail(QString filename, QImage thumbnail, durat } } -void DivePictureModel::updateDivePictureOffset(int diveId, const QString &filenameIn, int offsetSeconds) +void DivePictureModel::pictureOffsetChanged(dive *d, const QString filenameIn, offset_t offset) { std::string filename = filenameIn.toStdString(); // 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; }); + auto from = std::find_if(pictures.begin(), pictures.end(), [d](const PictureEntry &e) { return e.diveId == d->id; }); + auto to = std::find_if(from, pictures.end(), [d](const PictureEntry &e) { return e.diveId != d->id; }); // Find picture with the given filename auto oldPos = std::find_if(from, to, [filename](const PictureEntry &e) { return e.filename == filename; }); @@ -226,20 +229,11 @@ void DivePictureModel::updateDivePictureOffset(int diveId, const QString &filena return; // Find new position - auto newPos = std::find_if(from, to, [offsetSeconds](const PictureEntry &e) { return e.offsetSeconds > offsetSeconds; }); + auto newPos = std::find_if(from, to, [offset](const PictureEntry &e) { return e.offsetSeconds > offset.seconds; }); // 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); - } + oldPos->offsetSeconds = offset.seconds; + copy_dive(current_dive, &displayed_dive); // TODO: remove once profile can display arbitrary dives // Henceforth we will work with indices instead of iterators int oldIndex = oldPos - pictures.begin(); diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h index 1ed5e8095..2d628f429 100644 --- a/qt-models/divepicturemodel.h +++ b/qt-models/divepicturemodel.h @@ -18,6 +18,7 @@ struct PictureEntry { duration_t length; }; +struct dive; class DivePictureModel : public QAbstractTableModel { Q_OBJECT public: @@ -27,12 +28,12 @@ public: int rowCount(const QModelIndex &parent = QModelIndex()) const override; void updateDivePictures(); void removePictures(const QVector &fileUrls); - void updateDivePictureOffset(int diveId, const QString &filename, int offsetSeconds); signals: void picturesRemoved(const QVector &fileUrls); 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); private: DivePictureModel(); QVector pictures; -- cgit v1.2.3-70-g09d2