maoschanz / drawing

Simple image editor for Linux
https://maoschanz.github.io/drawing/
GNU General Public License v3.0
785 stars 101 forks source link

Improved undo performance. #561

Open gulis1 opened 1 year ago

gulis1 commented 1 year ago

The two main performance bottlenecks were:

  1. Having to loop over the whole history to find the last remembered state.
  2. If the user doesn't save to disk, to undo the last action the drawing was always being rebuilt from the initial operation.

To address these issues, the class State was made (it could be turned into a dict, but I feel a class fits better for this). This new class contains a remembered state, as well as the next __maxoperations following operations.

Now the undohistory attribute is a list of State objects instead of individual operations. This lets us access the last remembered state in constant time instead of having to loop over all previous operations. (the redohistory remains exactly the same)

When a new operation arrives, it is stored in the last entry of the undohistory list only if there's less than maxoperations operations stored in that object. Otherwise, a new State object containing a copy of the current pixbuf is appended to the list, and the operation is saved there.

These changes are specially noticeable when the user has drawn a lot of operations, because we don't have to loop over all of them, and to redraw the picture we only have to redraw at most __maxoperations operations.

For now, I've set the value of __maxoperations to 20, but this value can be easily changed. It might be a good idea to create new settings in the preferences menu where the user could change this value, as well as the maximum memory the _historymanager is allowed to use.

Resolves #200

maoschanz commented 1 year ago

Having to loop over the whole history to find the last remembered state.

this was indeed not elegant, but not an actual performance bottleneck lol, the only real bottleneck is your 2nd point

it might be a good idea to create new settings in the preferences menu where the user could change this value

the user shouldn't worry about such implementation details. If they want to have control on this kind of technical aspect, they can already force the creation of a state by saving their work 🤷

maoschanz commented 1 year ago

oh, and you can credit your name in the "about" dialog (main.py) if you want

gulis1 commented 1 year ago

oh, and you can credit your name in the "about" dialog (main.py) if you want

Cool, thanks! I added the entry in the "authors" section. Let me know if it is supposed to go elsewhere and I'll change it ASAP.

I think both the state merging and operations weights are great ideas, I can help you implement them in the future if you want.

gulis1 commented 1 year ago

Sorry, I was checking out how reviews worked (it's my first time using them), and I accidentally pressed the review request button. 😓

maoschanz commented 1 year ago

you were wondering about the big red "changes requested" thing? i don't remember how to remove it (nevermind i do), but i have the button to merge anyway don't worry

i'm trying to get a good enough code to release the version 1.2.0, and then your code will be merged

gulis1 commented 1 year ago

No worries, thanks. I was just wondering if there were any change requests I missed last time. 👍