From 6d74898f1757742f45ae789fde098782e62ae53d Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 28 Sep 2024 12:51:23 +0200 Subject: [PATCH 01/10] UndoRedo: Implement keyframe removal --- core_lib/src/interface/editor.cpp | 3 ++ core_lib/src/interface/undoredocommand.cpp | 59 ++++++++++++++++++++-- core_lib/src/interface/undoredocommand.h | 27 +++++++++- core_lib/src/managers/undoredomanager.cpp | 22 ++++++-- core_lib/src/managers/undoredomanager.h | 8 +-- 5 files changed, 106 insertions(+), 13 deletions(-) diff --git a/core_lib/src/interface/editor.cpp b/core_lib/src/interface/editor.cpp index d5aabb9acb..5d39460b8b 100644 --- a/core_lib/src/interface/editor.cpp +++ b/core_lib/src/interface/editor.cpp @@ -936,12 +936,15 @@ void Editor::removeKey() return; } + const UndoSaveState* state = undoRedo()->state(UndoRedoRecordType::KEYFRAME_REMOVE); backup(tr("Remove frame")); deselectAll(); layer->removeKeyFrame(currentFrame()); layers()->notifyAnimationLengthChanged(); emit layers()->currentLayerChanged(layers()->currentLayerIndex()); // trigger timeline repaint. + + undoRedo()->record(state, tr("Remove frame")); } void Editor::scrubNextKeyFrame() diff --git a/core_lib/src/interface/undoredocommand.cpp b/core_lib/src/interface/undoredocommand.cpp index 8439fd8c54..7887bcb224 100644 --- a/core_lib/src/interface/undoredocommand.cpp +++ b/core_lib/src/interface/undoredocommand.cpp @@ -24,6 +24,7 @@ GNU General Public License for more details. #include "layersound.h" #include "layerbitmap.h" #include "layervector.h" +#include "layer.h" #include "editor.h" #include "undoredocommand.h" @@ -38,6 +39,53 @@ UndoRedoCommand::~UndoRedoCommand() { } +KeyFrameRemoveCommand::KeyFrameRemoveCommand(const KeyFrame* undoKeyFrame, + int undoLayerId, + const QString &description, + Editor *editor, + QUndoCommand *parent) : UndoRedoCommand(editor, parent) +{ + this->undoKeyFrame = undoKeyFrame->clone(); + this->undoLayerId = undoLayerId; + + this->redoLayerId = editor->layers()->currentLayer()->id(); + this->redoPosition = editor->currentFrame(); + + setText(description); +} + +KeyFrameRemoveCommand::~KeyFrameRemoveCommand() +{ + delete undoKeyFrame; +} + +void KeyFrameRemoveCommand::undo() +{ + UndoRedoCommand::undo(); + + Layer* layer = editor()->layers()->findLayerById(undoLayerId); + + layer->addKeyFrame(undoKeyFrame->pos(), undoKeyFrame->clone()); + + emit editor()->frameModified(undoKeyFrame->pos()); + editor()->layers()->notifyAnimationLengthChanged(); + editor()->scrubTo(undoKeyFrame->pos()); +} + +void KeyFrameRemoveCommand::redo() +{ + UndoRedoCommand::redo(); + + if (isFirstRedo()) { setFirstRedo(false); return; } + + Layer* layer = editor()->layers()->findLayerById(redoLayerId); + layer->removeKeyFrame(redoPosition); + + emit editor()->frameModified(redoPosition); + editor()->layers()->notifyAnimationLengthChanged(); + editor()->scrubTo(redoPosition); +} + BitmapReplaceCommand::BitmapReplaceCommand(const BitmapImage* undoBitmap, const int undoLayerId, const QString& description, @@ -58,7 +106,7 @@ BitmapReplaceCommand::BitmapReplaceCommand(const BitmapImage* undoBitmap, void BitmapReplaceCommand::undo() { - QUndoCommand::undo(); + UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); static_cast(layer)->replaceKeyFrame(&undoBitmap); @@ -68,7 +116,7 @@ void BitmapReplaceCommand::undo() void BitmapReplaceCommand::redo() { - QUndoCommand::redo(); + UndoRedoCommand::redo(); // Ignore automatic redo when added to undo stack if (isFirstRedo()) { setFirstRedo(false); return; } @@ -98,7 +146,7 @@ VectorReplaceCommand::VectorReplaceCommand(const VectorImage* undoVector, void VectorReplaceCommand::undo() { - QUndoCommand::undo(); + UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); @@ -109,7 +157,7 @@ void VectorReplaceCommand::undo() void VectorReplaceCommand::redo() { - QUndoCommand::redo(); + UndoRedoCommand::redo(); // Ignore automatic redo when added to undo stack if (isFirstRedo()) { setFirstRedo(false); return; } @@ -154,6 +202,7 @@ TransformCommand::TransformCommand(const QRectF& undoSelectionRect, void TransformCommand::undo() { + UndoRedoCommand::undo(); apply(undoSelectionRect, undoTranslation, undoRotationAngle, @@ -165,6 +214,8 @@ void TransformCommand::undo() void TransformCommand::redo() { + UndoRedoCommand::redo(); + // Ignore automatic redo when added to undo stack if (isFirstRedo()) { setFirstRedo(false); return; } diff --git a/core_lib/src/interface/undoredocommand.h b/core_lib/src/interface/undoredocommand.h index 05c4b4766c..38504cf2df 100644 --- a/core_lib/src/interface/undoredocommand.h +++ b/core_lib/src/interface/undoredocommand.h @@ -24,13 +24,15 @@ GNU General Public License for more details. #include "bitmapimage.h" #include "vectorimage.h" +#include "soundclip.h" +#include "camera.h" +#include "layer.h" class Editor; class UndoRedoManager; class PreferenceManager; class SoundClip; class Camera; -class Layer; class KeyFrame; class TransformCommand; @@ -51,6 +53,29 @@ class UndoRedoCommand : public QUndoCommand bool mIsFirstRedo = true; }; +class KeyFrameRemoveCommand : public UndoRedoCommand +{ +public: + KeyFrameRemoveCommand(const KeyFrame* undoKeyFrame, + int undoLayerId, + const QString& description, + Editor* editor, + QUndoCommand* parent = nullptr + ); + ~KeyFrameRemoveCommand(); + + void undo() override; + void redo() override; + +private: + + int undoLayerId = 0; + int redoLayerId = 0; + + KeyFrame* undoKeyFrame = nullptr; + int redoPosition; +}; + class BitmapReplaceCommand : public UndoRedoCommand { diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 7dea4748fe..99567717e8 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -108,6 +108,10 @@ void UndoRedoManager::record(const UndoSaveState*& undoState, const QString& des replaceKeyFrame(*undoState, description); break; } + case UndoRedoRecordType::KEYFRAME_REMOVE: { + removeKeyFrame(*undoState, description); + break; + } default: { QString reason("Unhandled case for: "); reason.append(description); @@ -141,6 +145,15 @@ void UndoRedoManager::pushCommand(QUndoCommand* command) emit didUpdateUndoStack(); } +void UndoRedoManager::removeKeyFrame(const UndoSaveState& undoState, const QString& description) +{ + KeyFrameRemoveCommand* element = new KeyFrameRemoveCommand(undoState.keyframe.get(), + undoState.layerId, + description, + editor()); + pushCommand(element); +} + void UndoRedoManager::replaceKeyFrame(const UndoSaveState& undoState, const QString& description) { if (undoState.layerType == Layer::BITMAP) { @@ -204,18 +217,19 @@ const UndoSaveState* UndoRedoManager::state(UndoRedoRecordType recordType) const switch (recordType) { - case UndoRedoRecordType::KEYFRAME_MODIFY: { - return savedKeyFrameState(); + case UndoRedoRecordType::KEYFRAME_MODIFY: + case UndoRedoRecordType::KEYFRAME_REMOVE: { + return savedKeyFrameState(recordType); default: return nullptr; } } } -const UndoSaveState* UndoRedoManager::savedKeyFrameState() const +const UndoSaveState* UndoRedoManager::savedKeyFrameState(const UndoRedoRecordType& type) const { UndoSaveState* undoSaveState = new UndoSaveState(); - undoSaveState->recordType = UndoRedoRecordType::KEYFRAME_MODIFY; + undoSaveState->recordType = type; const Layer* layer = editor()->layers()->currentLayer(); undoSaveState->layerType = layer->type(); diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index 84f8d951d1..e66e816738 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -41,9 +41,7 @@ class UndoRedoCommand; /// The undo/redo type which correspond to what is being recorded enum class UndoRedoRecordType { KEYFRAME_MODIFY, // Any modification that involve a keyframe - - // Possible future actions - // KEYFRAME_REMOVE, // Removing a keyframe + KEYFRAME_REMOVE, // Removing a keyframe // KEYFRAME_ADD, // Adding a keyframe // SCRUB_LAYER, // Scrubbing layer // SCRUB_KEYFRAME, // Scrubbing keyframe @@ -156,7 +154,9 @@ class UndoRedoManager : public BaseManager void replaceBitmap(const UndoSaveState& undoState, const QString& description); void replaceVector(const UndoSaveState& undoState, const QString& description); - const UndoSaveState* savedKeyFrameState() const; + void removeKeyFrame(const UndoSaveState& undoState, const QString& description); + + const UndoSaveState* savedKeyFrameState(const UndoRedoRecordType& type) const; void pushCommand(QUndoCommand* command); From cf4b62ed9dc4c5b2224f2d0d661199869b12b725 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 28 Sep 2024 16:55:20 +0200 Subject: [PATCH 02/10] UndoRedo: Implement keyframe addition --- core_lib/src/interface/editor.cpp | 8 ++- core_lib/src/interface/undoredocommand.cpp | 71 ++++++++++++++++++++++ core_lib/src/interface/undoredocommand.h | 27 +++++++- core_lib/src/managers/layermanager.cpp | 2 +- core_lib/src/managers/layermanager.h | 2 +- core_lib/src/managers/undoredomanager.cpp | 17 +++++- core_lib/src/managers/undoredomanager.h | 4 +- 7 files changed, 124 insertions(+), 7 deletions(-) diff --git a/core_lib/src/interface/editor.cpp b/core_lib/src/interface/editor.cpp index 5d39460b8b..2cdee04056 100644 --- a/core_lib/src/interface/editor.cpp +++ b/core_lib/src/interface/editor.cpp @@ -914,9 +914,15 @@ KeyFrame* Editor::addKeyFrame(const int layerNumber, int frameIndex) const bool ok = layer->addNewKeyFrameAt(frameIndex); Q_ASSERT(ok); // We already ensured that there is no keyframe at frameIndex, so this should always succeed scrubTo(frameIndex); // currentFrameChanged() emit inside. + + const UndoSaveState* state = undoRedo()->state(UndoRedoRecordType::KEYFRAME_ADD); emit frameModified(frameIndex); layers()->notifyAnimationLengthChanged(); - return layer->getKeyFrameAt(frameIndex); + KeyFrame* newFrame = layer->getKeyFrameAt(frameIndex); + + undoRedo()->record(state, tr("Add frame")); + + return newFrame; } void Editor::removeKey() diff --git a/core_lib/src/interface/undoredocommand.cpp b/core_lib/src/interface/undoredocommand.cpp index 7887bcb224..317589f9e8 100644 --- a/core_lib/src/interface/undoredocommand.cpp +++ b/core_lib/src/interface/undoredocommand.cpp @@ -64,6 +64,11 @@ void KeyFrameRemoveCommand::undo() UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); + if (layer == nullptr) { + // Until we support layer deletion recovery, we mark the command as + // obsolete as soon as it's been + return setObsolete(true); + } layer->addKeyFrame(undoKeyFrame->pos(), undoKeyFrame->clone()); @@ -86,6 +91,61 @@ void KeyFrameRemoveCommand::redo() editor()->scrubTo(redoPosition); } +KeyFrameAddCommand::KeyFrameAddCommand(int undoPosition, + int undoLayerId, + const QString &description, + Editor *editor, + QUndoCommand *parent) + : UndoRedoCommand(editor, parent) +{ + this->undoPosition = undoPosition; + this->undoLayerId = undoLayerId; + + this->redoLayerId = editor->layers()->currentLayer()->id(); + this->redoPosition = editor->currentFrame(); + + setText(description); +} + +KeyFrameAddCommand::~KeyFrameAddCommand() +{ +} + +void KeyFrameAddCommand::undo() +{ + UndoRedoCommand::undo(); + + Layer* layer = editor()->layers()->findLayerById(undoLayerId); + if (layer == nullptr) { + // Until we support layer deletion recovery, we mark the command as + // obsolete as soon as it's been + return setObsolete(true); + } + + layer->removeKeyFrame(undoPosition); + + emit editor()->frameModified(undoPosition); + editor()->layers()->notifyAnimationLengthChanged(); + editor()->layers()->setCurrentLayer(layer); + editor()->scrubTo(undoPosition); +} + +void KeyFrameAddCommand::redo() +{ + UndoRedoCommand::redo(); + + // Ignore automatic redo when added to undo stack + if (isFirstRedo()) { setFirstRedo(false); return; } + + Layer* layer = editor()->layers()->findLayerById(redoLayerId); + layer->addNewKeyFrameAt(redoPosition); + + emit editor()->frameModified(redoPosition); + editor()->layers()->notifyAnimationLengthChanged(); + editor()->layers()->setCurrentLayer(layer); + editor()->scrubTo(redoPosition); +} + BitmapReplaceCommand::BitmapReplaceCommand(const BitmapImage* undoBitmap, const int undoLayerId, const QString& description, @@ -109,6 +169,12 @@ void BitmapReplaceCommand::undo() UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); + if (layer == nullptr) { + // Until we support layer deletion recovery, we mark the command as + // obsolete as soon as it's been + return setObsolete(true); + } + static_cast(layer)->replaceKeyFrame(&undoBitmap); editor()->scrubTo(undoBitmap.pos()); @@ -149,6 +215,11 @@ void VectorReplaceCommand::undo() UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); + if (layer == nullptr) { + // Until we support layer deletion recovery, we mark the command as + // obsolete as soon as it's been + return setObsolete(true); + } static_cast(layer)->replaceKeyFrame(&undoVector); diff --git a/core_lib/src/interface/undoredocommand.h b/core_lib/src/interface/undoredocommand.h index 38504cf2df..0ebf981459 100644 --- a/core_lib/src/interface/undoredocommand.h +++ b/core_lib/src/interface/undoredocommand.h @@ -43,7 +43,7 @@ class UndoRedoCommand : public QUndoCommand ~UndoRedoCommand() override; protected: - Editor* editor() { return mEditor; } + Editor* editor() const { return mEditor; } bool isFirstRedo() const { return mIsFirstRedo; } void setFirstRedo(const bool state) { mIsFirstRedo = state; } @@ -73,7 +73,30 @@ class KeyFrameRemoveCommand : public UndoRedoCommand int redoLayerId = 0; KeyFrame* undoKeyFrame = nullptr; - int redoPosition; + int redoPosition = 0; +}; + +class KeyFrameAddCommand : public UndoRedoCommand +{ +public: + KeyFrameAddCommand(int undoPosition, + int undoLayerId, + const QString& description, + Editor* editor, + QUndoCommand* parent = nullptr + ); + ~KeyFrameAddCommand(); + + void undo() override; + void redo() override; + +private: + + int undoLayerId = 0; + int redoLayerId = 0; + + int undoPosition = 0; + int redoPosition = 0; }; class BitmapReplaceCommand : public UndoRedoCommand diff --git a/core_lib/src/managers/layermanager.cpp b/core_lib/src/managers/layermanager.cpp index 5876937dac..12bb743a9f 100644 --- a/core_lib/src/managers/layermanager.cpp +++ b/core_lib/src/managers/layermanager.cpp @@ -100,7 +100,7 @@ Layer* LayerManager::findLayerByName(QString sName, Layer::LAYER_TYPE type) return object()->findLayerByName(sName, type); } -Layer* LayerManager::findLayerById(int layerId) +Layer* LayerManager::findLayerById(int layerId) const { return object()->findLayerById(layerId); } diff --git a/core_lib/src/managers/layermanager.h b/core_lib/src/managers/layermanager.h index 254ebedb7d..01cc52beb1 100644 --- a/core_lib/src/managers/layermanager.h +++ b/core_lib/src/managers/layermanager.h @@ -44,7 +44,7 @@ class LayerManager : public BaseManager Layer* getLayer(int index); LayerCamera* getCameraLayerBelow(int layerIndex) const; Layer* findLayerByName(QString sName, Layer::LAYER_TYPE type = Layer::UNDEFINED); - Layer* findLayerById(int layerId); + Layer* findLayerById(int layerId) const; Layer* getLastCameraLayer(); int currentLayerIndex(); void setCurrentLayer(int nIndex); diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 99567717e8..2e275aa180 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -112,6 +112,10 @@ void UndoRedoManager::record(const UndoSaveState*& undoState, const QString& des removeKeyFrame(*undoState, description); break; } + case UndoRedoRecordType::KEYFRAME_ADD: { + addKeyFrame(*undoState, description); + break; + } default: { QString reason("Unhandled case for: "); reason.append(description); @@ -154,6 +158,15 @@ void UndoRedoManager::removeKeyFrame(const UndoSaveState& undoState, const QStri pushCommand(element); } +void UndoRedoManager::addKeyFrame(const UndoSaveState& undoState, const QString& description) +{ + KeyFrameAddCommand* element = new KeyFrameAddCommand(undoState.currentFrameIndex, + undoState.layerId, + description, + editor()); + pushCommand(element); +} + void UndoRedoManager::replaceKeyFrame(const UndoSaveState& undoState, const QString& description) { if (undoState.layerType == Layer::BITMAP) { @@ -217,10 +230,11 @@ const UndoSaveState* UndoRedoManager::state(UndoRedoRecordType recordType) const switch (recordType) { + case UndoRedoRecordType::KEYFRAME_ADD: case UndoRedoRecordType::KEYFRAME_MODIFY: case UndoRedoRecordType::KEYFRAME_REMOVE: { return savedKeyFrameState(recordType); - default: + case UndoRedoRecordType::INVALID: return nullptr; } } @@ -234,6 +248,7 @@ const UndoSaveState* UndoRedoManager::savedKeyFrameState(const UndoRedoRecordTyp const Layer* layer = editor()->layers()->currentLayer(); undoSaveState->layerType = layer->type(); undoSaveState->layerId = layer->id(); + undoSaveState->currentFrameIndex = editor()->currentFrame(); if (layer->type() == Layer::BITMAP || layer->type() == Layer::VECTOR) { auto selectMan = editor()->select(); diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index e66e816738..1e237ec655 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -42,7 +42,7 @@ class UndoRedoCommand; enum class UndoRedoRecordType { KEYFRAME_MODIFY, // Any modification that involve a keyframe KEYFRAME_REMOVE, // Removing a keyframe - // KEYFRAME_ADD, // Adding a keyframe + KEYFRAME_ADD, // Adding a keyframe // SCRUB_LAYER, // Scrubbing layer // SCRUB_KEYFRAME, // Scrubbing keyframe INVALID @@ -77,6 +77,7 @@ struct SelectionSaveState { /// whatever states that needs to be stored temporarily. struct UndoSaveState { int layerId = 0; + int currentFrameIndex = 0; Layer::LAYER_TYPE layerType = Layer::UNDEFINED; std::unique_ptr keyframe = nullptr; @@ -154,6 +155,7 @@ class UndoRedoManager : public BaseManager void replaceBitmap(const UndoSaveState& undoState, const QString& description); void replaceVector(const UndoSaveState& undoState, const QString& description); + void addKeyFrame(const UndoSaveState& undoState, const QString& description); void removeKeyFrame(const UndoSaveState& undoState, const QString& description); const UndoSaveState* savedKeyFrameState(const UndoRedoRecordType& type) const; From 2bea9a11ad2fd1df0a7e9305a3284ce4a13fd7db Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 5 Oct 2024 11:55:27 +0200 Subject: [PATCH 03/10] UndoRedo: Implement move keyframes --- app/src/timelinecells.cpp | 8 ++- core_lib/src/interface/editor.cpp | 4 +- core_lib/src/interface/undoredocommand.cpp | 47 +++++++++++++ core_lib/src/interface/undoredocommand.h | 24 ++++++- core_lib/src/managers/undoredomanager.cpp | 81 +++++++++++----------- core_lib/src/managers/undoredomanager.h | 42 ++++++++--- core_lib/src/tool/movetool.cpp | 2 +- core_lib/src/tool/movetool.h | 2 +- core_lib/src/tool/polylinetool.cpp | 4 +- core_lib/src/tool/selecttool.cpp | 2 +- core_lib/src/tool/selecttool.h | 2 +- core_lib/src/tool/stroketool.cpp | 2 +- core_lib/src/tool/stroketool.h | 2 +- 13 files changed, 161 insertions(+), 61 deletions(-) diff --git a/app/src/timelinecells.cpp b/app/src/timelinecells.cpp index 720108a42a..be70b9e9d5 100644 --- a/app/src/timelinecells.cpp +++ b/app/src/timelinecells.cpp @@ -32,6 +32,7 @@ GNU General Public License for more details. #include "object.h" #include "playbackmanager.h" #include "preferencemanager.h" +#include "undoredomanager.h" #include "timeline.h" #include "cameracontextmenu.h" @@ -1066,8 +1067,13 @@ void TimeLineCells::mouseReleaseEvent(QMouseEvent* event) int posUnderCursor = getFrameNumber(mMousePressX); int offset = frameNumber - posUnderCursor; - currentLayer->moveSelectedFrames(offset); + if (currentLayer->canMoveSelectedFramesToOffset(offset)) { + UndoSaveState* state = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MOVE); + state->moveFramesState = MoveFramesSaveState(offset, currentLayer->selectedKeyFramesPositions()); + currentLayer->moveSelectedFrames(offset); + mEditor->undoRedo()->record(state, tr("Move Frames")); + } mEditor->layers()->notifyAnimationLengthChanged(); emit mEditor->framesModified(); updateContent(); diff --git a/core_lib/src/interface/editor.cpp b/core_lib/src/interface/editor.cpp index 2cdee04056..3cd084dd00 100644 --- a/core_lib/src/interface/editor.cpp +++ b/core_lib/src/interface/editor.cpp @@ -915,7 +915,7 @@ KeyFrame* Editor::addKeyFrame(const int layerNumber, int frameIndex) Q_ASSERT(ok); // We already ensured that there is no keyframe at frameIndex, so this should always succeed scrubTo(frameIndex); // currentFrameChanged() emit inside. - const UndoSaveState* state = undoRedo()->state(UndoRedoRecordType::KEYFRAME_ADD); + UndoSaveState* state = undoRedo()->createState(UndoRedoRecordType::KEYFRAME_ADD); emit frameModified(frameIndex); layers()->notifyAnimationLengthChanged(); KeyFrame* newFrame = layer->getKeyFrameAt(frameIndex); @@ -942,7 +942,7 @@ void Editor::removeKey() return; } - const UndoSaveState* state = undoRedo()->state(UndoRedoRecordType::KEYFRAME_REMOVE); + UndoSaveState* state = undoRedo()->createState(UndoRedoRecordType::KEYFRAME_REMOVE); backup(tr("Remove frame")); deselectAll(); diff --git a/core_lib/src/interface/undoredocommand.cpp b/core_lib/src/interface/undoredocommand.cpp index 317589f9e8..a4333a6773 100644 --- a/core_lib/src/interface/undoredocommand.cpp +++ b/core_lib/src/interface/undoredocommand.cpp @@ -146,6 +146,53 @@ void KeyFrameAddCommand::redo() editor()->scrubTo(redoPosition); } +MoveKeyFramesCommand::MoveKeyFramesCommand(int offset, + QList listOfPositions, + int undoLayerId, + const QString& description, + Editor* editor, + QUndoCommand *parent) + : UndoRedoCommand(editor, parent) +{ + this->frameOffset = offset; + this->positions = listOfPositions; + + this->undoLayerId = undoLayerId; + this->redoLayerId = editor->layers()->currentLayer()->id(); + + setText(description); +} + +void MoveKeyFramesCommand::undo() +{ + UndoRedoCommand::undo(); + + Layer* undoLayer = editor()->layers()->findLayerById(undoLayerId); + + if (!undoLayer) { return; } + + for (int position : qAsConst(positions)) { + undoLayer->moveKeyFrame(position + frameOffset, -frameOffset); + } + + emit editor()->framesModified(); +} + +void MoveKeyFramesCommand::redo() +{ + UndoRedoCommand::redo(); + + // Ignore automatic redo when added to undo stack + if (isFirstRedo()) { setFirstRedo(false); return; } + + Layer* redoLayer = editor()->layers()->findLayerById(redoLayerId); + + for (int position : qAsConst(positions)) { + redoLayer->moveKeyFrame(position, frameOffset); + } + + emit editor()->framesModified(); +} BitmapReplaceCommand::BitmapReplaceCommand(const BitmapImage* undoBitmap, const int undoLayerId, const QString& description, diff --git a/core_lib/src/interface/undoredocommand.h b/core_lib/src/interface/undoredocommand.h index 0ebf981459..26b4fcef74 100644 --- a/core_lib/src/interface/undoredocommand.h +++ b/core_lib/src/interface/undoredocommand.h @@ -83,8 +83,7 @@ class KeyFrameAddCommand : public UndoRedoCommand int undoLayerId, const QString& description, Editor* editor, - QUndoCommand* parent = nullptr - ); + QUndoCommand* parent = nullptr); ~KeyFrameAddCommand(); void undo() override; @@ -99,6 +98,27 @@ class KeyFrameAddCommand : public UndoRedoCommand int redoPosition = 0; }; +class MoveKeyFramesCommand : public UndoRedoCommand +{ +public: + MoveKeyFramesCommand(int offset, + QList listOfPositions, + int undoLayerId, + const QString& description, + Editor* editor, + QUndoCommand* parent = nullptr); + + void undo() override; + void redo() override; + +private: + int undoLayerId = 0; + int redoLayerId = 0; + + int frameOffset = 0; + QList positions; +}; + class BitmapReplaceCommand : public UndoRedoCommand { diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 2e275aa180..93de09192a 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -92,13 +92,15 @@ Status UndoRedoManager::save(Object* /*o*/) return Status::OK; } -void UndoRedoManager::record(const UndoSaveState*& undoState, const QString& description) +void UndoRedoManager::record(UndoSaveState*& undoState, const QString& description) { - if (!mNewBackupSystemEnabled) { + if (!undoState) { return; } - if (!undoState) { + if (!mNewBackupSystemEnabled) { + delete undoState; + undoState = nullptr; return; } @@ -116,6 +118,10 @@ void UndoRedoManager::record(const UndoSaveState*& undoState, const QString& des addKeyFrame(*undoState, description); break; } + case UndoRedoRecordType::KEYFRAME_MOVE: { + moveKeyFrames(*undoState, description); + break; + } default: { QString reason("Unhandled case for: "); reason.append(description); @@ -178,6 +184,16 @@ void UndoRedoManager::replaceKeyFrame(const UndoSaveState& undoState, const QStr } } +void UndoRedoManager::moveKeyFrames(const UndoSaveState& undoState, const QString& description) +{ + const MoveFramesSaveState& state = undoState.moveFramesState; + MoveKeyFramesCommand* element = new MoveKeyFramesCommand(state.offset, + state.positions, + undoState.layerId, + description, + editor()); + pushCommand(element); +} void UndoRedoManager::replaceBitmap(const UndoSaveState& undoState, const QString& description) { @@ -187,13 +203,13 @@ void UndoRedoManager::replaceBitmap(const UndoSaveState& undoState, const QStrin description, editor()); - const SelectionSaveState* selectionState = undoState.selectionState.get(); - new TransformCommand(selectionState->bounds, - selectionState->translation, - selectionState->rotationAngle, - selectionState->scaleX, - selectionState->scaleY, - selectionState->anchor, + const SelectionSaveState& selectionState = undoState.selectionState; + new TransformCommand(selectionState.bounds, + selectionState.translation, + selectionState.rotationAngle, + selectionState.scaleX, + selectionState.scaleY, + selectionState.anchor, true, // roundPixels description, editor(), element); @@ -209,42 +225,30 @@ void UndoRedoManager::replaceVector(const UndoSaveState& undoState, const QStrin description, editor()); - const SelectionSaveState* selectionState = undoState.selectionState.get(); - new TransformCommand(selectionState->bounds, - selectionState->translation, - selectionState->rotationAngle, - selectionState->scaleX, - selectionState->scaleY, - selectionState->anchor, + const SelectionSaveState& selectionState = undoState.selectionState; + new TransformCommand(selectionState.bounds, + selectionState.translation, + selectionState.rotationAngle, + selectionState.scaleX, + selectionState.scaleY, + selectionState.anchor, false, // Round pixels description, editor(), element); pushCommand(element); } -const UndoSaveState* UndoRedoManager::state(UndoRedoRecordType recordType) const +UndoSaveState* UndoRedoManager::createState(UndoRedoRecordType recordType) const { - if (!mNewBackupSystemEnabled) { - return nullptr; - } + UndoSaveState* undoSaveState = new UndoSaveState(); + undoSaveState->recordType = recordType; + initCommonKeyFrameState(undoSaveState); - switch (recordType) - { - case UndoRedoRecordType::KEYFRAME_ADD: - case UndoRedoRecordType::KEYFRAME_MODIFY: - case UndoRedoRecordType::KEYFRAME_REMOVE: { - return savedKeyFrameState(recordType); - case UndoRedoRecordType::INVALID: - return nullptr; - } - } + return undoSaveState; } -const UndoSaveState* UndoRedoManager::savedKeyFrameState(const UndoRedoRecordType& type) const +void UndoRedoManager::initCommonKeyFrameState(UndoSaveState* undoSaveState) const { - UndoSaveState* undoSaveState = new UndoSaveState(); - undoSaveState->recordType = type; - const Layer* layer = editor()->layers()->currentLayer(); undoSaveState->layerType = layer->type(); undoSaveState->layerId = layer->id(); @@ -252,14 +256,13 @@ const UndoSaveState* UndoRedoManager::savedKeyFrameState(const UndoRedoRecordTyp if (layer->type() == Layer::BITMAP || layer->type() == Layer::VECTOR) { auto selectMan = editor()->select(); - undoSaveState->selectionState = std::unique_ptr( new SelectionSaveState( + undoSaveState->selectionState = SelectionSaveState( selectMan->mySelectionRect(), selectMan->myRotation(), selectMan->myScaleX(), selectMan->myScaleY(), selectMan->myTranslation(), - selectMan->currentTransformAnchor()) - ); + selectMan->currentTransformAnchor()); } const int frameIndex = editor()->currentFrame(); @@ -271,8 +274,6 @@ const UndoSaveState* UndoRedoManager::savedKeyFrameState(const UndoRedoRecordTyp { undoSaveState->keyframe = std::unique_ptr(layer->getKeyFrameWhichCovers(frameIndex)->clone()); } - - return undoSaveState; } QAction* UndoRedoManager::createUndoAction(QObject* parent, const QIcon& icon) diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index 1e237ec655..4df1e28980 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -43,6 +43,7 @@ enum class UndoRedoRecordType { KEYFRAME_MODIFY, // Any modification that involve a keyframe KEYFRAME_REMOVE, // Removing a keyframe KEYFRAME_ADD, // Adding a keyframe + KEYFRAME_MOVE, // SCRUB_LAYER, // Scrubbing layer // SCRUB_KEYFRAME, // Scrubbing keyframe INVALID @@ -50,6 +51,7 @@ enum class UndoRedoRecordType { struct SelectionSaveState { + SelectionSaveState() { } SelectionSaveState(const QRectF& rect, const qreal rotationAngle, const qreal scaleX, @@ -73,17 +75,40 @@ struct SelectionSaveState { QPointF anchor; }; + +struct MoveFramesSaveState { + + MoveFramesSaveState() { } + MoveFramesSaveState(int offset, + const QList& positions) + { + this->offset = offset; + this->positions = positions; + } + + int offset = 0; + QList positions; +}; + /// This is the main undo/redo state structure which is meant to populate /// whatever states that needs to be stored temporarily. struct UndoSaveState { + + ~UndoSaveState() + { + keyframe.reset(); + } + + // Common data + UndoRedoRecordType recordType = UndoRedoRecordType::INVALID; int layerId = 0; int currentFrameIndex = 0; Layer::LAYER_TYPE layerType = Layer::UNDEFINED; - std::unique_ptr keyframe = nullptr; - std::unique_ptr selectionState = nullptr; + SelectionSaveState selectionState = {}; + // - UndoRedoRecordType recordType = UndoRedoRecordType::INVALID; + MoveFramesSaveState moveFramesState = {}; }; class UndoRedoManager : public BaseManager @@ -103,16 +128,16 @@ class UndoRedoManager : public BaseManager * @param undoState The state to record. * @param description The description that will bound to the undo/redo action. */ - void record(const UndoSaveState*& undoState, const QString& description); + void record(UndoSaveState*& undoState, const QString& description); /** Checks whether there are unsaved changes. * @return true if there are unsaved changes, otherwise false */ bool hasUnsavedChanges() const; - /** Prepares and returns a save state with the given scope. - * @return A struct with state of the given record type */ - const UndoSaveState* state(UndoRedoRecordType recordType) const; + /** Prepares and returns an save state with common data + * @return A UndoSaveState struct with common keyframe data */ + UndoSaveState* createState(UndoRedoRecordType recordType) const; QAction* createUndoAction(QObject* parent, const QIcon& icon); QAction* createRedoAction(QObject* parent, const QIcon& icon); @@ -157,8 +182,9 @@ class UndoRedoManager : public BaseManager void addKeyFrame(const UndoSaveState& undoState, const QString& description); void removeKeyFrame(const UndoSaveState& undoState, const QString& description); + void moveKeyFrames(const UndoSaveState& undoState, const QString& description); - const UndoSaveState* savedKeyFrameState(const UndoRedoRecordType& type) const; + void initCommonKeyFrameState(UndoSaveState* undoSaveState) const; void pushCommand(QUndoCommand* command); diff --git a/core_lib/src/tool/movetool.cpp b/core_lib/src/tool/movetool.cpp index 56876aa696..92d2bac4a7 100644 --- a/core_lib/src/tool/movetool.cpp +++ b/core_lib/src/tool/movetool.cpp @@ -212,7 +212,7 @@ void MoveTool::beginInteraction(const QPointF& pos, Qt::KeyboardModifiers keyMod QRectF selectionRect = selectMan->mySelectionRect(); if (!selectionRect.isNull()) { - mUndoSaveState = mEditor->undoRedo()->state(UndoRedoRecordType::KEYFRAME_MODIFY); + mUndoSaveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); mEditor->backup(typeName()); } diff --git a/core_lib/src/tool/movetool.h b/core_lib/src/tool/movetool.h index 6eb16041f7..d7dd82c710 100644 --- a/core_lib/src/tool/movetool.h +++ b/core_lib/src/tool/movetool.h @@ -67,7 +67,7 @@ class MoveTool : public BaseTool MoveMode mPerspMode; QPointF mOffset; - const UndoSaveState* mUndoSaveState = nullptr; + UndoSaveState* mUndoSaveState = nullptr; }; #endif diff --git a/core_lib/src/tool/polylinetool.cpp b/core_lib/src/tool/polylinetool.cpp index 97dd109248..ad0ad2458b 100644 --- a/core_lib/src/tool/polylinetool.cpp +++ b/core_lib/src/tool/polylinetool.cpp @@ -207,7 +207,7 @@ void PolylineTool::pointerDoubleClickEvent(PointerEvent* event) // include the current point before ending the line. mPoints << getCurrentPoint(); - const UndoSaveState* saveState = mEditor->undoRedo()->state(UndoRedoRecordType::KEYFRAME_MODIFY); + UndoSaveState* saveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); mEditor->backup(typeName()); endPolyline(mPoints); @@ -243,7 +243,7 @@ bool PolylineTool::keyPressEvent(QKeyEvent* event) case Qt::Key_Return: if (mPoints.size() > 0) { - const UndoSaveState* saveState = mEditor->undoRedo()->state(UndoRedoRecordType::KEYFRAME_MODIFY); + UndoSaveState* saveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); endPolyline(mPoints); mEditor->undoRedo()->record(saveState, typeName()); return true; diff --git a/core_lib/src/tool/selecttool.cpp b/core_lib/src/tool/selecttool.cpp index 87e36fcfbc..e5a1e54321 100644 --- a/core_lib/src/tool/selecttool.cpp +++ b/core_lib/src/tool/selecttool.cpp @@ -126,7 +126,7 @@ void SelectTool::pointerPressEvent(PointerEvent* event) if (event->button() != Qt::LeftButton) { return; } auto selectMan = mEditor->select(); - mUndoState = mEditor->undoRedo()->state(UndoRedoRecordType::KEYFRAME_MODIFY); + mUndoState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); mPressPoint = event->canvasPos(); selectMan->setMoveModeForAnchorInRange(mPressPoint); diff --git a/core_lib/src/tool/selecttool.h b/core_lib/src/tool/selecttool.h index 09aa833137..f0450f99d0 100644 --- a/core_lib/src/tool/selecttool.h +++ b/core_lib/src/tool/selecttool.h @@ -69,7 +69,7 @@ class SelectTool : public BaseTool QPixmap mCursorPixmap = QPixmap(24, 24); - const UndoSaveState* mUndoState = nullptr; + UndoSaveState* mUndoState = nullptr; }; #endif diff --git a/core_lib/src/tool/stroketool.cpp b/core_lib/src/tool/stroketool.cpp index 108788bb8b..b0d93a878e 100644 --- a/core_lib/src/tool/stroketool.cpp +++ b/core_lib/src/tool/stroketool.cpp @@ -140,7 +140,7 @@ void StrokeTool::startStroke(PointerEvent::InputType inputType) mStrokePressures << mInterpolator.getPressure(); mCurrentInputType = inputType; - mUndoSaveState = mEditor->undoRedo()->state(UndoRedoRecordType::KEYFRAME_MODIFY); + mUndoSaveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); disableCoalescing(); } diff --git a/core_lib/src/tool/stroketool.h b/core_lib/src/tool/stroketool.h index 2c01b8298c..83c5dcff2c 100644 --- a/core_lib/src/tool/stroketool.h +++ b/core_lib/src/tool/stroketool.h @@ -112,7 +112,7 @@ public slots: StrokeInterpolator mInterpolator; - const UndoSaveState* mUndoSaveState = nullptr; + UndoSaveState* mUndoSaveState = nullptr; private: /// Sets the width value without calling settings to store the state From b527ab343a4057740b55a23755f6820d1635a7d9 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 19 Oct 2024 11:40:23 +0200 Subject: [PATCH 04/10] Refine undo state management To avoid memory leaks when calling createState too many times, without applying the state --- core_lib/src/managers/undoredomanager.cpp | 30 ++++++++++++++--------- core_lib/src/managers/undoredomanager.h | 7 ++++-- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 93de09192a..b15393e148 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -92,15 +92,14 @@ Status UndoRedoManager::save(Object* /*o*/) return Status::OK; } -void UndoRedoManager::record(UndoSaveState*& undoState, const QString& description) +void UndoRedoManager::record(const UndoSaveState* undoState, const QString& description) { if (!undoState) { return; } - if (!mNewBackupSystemEnabled) { - delete undoState; - undoState = nullptr; + if (!mNewBackupSystemEnabled && undoState) { + clearCurrentState(); return; } @@ -132,8 +131,15 @@ void UndoRedoManager::record(UndoSaveState*& undoState, const QString& descripti // The save state has now been used and should be invalidated so we can't use it again. - delete undoState; - undoState = nullptr; + clearCurrentState(); +} + +void UndoRedoManager::clearCurrentState() +{ + if (mCurrentState) { + delete mCurrentState; + mCurrentState = nullptr; + } } bool UndoRedoManager::hasUnsavedChanges() const @@ -238,13 +244,15 @@ void UndoRedoManager::replaceVector(const UndoSaveState& undoState, const QStrin pushCommand(element); } -UndoSaveState* UndoRedoManager::createState(UndoRedoRecordType recordType) const +UndoSaveState* UndoRedoManager::createState(UndoRedoRecordType recordType) { - UndoSaveState* undoSaveState = new UndoSaveState(); - undoSaveState->recordType = recordType; - initCommonKeyFrameState(undoSaveState); + clearCurrentState(); + + mCurrentState = new UndoSaveState(); + mCurrentState->recordType = recordType; + initCommonKeyFrameState(mCurrentState); - return undoSaveState; + return mCurrentState; } void UndoRedoManager::initCommonKeyFrameState(UndoSaveState* undoSaveState) const diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index 4df1e28980..c3abd5488e 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -128,7 +128,7 @@ class UndoRedoManager : public BaseManager * @param undoState The state to record. * @param description The description that will bound to the undo/redo action. */ - void record(UndoSaveState*& undoState, const QString& description); + void record(const UndoSaveState* undoState, const QString& description); /** Checks whether there are unsaved changes. @@ -137,7 +137,7 @@ class UndoRedoManager : public BaseManager /** Prepares and returns an save state with common data * @return A UndoSaveState struct with common keyframe data */ - UndoSaveState* createState(UndoRedoRecordType recordType) const; + UndoSaveState* createState(UndoRedoRecordType recordType); QAction* createUndoAction(QObject* parent, const QIcon& icon); QAction* createRedoAction(QObject* parent, const QIcon& icon); @@ -188,9 +188,12 @@ class UndoRedoManager : public BaseManager void pushCommand(QUndoCommand* command); + void clearCurrentState(); + void legacyUndo(); void legacyRedo(); + UndoSaveState* mCurrentState = nullptr; QUndoStack mUndoStack; // Legacy system From 57924f6c884a9cc21b6cec1383270543a43af484 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 19 Oct 2024 12:00:45 +0200 Subject: [PATCH 05/10] Fix crash when trying to undo/redo on a layer that has been deleted This is a temporary issue that will be obsolete once we support restoring the layer state --- core_lib/src/interface/undoredocommand.cpp | 31 +++++++++++++++------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/core_lib/src/interface/undoredocommand.cpp b/core_lib/src/interface/undoredocommand.cpp index a4333a6773..807e6441d5 100644 --- a/core_lib/src/interface/undoredocommand.cpp +++ b/core_lib/src/interface/undoredocommand.cpp @@ -116,9 +116,7 @@ void KeyFrameAddCommand::undo() UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); - if (layer == nullptr) { - // Until we support layer deletion recovery, we mark the command as - // obsolete as soon as it's been + if (!layer) { return setObsolete(true); } @@ -138,6 +136,10 @@ void KeyFrameAddCommand::redo() if (isFirstRedo()) { setFirstRedo(false); return; } Layer* layer = editor()->layers()->findLayerById(redoLayerId); + if (!layer) { + return setObsolete(true); + } + layer->addNewKeyFrameAt(redoPosition); emit editor()->frameModified(redoPosition); @@ -169,7 +171,9 @@ void MoveKeyFramesCommand::undo() Layer* undoLayer = editor()->layers()->findLayerById(undoLayerId); - if (!undoLayer) { return; } + if (!undoLayer) { + return setObsolete(true); + } for (int position : qAsConst(positions)) { undoLayer->moveKeyFrame(position + frameOffset, -frameOffset); @@ -187,6 +191,10 @@ void MoveKeyFramesCommand::redo() Layer* redoLayer = editor()->layers()->findLayerById(redoLayerId); + if (!redoLayer) { + return setObsolete(true); + } + for (int position : qAsConst(positions)) { redoLayer->moveKeyFrame(position, frameOffset); } @@ -216,9 +224,7 @@ void BitmapReplaceCommand::undo() UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); - if (layer == nullptr) { - // Until we support layer deletion recovery, we mark the command as - // obsolete as soon as it's been + if (!layer) { return setObsolete(true); } @@ -235,6 +241,10 @@ void BitmapReplaceCommand::redo() if (isFirstRedo()) { setFirstRedo(false); return; } Layer* layer = editor()->layers()->findLayerById(redoLayerId); + if (!layer) { + return setObsolete(true); + } + static_cast(layer)->replaceKeyFrame(&redoBitmap); editor()->scrubTo(redoBitmap.pos()); @@ -262,9 +272,7 @@ void VectorReplaceCommand::undo() UndoRedoCommand::undo(); Layer* layer = editor()->layers()->findLayerById(undoLayerId); - if (layer == nullptr) { - // Until we support layer deletion recovery, we mark the command as - // obsolete as soon as it's been + if (!layer) { return setObsolete(true); } @@ -281,6 +289,9 @@ void VectorReplaceCommand::redo() if (isFirstRedo()) { setFirstRedo(false); return; } Layer* layer = editor()->layers()->findLayerById(redoLayerId); + if (!layer) { + return setObsolete(true); + } static_cast(layer)->replaceKeyFrame(&redoVector); From 9373949c040ff3007b50a225c8e04cad57c47be8 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Fri, 28 Feb 2025 08:10:26 +0100 Subject: [PATCH 06/10] Fix silly mistake So we can set the input ptr to null again --- core_lib/src/managers/undoredomanager.cpp | 11 ++++++----- core_lib/src/managers/undoredomanager.h | 13 +++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 576247f870..7d8cfeeec5 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -92,14 +92,14 @@ Status UndoRedoManager::save(Object* /*o*/) return Status::OK; } -void UndoRedoManager::record(const UndoSaveState* undoState, const QString& description) +void UndoRedoManager::record(UndoSaveState*& undoState, const QString& description) { if (!undoState) { return; } if (!mNewBackupSystemEnabled && undoState) { - clearCurrentState(); + clearState(undoState); return; } @@ -131,14 +131,15 @@ void UndoRedoManager::record(const UndoSaveState* undoState, const QString& desc // The save state has now been used and should be invalidated so we can't use it again. - clearCurrentState(); + clearState(undoState); } -void UndoRedoManager::clearCurrentState() +void UndoRedoManager::clearState(UndoSaveState*& state) { if (mCurrentState) { delete mCurrentState; mCurrentState = nullptr; + state = nullptr; } } @@ -246,7 +247,7 @@ void UndoRedoManager::replaceVector(const UndoSaveState& undoState, const QStrin UndoSaveState* UndoRedoManager::createState(UndoRedoRecordType recordType) { - clearCurrentState(); + clearState(mCurrentState); mCurrentState = new UndoSaveState(); mCurrentState->recordType = recordType; diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index c3abd5488e..d2f4385a13 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -96,7 +96,9 @@ struct UndoSaveState { ~UndoSaveState() { - keyframe.reset(); + if (recordType != UndoRedoRecordType::INVALID) { + keyframe.reset(); + } } // Common data @@ -104,10 +106,9 @@ struct UndoSaveState { int layerId = 0; int currentFrameIndex = 0; Layer::LAYER_TYPE layerType = Layer::UNDEFINED; - std::unique_ptr keyframe = nullptr; - SelectionSaveState selectionState = {}; - // + std::unique_ptr keyframe; + SelectionSaveState selectionState = {}; MoveFramesSaveState moveFramesState = {}; }; @@ -128,7 +129,7 @@ class UndoRedoManager : public BaseManager * @param undoState The state to record. * @param description The description that will bound to the undo/redo action. */ - void record(const UndoSaveState* undoState, const QString& description); + void record(UndoSaveState*& undoState, const QString& description); /** Checks whether there are unsaved changes. @@ -188,7 +189,7 @@ class UndoRedoManager : public BaseManager void pushCommand(QUndoCommand* command); - void clearCurrentState(); + void clearState(UndoSaveState*& state); void legacyUndo(); void legacyRedo(); From 86eb579387ff7a5f912bffb96fe1794abb5f451b Mon Sep 17 00:00:00 2001 From: MrStevns Date: Fri, 26 Sep 2025 18:28:26 +0200 Subject: [PATCH 07/10] Fix crash due to wrong state being assumed when deleted The scenario is you call create state from - Move tool, move selection - Meanwhile you move the keyframe to a different layer - You then deselect which creates a new keyframe due to no keyframe being there - this deletes mCurrentState but that is not the same ptr as the one in MoveTool.. as such it is now dangling - Then we get into MoveTool::mouseReleaseEvent which tries to create a record of the undoState but that has now become garbage. Now it is only the caller who can clear the ptr rather, although it is still handled automatically due to record --- core_lib/src/managers/undoredomanager.cpp | 15 ++++++--------- core_lib/src/managers/undoredomanager.h | 1 - 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 7d8cfeeec5..a00508e87b 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -136,9 +136,8 @@ void UndoRedoManager::record(UndoSaveState*& undoState, const QString& descripti void UndoRedoManager::clearState(UndoSaveState*& state) { - if (mCurrentState) { - delete mCurrentState; - mCurrentState = nullptr; + if (state) { + delete state; state = nullptr; } } @@ -247,13 +246,11 @@ void UndoRedoManager::replaceVector(const UndoSaveState& undoState, const QStrin UndoSaveState* UndoRedoManager::createState(UndoRedoRecordType recordType) { - clearState(mCurrentState); + UndoSaveState* state = new UndoSaveState(); + state->recordType = recordType; + initCommonKeyFrameState(state); - mCurrentState = new UndoSaveState(); - mCurrentState->recordType = recordType; - initCommonKeyFrameState(mCurrentState); - - return mCurrentState; + return state; } void UndoRedoManager::initCommonKeyFrameState(UndoSaveState* undoSaveState) const diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index d2f4385a13..e4956d4c6c 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -194,7 +194,6 @@ class UndoRedoManager : public BaseManager void legacyUndo(); void legacyRedo(); - UndoSaveState* mCurrentState = nullptr; QUndoStack mUndoStack; // Legacy system From 3c451f3a5ec84d625e0d05814dab8e98d75a28f7 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 27 Sep 2025 12:59:28 +0200 Subject: [PATCH 08/10] Introduce SAVESTATE_ID to deal with potential corrupt save state The mCurrentState was flawed in the way that it assumed you would always have a createState followed by record(...) which is not always the case... the scenario i ran into was: creating a selection and then moving some keyframes, leaving the scrubbed position with no keyframe, this would then call createState, deleting the current mCurrentState in the manager (and leaving the one stored in MoveTool dangling) which when you then tried to apply the selection, it would crash because the UndoSaveState had been deleted memory wise but the ptr wouldn't know that. The solution i came up with to fix this is that we instead of getting access to the raw ptr, we get a handle now called SAVESTATE_ID. The id is used to fetch a given saveState stored temporarily in undoRedoManager. This now also means that we can create multiple save states and apply all of them without having to think about dealing with their lifetime. In order to still allow the caller to add additional state to the record, I have also introduced a new struct called UserSaveState which is meant to work as a mega struct in the way that it can store all kinds of data but you should only add the data you actually need. --- app/src/timelinecells.cpp | 8 ++-- core_lib/src/interface/editor.cpp | 8 ++-- core_lib/src/managers/undoredomanager.cpp | 47 ++++++++++++++++------- core_lib/src/managers/undoredomanager.h | 35 ++++++++++++++--- core_lib/src/tool/movetool.cpp | 4 +- core_lib/src/tool/movetool.h | 2 +- core_lib/src/tool/polylinetool.cpp | 8 ++-- core_lib/src/tool/selecttool.cpp | 4 +- core_lib/src/tool/selecttool.h | 2 +- core_lib/src/tool/stroketool.cpp | 4 +- core_lib/src/tool/stroketool.h | 2 +- 11 files changed, 84 insertions(+), 40 deletions(-) diff --git a/app/src/timelinecells.cpp b/app/src/timelinecells.cpp index 79ebe16aec..8cb57985fa 100644 --- a/app/src/timelinecells.cpp +++ b/app/src/timelinecells.cpp @@ -1067,11 +1067,13 @@ void TimeLineCells::mouseReleaseEvent(QMouseEvent* event) int offset = frameNumber - posUnderCursor; if (currentLayer->canMoveSelectedFramesToOffset(offset)) { - UndoSaveState* state = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MOVE); - state->moveFramesState = MoveFramesSaveState(offset, currentLayer->selectedKeyFramesPositions()); + SAVESTATE_ID saveStateId = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MOVE); + UserSaveState userState; + userState.moveFramesState = MoveFramesSaveState(offset, currentLayer->selectedKeyFramesPositions()); + mEditor->undoRedo()->addUserState(saveStateId, userState); currentLayer->moveSelectedFrames(offset); - mEditor->undoRedo()->record(state, tr("Move Frames")); + mEditor->undoRedo()->record(saveStateId, tr("Move Frames")); } mEditor->layers()->notifyAnimationLengthChanged(); emit mEditor->framesModified(); diff --git a/core_lib/src/interface/editor.cpp b/core_lib/src/interface/editor.cpp index 9f3bc5ee07..696b041904 100644 --- a/core_lib/src/interface/editor.cpp +++ b/core_lib/src/interface/editor.cpp @@ -935,12 +935,12 @@ KeyFrame* Editor::addKeyFrame(const int layerNumber, int frameIndex) Q_ASSERT(ok); // We already ensured that there is no keyframe at frameIndex, so this should always succeed scrubTo(frameIndex); // currentFrameChanged() emit inside. - UndoSaveState* state = undoRedo()->createState(UndoRedoRecordType::KEYFRAME_ADD); + SAVESTATE_ID saveStateId = undoRedo()->createState(UndoRedoRecordType::KEYFRAME_ADD); emit frameModified(frameIndex); layers()->notifyAnimationLengthChanged(); KeyFrame* newFrame = layer->getKeyFrameAt(frameIndex); - undoRedo()->record(state, tr("Add frame")); + undoRedo()->record(saveStateId, tr("Add frame")); return newFrame; } @@ -962,7 +962,7 @@ void Editor::removeKey() return; } - UndoSaveState* state = undoRedo()->createState(UndoRedoRecordType::KEYFRAME_REMOVE); + SAVESTATE_ID saveStateId = undoRedo()->createState(UndoRedoRecordType::KEYFRAME_REMOVE); backup(tr("Remove frame")); deselectAll(); @@ -970,7 +970,7 @@ void Editor::removeKey() layers()->notifyAnimationLengthChanged(); emit layers()->currentLayerChanged(layers()->currentLayerIndex()); // trigger timeline repaint. - undoRedo()->record(state, tr("Remove frame")); + undoRedo()->record(saveStateId, tr("Remove frame")); } void Editor::scrubNextKeyFrame() diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index a00508e87b..09df7a24c6 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -53,6 +53,7 @@ UndoRedoManager::~UndoRedoManager() { clearStack(); } + clearSaveStates(); qDebug() << "UndoRedoManager: destroyed"; } @@ -92,33 +93,35 @@ Status UndoRedoManager::save(Object* /*o*/) return Status::OK; } -void UndoRedoManager::record(UndoSaveState*& undoState, const QString& description) +void UndoRedoManager::record(SAVESTATE_ID saveStateId, const QString& description) { - if (!undoState) { + if (!mSaveStates.contains(saveStateId)) { return; } - if (!mNewBackupSystemEnabled && undoState) { - clearState(undoState); + UndoSaveState* saveState = mSaveStates.take(saveStateId); + + if (!mNewBackupSystemEnabled && saveState) { + clearState(saveState); return; } - switch (undoState->recordType) + switch (saveState->recordType) { case UndoRedoRecordType::KEYFRAME_MODIFY: { - replaceKeyFrame(*undoState, description); + replaceKeyFrame(*saveState, description); break; } case UndoRedoRecordType::KEYFRAME_REMOVE: { - removeKeyFrame(*undoState, description); + removeKeyFrame(*saveState, description); break; } case UndoRedoRecordType::KEYFRAME_ADD: { - addKeyFrame(*undoState, description); + addKeyFrame(*saveState, description); break; } case UndoRedoRecordType::KEYFRAME_MOVE: { - moveKeyFrames(*undoState, description); + moveKeyFrames(*saveState, description); break; } default: { @@ -129,9 +132,8 @@ void UndoRedoManager::record(UndoSaveState*& undoState, const QString& descripti } } - // The save state has now been used and should be invalidated so we can't use it again. - clearState(undoState); + clearState(saveState); } void UndoRedoManager::clearState(UndoSaveState*& state) @@ -142,6 +144,13 @@ void UndoRedoManager::clearState(UndoSaveState*& state) } } +void UndoRedoManager::clearSaveStates() +{ + if (mSaveStates.isEmpty()) { return; } + + mSaveStates.clear(); +} + bool UndoRedoManager::hasUnsavedChanges() const { if (mNewBackupSystemEnabled) { @@ -192,7 +201,7 @@ void UndoRedoManager::replaceKeyFrame(const UndoSaveState& undoState, const QStr void UndoRedoManager::moveKeyFrames(const UndoSaveState& undoState, const QString& description) { - const MoveFramesSaveState& state = undoState.moveFramesState; + const MoveFramesSaveState& state = undoState.userState.moveFramesState; MoveKeyFramesCommand* element = new MoveKeyFramesCommand(state.offset, state.positions, undoState.layerId, @@ -244,13 +253,23 @@ void UndoRedoManager::replaceVector(const UndoSaveState& undoState, const QStrin pushCommand(element); } -UndoSaveState* UndoRedoManager::createState(UndoRedoRecordType recordType) +SAVESTATE_ID UndoRedoManager::createState(UndoRedoRecordType recordType) { + int saveStateId = mSaveStateId; UndoSaveState* state = new UndoSaveState(); state->recordType = recordType; initCommonKeyFrameState(state); - return state; + mSaveStates[mSaveStateId] = state; + mSaveStateId += 1; + + return saveStateId; +} + +void UndoRedoManager::addUserState(SAVESTATE_ID saveStateId, UserSaveState userState) +{ + if (!mSaveStates.contains(saveStateId)) { return; } + mSaveStates[saveStateId]->userState = userState; } void UndoRedoManager::initCommonKeyFrameState(UndoSaveState* undoSaveState) const diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index e4956d4c6c..8dae77736a 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -26,6 +26,7 @@ GNU General Public License for more details. #include #include +#include class QAction; class QUndoCommand; @@ -38,6 +39,8 @@ class KeyFrame; class LegacyBackupElement; class UndoRedoCommand; +typedef int SAVESTATE_ID; + /// The undo/redo type which correspond to what is being recorded enum class UndoRedoRecordType { KEYFRAME_MODIFY, // Any modification that involve a keyframe @@ -75,7 +78,6 @@ struct SelectionSaveState { QPointF anchor; }; - struct MoveFramesSaveState { MoveFramesSaveState() { } @@ -90,6 +92,15 @@ struct MoveFramesSaveState { QList positions; }; +/// Use this struct to store user related data that will later be added to the backup +/// This struct is meant to be safely shared and stored temporarily, +/// as such don't store ptrs here... +/// All data stored in here should be based on ZII (zero is initialization) principle +/// Only store what you need. +struct UserSaveState { + MoveFramesSaveState moveFramesState = {}; +}; + /// This is the main undo/redo state structure which is meant to populate /// whatever states that needs to be stored temporarily. struct UndoSaveState { @@ -106,10 +117,10 @@ struct UndoSaveState { int layerId = 0; int currentFrameIndex = 0; Layer::LAYER_TYPE layerType = Layer::UNDEFINED; - std::unique_ptr keyframe; SelectionSaveState selectionState = {}; - MoveFramesSaveState moveFramesState = {}; + + UserSaveState userState = {}; }; class UndoRedoManager : public BaseManager @@ -126,10 +137,10 @@ class UndoRedoManager : public BaseManager /** Records the given save state. * The input save state is cleaned up and set to nullptr after use. - * @param undoState The state to record. + * @param SaveStateId The state that will be fetched and recorded based on the input SaveStateId. * @param description The description that will bound to the undo/redo action. */ - void record(UndoSaveState*& undoState, const QString& description); + void record(SAVESTATE_ID SaveStateId, const QString& description); /** Checks whether there are unsaved changes. @@ -138,7 +149,14 @@ class UndoRedoManager : public BaseManager /** Prepares and returns an save state with common data * @return A UndoSaveState struct with common keyframe data */ - UndoSaveState* createState(UndoRedoRecordType recordType); + SAVESTATE_ID createState(UndoRedoRecordType recordType); + + /** Adds userState to the saveState found at SaveStateId + * If no record is found matching the id, nothing happens. + * @param SaveStateId The id used to fetch the saveState + * @param userState The data to be inserted onto on the saveState + */ + void addUserState(SAVESTATE_ID SaveStateId, UserSaveState userState); QAction* createUndoAction(QObject* parent, const QIcon& icon); QAction* createRedoAction(QObject* parent, const QIcon& icon); @@ -190,12 +208,15 @@ class UndoRedoManager : public BaseManager void pushCommand(QUndoCommand* command); void clearState(UndoSaveState*& state); + void clearSaveStates(); void legacyUndo(); void legacyRedo(); QUndoStack mUndoStack; + QMap mSaveStates; + // Legacy system int mLegacyBackupIndex = -1; LegacyBackupElement* mLegacyBackupAtSave = nullptr; @@ -204,6 +225,8 @@ class UndoRedoManager : public BaseManager int mLegacyLastModifiedLayer = -1; int mLegacyLastModifiedFrame = -1; + SAVESTATE_ID mSaveStateId = 1; + bool mNewBackupSystemEnabled = false; }; diff --git a/core_lib/src/tool/movetool.cpp b/core_lib/src/tool/movetool.cpp index 7fca1b385d..0c39f45252 100644 --- a/core_lib/src/tool/movetool.cpp +++ b/core_lib/src/tool/movetool.cpp @@ -174,7 +174,7 @@ void MoveTool::pointerMoveEvent(PointerEvent* event) void MoveTool::pointerReleaseEvent(PointerEvent*) { - mEditor->undoRedo()->record(mUndoSaveState, typeName()); + mEditor->undoRedo()->record(mUndoSaveStateId, typeName()); if (mEditor->overlays()->anyOverlayEnabled()) { @@ -224,7 +224,7 @@ void MoveTool::beginInteraction(const QPointF& pos, Qt::KeyboardModifiers keyMod QRectF selectionRect = selectMan->mySelectionRect(); if (!selectionRect.isNull()) { - mUndoSaveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); + mUndoSaveStateId = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); mEditor->backup(typeName()); } diff --git a/core_lib/src/tool/movetool.h b/core_lib/src/tool/movetool.h index f51cfd3b04..46f4315333 100644 --- a/core_lib/src/tool/movetool.h +++ b/core_lib/src/tool/movetool.h @@ -68,7 +68,7 @@ class MoveTool : public BaseTool MoveMode mPerspMode; QPointF mOffset; - UndoSaveState* mUndoSaveState = nullptr; + SAVESTATE_ID mUndoSaveStateId = 0; }; #endif diff --git a/core_lib/src/tool/polylinetool.cpp b/core_lib/src/tool/polylinetool.cpp index bc1d64d2b6..aad63255d6 100644 --- a/core_lib/src/tool/polylinetool.cpp +++ b/core_lib/src/tool/polylinetool.cpp @@ -203,11 +203,11 @@ void PolylineTool::pointerDoubleClickEvent(PointerEvent* event) // include the current point before ending the line. mPoints << getCurrentPoint(); - UndoSaveState* saveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); + SAVESTATE_ID saveStateId = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); mEditor->backup(typeName()); endPolyline(mPoints); - mEditor->undoRedo()->record(saveState, typeName()); + mEditor->undoRedo()->record(saveStateId, typeName()); } void PolylineTool::removeLastPolylineSegment() @@ -237,9 +237,9 @@ bool PolylineTool::keyPressEvent(QKeyEvent* event) case Qt::Key_Return: if (mPoints.size() > 0) { - UndoSaveState* saveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); + SAVESTATE_ID saveStateId = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); endPolyline(mPoints); - mEditor->undoRedo()->record(saveState, typeName()); + mEditor->undoRedo()->record(saveStateId, typeName()); return true; } break; diff --git a/core_lib/src/tool/selecttool.cpp b/core_lib/src/tool/selecttool.cpp index 0c29085e61..09188912fd 100644 --- a/core_lib/src/tool/selecttool.cpp +++ b/core_lib/src/tool/selecttool.cpp @@ -132,7 +132,7 @@ void SelectTool::pointerPressEvent(PointerEvent* event) if (event->button() != Qt::LeftButton) { return; } auto selectMan = mEditor->select(); - mUndoState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); + mUndoStateId = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); mPressPoint = event->canvasPos(); selectMan->setMoveModeForAnchorInRange(mPressPoint); @@ -193,7 +193,7 @@ void SelectTool::pointerReleaseEvent(PointerEvent* event) keepSelection(currentLayer); } - mEditor->undoRedo()->record(mUndoState, typeName()); + mEditor->undoRedo()->record(mUndoStateId, typeName()); mStartMoveMode = MoveMode::NONE; mSelectionRect = mEditor->select()->mapToSelection(mEditor->select()->mySelectionRect()).boundingRect(); diff --git a/core_lib/src/tool/selecttool.h b/core_lib/src/tool/selecttool.h index 3782131b1e..d3dc77a3e1 100644 --- a/core_lib/src/tool/selecttool.h +++ b/core_lib/src/tool/selecttool.h @@ -70,7 +70,7 @@ class SelectTool : public BaseTool QPixmap mCursorPixmap = QPixmap(24, 24); - UndoSaveState* mUndoState = nullptr; + SAVESTATE_ID mUndoStateId = 0; }; #endif diff --git a/core_lib/src/tool/stroketool.cpp b/core_lib/src/tool/stroketool.cpp index ea577aff8c..d7b2a6f073 100644 --- a/core_lib/src/tool/stroketool.cpp +++ b/core_lib/src/tool/stroketool.cpp @@ -140,7 +140,7 @@ void StrokeTool::startStroke(PointerEvent::InputType inputType) mStrokePressures << mInterpolator.getPressure(); mCurrentInputType = inputType; - mUndoSaveState = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); + mUndoSaveStateId = mEditor->undoRedo()->createState(UndoRedoRecordType::KEYFRAME_MODIFY); disableCoalescing(); } @@ -181,7 +181,7 @@ void StrokeTool::endStroke() mEditor->setModified(mEditor->currentLayerIndex(), mEditor->currentFrame()); mScribbleArea->endStroke(); - mEditor->undoRedo()->record(mUndoSaveState, typeName()); + mEditor->undoRedo()->record(mUndoSaveStateId, typeName()); } void StrokeTool::drawStroke() diff --git a/core_lib/src/tool/stroketool.h b/core_lib/src/tool/stroketool.h index a409050615..5ed4448b22 100644 --- a/core_lib/src/tool/stroketool.h +++ b/core_lib/src/tool/stroketool.h @@ -112,7 +112,7 @@ public slots: StrokeInterpolator mInterpolator; - UndoSaveState* mUndoSaveState = nullptr; + SAVESTATE_ID mUndoSaveStateId = 0; }; #endif // STROKETOOL_H From 697622211ca1c319a0030060d9f1e5e8f60f0168 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sat, 27 Sep 2025 13:19:14 +0200 Subject: [PATCH 09/10] Fix child undo/redo commands would be called before checking if obsolete --- core_lib/src/interface/undoredocommand.cpp | 68 ++++++++++++---------- 1 file changed, 37 insertions(+), 31 deletions(-) diff --git a/core_lib/src/interface/undoredocommand.cpp b/core_lib/src/interface/undoredocommand.cpp index 807e6441d5..27d4a34d6b 100644 --- a/core_lib/src/interface/undoredocommand.cpp +++ b/core_lib/src/interface/undoredocommand.cpp @@ -61,8 +61,6 @@ KeyFrameRemoveCommand::~KeyFrameRemoveCommand() void KeyFrameRemoveCommand::undo() { - UndoRedoCommand::undo(); - Layer* layer = editor()->layers()->findLayerById(undoLayerId); if (layer == nullptr) { // Until we support layer deletion recovery, we mark the command as @@ -70,6 +68,8 @@ void KeyFrameRemoveCommand::undo() return setObsolete(true); } + UndoRedoCommand::undo(); + layer->addKeyFrame(undoKeyFrame->pos(), undoKeyFrame->clone()); emit editor()->frameModified(undoKeyFrame->pos()); @@ -79,11 +79,17 @@ void KeyFrameRemoveCommand::undo() void KeyFrameRemoveCommand::redo() { + Layer* layer = editor()->layers()->findLayerById(redoLayerId); + if (layer == nullptr) { + // Until we support layer deletion recovery, we mark the command as + // obsolete as soon as it's been + return setObsolete(true); + } + UndoRedoCommand::redo(); if (isFirstRedo()) { setFirstRedo(false); return; } - Layer* layer = editor()->layers()->findLayerById(redoLayerId); layer->removeKeyFrame(redoPosition); emit editor()->frameModified(redoPosition); @@ -113,13 +119,13 @@ KeyFrameAddCommand::~KeyFrameAddCommand() void KeyFrameAddCommand::undo() { - UndoRedoCommand::undo(); - Layer* layer = editor()->layers()->findLayerById(undoLayerId); if (!layer) { return setObsolete(true); } + UndoRedoCommand::undo(); + layer->removeKeyFrame(undoPosition); emit editor()->frameModified(undoPosition); @@ -130,16 +136,16 @@ void KeyFrameAddCommand::undo() void KeyFrameAddCommand::redo() { - UndoRedoCommand::redo(); - - // Ignore automatic redo when added to undo stack - if (isFirstRedo()) { setFirstRedo(false); return; } - Layer* layer = editor()->layers()->findLayerById(redoLayerId); if (!layer) { return setObsolete(true); } + UndoRedoCommand::redo(); + + // Ignore automatic redo when added to undo stack + if (isFirstRedo()) { setFirstRedo(false); return; } + layer->addNewKeyFrameAt(redoPosition); emit editor()->frameModified(redoPosition); @@ -167,14 +173,14 @@ MoveKeyFramesCommand::MoveKeyFramesCommand(int offset, void MoveKeyFramesCommand::undo() { - UndoRedoCommand::undo(); - Layer* undoLayer = editor()->layers()->findLayerById(undoLayerId); if (!undoLayer) { return setObsolete(true); } + UndoRedoCommand::undo(); + for (int position : qAsConst(positions)) { undoLayer->moveKeyFrame(position + frameOffset, -frameOffset); } @@ -184,17 +190,17 @@ void MoveKeyFramesCommand::undo() void MoveKeyFramesCommand::redo() { - UndoRedoCommand::redo(); - - // Ignore automatic redo when added to undo stack - if (isFirstRedo()) { setFirstRedo(false); return; } - Layer* redoLayer = editor()->layers()->findLayerById(redoLayerId); if (!redoLayer) { return setObsolete(true); } + UndoRedoCommand::redo(); + + // Ignore automatic redo when added to undo stack + if (isFirstRedo()) { setFirstRedo(false); return; } + for (int position : qAsConst(positions)) { redoLayer->moveKeyFrame(position, frameOffset); } @@ -221,13 +227,13 @@ BitmapReplaceCommand::BitmapReplaceCommand(const BitmapImage* undoBitmap, void BitmapReplaceCommand::undo() { - UndoRedoCommand::undo(); - Layer* layer = editor()->layers()->findLayerById(undoLayerId); if (!layer) { return setObsolete(true); } + UndoRedoCommand::undo(); + static_cast(layer)->replaceKeyFrame(&undoBitmap); editor()->scrubTo(undoBitmap.pos()); @@ -235,16 +241,16 @@ void BitmapReplaceCommand::undo() void BitmapReplaceCommand::redo() { - UndoRedoCommand::redo(); - - // Ignore automatic redo when added to undo stack - if (isFirstRedo()) { setFirstRedo(false); return; } - Layer* layer = editor()->layers()->findLayerById(redoLayerId); if (!layer) { return setObsolete(true); } + UndoRedoCommand::redo(); + + // Ignore automatic redo when added to undo stack + if (isFirstRedo()) { setFirstRedo(false); return; } + static_cast(layer)->replaceKeyFrame(&redoBitmap); editor()->scrubTo(redoBitmap.pos()); @@ -269,13 +275,13 @@ VectorReplaceCommand::VectorReplaceCommand(const VectorImage* undoVector, void VectorReplaceCommand::undo() { - UndoRedoCommand::undo(); - Layer* layer = editor()->layers()->findLayerById(undoLayerId); if (!layer) { return setObsolete(true); } + UndoRedoCommand::undo(); + static_cast(layer)->replaceKeyFrame(&undoVector); editor()->scrubTo(undoVector.pos()); @@ -283,16 +289,16 @@ void VectorReplaceCommand::undo() void VectorReplaceCommand::redo() { - UndoRedoCommand::redo(); - - // Ignore automatic redo when added to undo stack - if (isFirstRedo()) { setFirstRedo(false); return; } - Layer* layer = editor()->layers()->findLayerById(redoLayerId); if (!layer) { return setObsolete(true); } + UndoRedoCommand::redo(); + + // Ignore automatic redo when added to undo stack + if (isFirstRedo()) { setFirstRedo(false); return; } + static_cast(layer)->replaceKeyFrame(&redoVector); editor()->scrubTo(redoVector.pos()); From 28340d744be7c3ff149f1c7cd4ec3340590fbdc7 Mon Sep 17 00:00:00 2001 From: MrStevns Date: Sun, 28 Sep 2025 10:06:23 +0200 Subject: [PATCH 10/10] Apply minor improvement --- core_lib/src/managers/undoredomanager.cpp | 4 ++-- core_lib/src/managers/undoredomanager.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core_lib/src/managers/undoredomanager.cpp b/core_lib/src/managers/undoredomanager.cpp index 09df7a24c6..507ec0fc6b 100644 --- a/core_lib/src/managers/undoredomanager.cpp +++ b/core_lib/src/managers/undoredomanager.cpp @@ -260,13 +260,13 @@ SAVESTATE_ID UndoRedoManager::createState(UndoRedoRecordType recordType) state->recordType = recordType; initCommonKeyFrameState(state); - mSaveStates[mSaveStateId] = state; + mSaveStates[saveStateId] = state; mSaveStateId += 1; return saveStateId; } -void UndoRedoManager::addUserState(SAVESTATE_ID saveStateId, UserSaveState userState) +void UndoRedoManager::addUserState(SAVESTATE_ID saveStateId, const UserSaveState& userState) { if (!mSaveStates.contains(saveStateId)) { return; } mSaveStates[saveStateId]->userState = userState; diff --git a/core_lib/src/managers/undoredomanager.h b/core_lib/src/managers/undoredomanager.h index 8dae77736a..9d49523a45 100644 --- a/core_lib/src/managers/undoredomanager.h +++ b/core_lib/src/managers/undoredomanager.h @@ -156,7 +156,7 @@ class UndoRedoManager : public BaseManager * @param SaveStateId The id used to fetch the saveState * @param userState The data to be inserted onto on the saveState */ - void addUserState(SAVESTATE_ID SaveStateId, UserSaveState userState); + void addUserState(SAVESTATE_ID SaveStateId, const UserSaveState& userState); QAction* createUndoAction(QObject* parent, const QIcon& icon); QAction* createRedoAction(QObject* parent, const QIcon& icon);