[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-kimageshop
Subject:    Re: KisImage::undo() - what is it?
From:       Dmitry Kazakov <dimula73 () gmail ! com>
Date:       2010-06-07 6:43:24
Message-ID: AANLkTimKjeSkiYok_BovJ9DMZgqF02luGvxN3UZVOyn7 () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


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

[Attachment #5 (text/html)]

Well, i&#39;ve done some investigations about KisImage::undo(). This method turns off \
versioning for the actions. It&#39;s better to say it is supposed to turn off \
versioning, but it doesn&#39;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&#39;s actions, in tools...<br> <br>So why all \
this? It&#39;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?<br><br>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&#39;t really see any reason in it. Do you know \
one?<br> <br>And i&#39;ve made a small benchmark that loads load_test.kra with these \
undo() capabilities and with undo always set to true. And i haven&#39;t found any \
difference in speed. <br><br><br>Conclusions:<br>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.<br> 2) This method doesn&#39;t \
do what it is supposed to do - no speedups.<br>3) If we still want to have these \
undo-disable capabilities (though i still don&#39;t see any reason) they must be \
encapsulated inside KisTransaction and KisUndoAdapter.<br> <br>So want do i \
want:<br>1) Remove most of these undo() checks throughout the Krita code<br>2) \
Deprecate/remove undo()/setUndo() methods of KisImage.<br><br>This will allow all the \
code to create KisTransaction objects inside a stack instead of a heap.<br> \
<br><br><br>PS:<br clear="all">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.<br><br>-- <br>Dmitry Kazakov<br>



_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop


[prev in list] [next in list] [prev in thread] [next in thread]