A very hacky undo redo implementation #531
Conversation
Should serve as a proof of concept...and even if it gets implemented like this (with major cleanup and fixes), it would serve as a way to start adding the right events in the right place. And also see any reworks that may need to happen.
mbfraga
changed the title
|
|
||
|
|
||
|
|
||
| public class Akira.Lib.Managers.UndoManager : Object { |
There was a problem hiding this comment.
I'd call this HistoryManager since it will handle both undos and redos.
| } | ||
|
|
||
| while (to_delete.get_length () != 0) { | ||
| canvas.window.event_bus.request_delete_item (to_delete.pop_head()); |
There was a problem hiding this comment.
We don't need to collect the images as the request_delete_item will remove those automatically.
Just artboards and free items.
Here we could also make this method async and use multi threading to prevent UI freeze when clearing a very busy canvas.
| clear_canvas(canvas); | ||
| Akira.FileFormat.JsonLoader.inner_load_content(canvas, obj); |
There was a problem hiding this comment.
I'm not sure about this, it seems a bit extreme.
With this if we move an item and we want to revert to its previous position, we're clearing the canvas and reloading everything. That seems expensive.
There was a problem hiding this comment.
If clearing and reloading is expensive, we have bigger issues than undo/redo. There is no reason the view should be expensive to regenerate.
I'm open to new ideas, but am cautious about an alternative that doesn't require cleaning the slate. They tend to be rather brittle and crash or artifact prone approaches.
There was a problem hiding this comment.
To be clear, the current implementation of clearing and reloading is not efficient, but that's a different story.
|
@mbfraga wanna close this as it's gonna be invalid with the components refactor you're doing? |
|
@mbfraga can we close this since it doesn't fit with the component refactor you're doing? |
Summary / How this PR fixes the problem?
Implement a very hacky undo redo using existing serialization methods
Should serve as a proof of concept...and even if it gets implemented like this (with major cleanup and fixes), it would serve as a way to start adding the right events in the right place. And also see any reworks that may need to happen.
Known Issues / Things To Do
Super hacks--if this is the direction initial undo/redo takes, then serialization / deserialization needs to be refactored correctly.
This PR fixes/implements the following bugs/features: