Improved undo performance.

[gulis1]

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 _max_operations following operations.

Now the _undo_history 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 _redo_history remains exactly the same)

When a new operation arrives, it is stored in the last entry of the _undo_history list only if there's less than _max_operations 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 _max_operations operations.

For now, I've set the value of _max_operations 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 history_manager is allowed to use.

Resolves #200

[maoschanz]

i finally have time to review this

it looks very great but i still made a few minor remarks

---

concerning the _max_operations thing, the number of operations should be different depending on what tool the user used:

• the slow blur filter is very very horrible
• my attempt to get an airbrush is horrible
• cropping/resizing can be heavy
• manipulating the selection can be heavy too
• there is some very stupid shit happening with the filling paint bucket
• the rest is usually fine

so in my opinion, the heavier operations should have a "weight" associated to them when the tool generates the dict, and the constant should be named _max_capacity or something like that: a new state would appear after 4 operations of weight 5, or 5 operations of weight 2 + 10 of weight 1, etc.

but don't implement that weight idea now, it's not critical at all and it would create merge conflicts, i'll retrofit it later, it was just to say that you shouldn't worry about the value being 20 or 30 or 10 or anything specific

[maoschanz]

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]

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

[gulis1]

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]

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

[maoschanz]

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]

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