<div class="gmail_quote">On Tue, May 15, 2012 at 9:47 AM, Dmitry Kazakov <span dir="ltr"><<a href="mailto:dimula73@gmail.com" target="_blank">dimula73@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><br><div class="gmail_quote"><div>On Tue, May 15, 2012 at 2:45 AM, Sven Langkamp <span dir="ltr"><<a href="mailto:sven.langkamp@gmail.com" target="_blank">sven.langkamp@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_quote"><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><br></div>How it can be achieved in the constraints of the strokes [0]:<br>
<br>1) The caller asks KisStrokeStrategyFactoryRegistry for the factory of "ScaleLayer" stroke.<br>2) The factory does the following: i) calls image->barrierLock() to fetch right size information about the image; ii) calls image->unlock(); iii) fires up the configuration dialog.<br>
3) The factory creates the stroke strategy for the "ScaleLayer" stroke (usual undo command based stroke)<br>4) The factory uploads the XML data about the stroke into the strategy<br>4) The factory starts a new stroke and populates it with commands<br>
5) The strokes system calls the stroke strategy's method toXML, which saves the data<br><br>Comment:<br><br>For each action we create one (!) class only: KisStrokeStrategyFactory. The factory creates a simple undo-command-based stroke, forms XML, passes it to the stroke strategy, then populates the strategy with the commands. Later, when the stroke is finished, the framework asks the strategy for the XML and it returns the value formed by the factory. <br>
<br>The advantage of this approach is that we shouldn't change anything and
only need to add one class per action. More than that this approach will
not demand us to add toXML/fromXML to the undo commands: you are right,
commands are too lowlevel for that.<br>
<br>Btw, take into account, that the strokes framework is capable of canceling the tasks! It works right now (and is tested in unittests), but it isn't connected anywhere to the UI because of the problems with shortcuts in our tools.<br>
<br>If you agree that we implement it this way we can discuss it further and assign the tasks ;). I can do the work of adding gathering the XML into the stroke system (it shouldn't be much of work) and create base classes for the factories.<br>
<br><br>[0] - <a href="http://community.kde.org/Krita/Recording_System" target="_blank">http://community.kde.org/Krita/Recording_System</a><span></span><br></blockquote></div></div><div><br>I'm not sure why you want to do step 1. The stroke would be empty at that point and only be used much later.<br>
</div></div></blockquote></div><div><br>Yes, there is no stroke at the step 1 yet, but I'd prefer to encapsulate the dialog, formation of the XML data and the creation of the stroke in a separate object for the following reasons:<br>
<br>1) In several cases we must to barrierLock the image before showing the dialog, in others we needn't. It means we need to have a general code for this locking in the base class.<br>2) The second reason is even more important. The user should be able to see the dialog for the recorded actions. He can use it either for editing the recorded action or for modifying the action on-the-go. Both the usecases are implemented in PS.<br>
</div></div></blockquote><div><br>This is only needed for showing the widget. I think we can handle the UI and the data extraction seperately. First we switch the data to KisPropertiesConfiguration with the UI still working as it's now. Once that is done, we can handle the rest.<br>
</div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote">
<div><br></div><div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div>
The XML data you mention could be much easier be done with the KisPropertiesConfiguration. There is no way around it as we have to save the input values at some point. This should also be used by scripts and macros I think.<br>
</div></div></blockquote></div><div><br>Yes, I agree. The highlevel code in factories can surely use it. What I'm not sure about is whether properties can work good for usual strokes, which might contain very long array of short values (points). Is it possible to do without working it around with a set of preperties pt1,pt2,...,ptN?<br>
</div></div></blockquote><div><br>It's enough for the filter and I don't think the other actions are more complicated. Arrays can be saved like curves, were the list is serialize before it's stored in the properties. If we are missing properties we can also add them to KisPropertiesConfiguration.<br>
</div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div></div><div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_quote"><div>
I'm not sold on the undo command based stroke. These tend to really make the code more complicated and I would prefer if we could make this more simple. Might be better to base it on KisStrokeJob. Btw we really need more documentation for the Strokes system, without it's pretty hard to grasp what the classes do or what sequentiality and exclusivity do.<br>
</div></div></blockquote></div><div><br>There is a documentation on undo-command based strokes in ./krita/doc/strokes/strokes_documentation.html<br>If you need some more information on a particular topic, please ask and I'll add it.<br>
<br>The system will not become more complicated, because the factories would use the same KisProcessingApplicator, which would get one more method (something like attachXMLData()). So the code of, e.g. "ScaleImage" will not change much. It will just be moved to a separate class and be coupled with function for generating XML.<br>
</div></div></blockquote><div><br>Aha. The apidox should at least point to the external documentation.<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_quote"><div>
</div><div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div>
</div></div>One class for each action sounds very heavy. We have probably over a hundred action that are possible and I think we should keep the needed code to a minimum. Maybe we need to pick some simple action and then think it through and check which changes would be needed. We really need to get it right before the big porting starts.<br>
</blockquote></div><div><br>Yes, sure. We can start with, e.g. "ScaleImage" method of KisImage. It is already ported to the strokes, so it is easier to port it =)<span><font color="#888888"><br></font></span></div>
</div></blockquote><div><br>Ok. So in the image size dialog we currently have:<br><br> if (dlgImageSize->exec() == QDialog::Accepted) {<br> qint32 w = dlgImageSize->width();<br> qint32 h = dlgImageSize->height();<br>
double res = dlgImageSize->resolution();<br><br> m_view->imageManager()->scaleCurrentImage(QSize(w, h), res, res, dlgImageSize->filterType());<br> }<br><br>that would become (without adding a QSize property)<br>
<br> if (dlgImageSize->exec() == QDialog::Accepted) {<br> KisActionCommand* command = new KisActionCommand("scaleImage")<br> command->setProperty("width", dlgImageSize->width());<br>
command->setProperty("height", dlgImageSize->height());<br> command->setProperty("xres", dlgImageSize->resolution());<br> command->setProperty("yres", dlgImageSize->resolution());<br>
command->setProperty("filterID", dlgImageSize->filterType()->id());<br>
m_view->runCommand(command);<br>
}<br> <br>on the other end it would need:<br><br> ...processCommand(KisActionCommand* command) {<br>
qint32 width = command->getInt("width");<br> ...<br>
scaleImage(width, ...);<br>
}<br><br>This code could be auto-generated, if we generate the methods that do the set/get of the properties.<br></div></div>