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

List:       kde-kimageshop
Subject:    Re: Fixed bug in KisPerChannelFilter that prevented it from
From:       Dmitry Kazakov <dimula73 () gmail ! com>
Date:       2009-03-10 21:31:42
Message-ID: ae32c1ef0903101431t4c3ca672w3553dfea3f52b20a () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


>
> > In such a case we miss loading of the default configuration from kritarc.
> > So return d->parentFilter->defaultConfiguration(); instead :P
> No we don't, since the factory is mainly used by the bookmark manager when
> reading from kritarc, and by using d->parentFilter->defaultConfiguration();
> we create an infinite loop, since:
> defaultConfiguration() calls the bookmarkmanager which call the
> createDefault() from factory which calls defaultConfiguration() which calls
> the bookmarkmanager which call the createDefault() from factory which calls
> defaultConfiguration() which....
>
>
In such a case we should name it another way, because it doesn't load "real
default configuration" (that is seen as default by the user).
What about KisFilterConfigurationFactory::factoryConfiguration() ?
It just creates a new object, but not default config.


> Ok. Now i've updated my patch to the fourth version =)
http://reviewboard.kde.org/r/203/
Hope, it'll be the last one (concerning this very bug). I've changed
to/fromXML to be more like OpenRaster.

But i found some issues:
1a) KisPropertiesConfiguration doesn't conform to OpenRaster as it doesn't
create something like

<filter name="perchannel" type="applications:Krita:PerChannel">...</filter>

1b) It packs all the properties into CDATA structure. As i understood from
specification properties should be written as simple text? Or this is the
same?


-- 
Dmitry Kazakov

[Attachment #5 (text/html)]

<div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px \
solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div \
style="font-family: &#39;DejaVu Sans&#39;; font-size: 9pt; font-weight: 400; \
font-style: normal;"> <div class="im">
&gt; In such a case we miss loading of the default configuration from kritarc.<br>
&gt; So return d-&gt;parentFilter-&gt;defaultConfiguration(); instead :P<br></div>
No we don&#39;t, since the factory is mainly used by the bookmark manager when \
reading from kritarc, and by using d-&gt;parentFilter-&gt;defaultConfiguration(); we \
create an infinite loop, since:<br> defaultConfiguration() calls the bookmarkmanager \
which call the createDefault() from factory which calls defaultConfiguration() which \
calls the bookmarkmanager which call the createDefault() from factory which calls \
defaultConfiguration() which....<br>

<p style="margin: 0px; text-indent: 0px;"></p></div></blockquote><div><br>In such a \
case we should name it another way, because it doesn&#39;t load &quot;real default \
configuration&quot; (that is seen as default by the user).<br> What about \
KisFilterConfigurationFactory::factoryConfiguration() ? <br>It just creates a new \
object, but not default config.<br>  </div><blockquote class="gmail_quote" \
style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; \
padding-left: 1ex;"> </blockquote></div>Ok. Now i&#39;ve updated my patch to the \
fourth version =) <br><a \
href="http://reviewboard.kde.org/r/203/">http://reviewboard.kde.org/r/203/</a><br>Hope, \
it&#39;ll be the last one (concerning this very bug). I&#39;ve changed to/fromXML to \
be more like OpenRaster.<br> <br>But i found some issues:<br>1a) \
KisPropertiesConfiguration doesn&#39;t conform to OpenRaster as it doesn&#39;t create \
something like <br><pre class="changedesc-text">&lt;filter \
name=&quot;perchannel&quot; \
type=&quot;applications:Krita:PerChannel&quot;&gt;...&lt;/filter&gt; </pre> 1b) It \
packs all the properties into CDATA structure. As i understood from specification \
properties should be written as simple text? Or this is the same?<br \
clear="all"><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]