KisImage::undo() - what is it?

[Dmitry Kazakov]

Well, i've done some investigations about KisImage::undo(). This method
turns off versioning for the actions. It's better to say it is supposed to
turn off versioning, but it doesn't encapsulate anything. It is just a
global variable that code all over Krita has to check before creation of a
transaction. You can see these checks everywhere: in KisImage's actions, in
tools...

So why all this? It's quite strange, but it just turns off the global
variable when the document is loading. And nothing more! And for the sake of
this we add these ugly checks everywhere?

Actually, with our tile manager (even with the old one, i guess) creation of
the initial revision of the image is very cheap, because all the default
tiles are shared, so i don't really see any reason in it. Do you know one?

And i've made a small benchmark that loads load_test.kra with these undo()
capabilities and with undo always set to true. And i haven't found any
difference in speed.


Conclusions:
1) This method (using global variable) is not good from design point of view
- the caller should check the variable himself, that causes weird
caller-side code.
2) This method doesn't do what it is supposed to do - no speedups.
3) If we still want to have these undo-disable capabilities (though i still
don't see any reason) they must be encapsulated inside KisTransaction and
KisUndoAdapter.

So want do i want:
1) Remove most of these undo() checks throughout the Krita code
2) Deprecate/remove undo()/setUndo() methods of KisImage.

This will allow all the code to create KisTransaction objects inside a stack
instead of a heap.



PS:
Btw, technically, in a new tile engine, there is no difference whether you
create this initial transaction or not, there will be the same code
execution path.

-- 
Dmitry Kazakov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20100607/0b0ba30e/attachment.htm