<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"><html><head><meta name="qrichtext" content="1" /><style type="text/css">p, li { white-space: pre-wrap; }</style></head><body style=" font-family:'DejaVu Sans'; font-size:9pt; font-weight:400; font-style:normal;">On Monday 09 March 2009, Dmitry Kazakov wrote:<br>
> > > The first issue is toXML/toLegacyXML stuff. These functions are<br>
> ><br>
> > implemented<br>
> ><br>
> > > in _different_ classes although they do the same things. More than<br>
> > > that, both of them are called from different parts of krita: bookmark<br>
> > > manager uses toXML, kis_kra_ uses toLegacyXML Why not have the only<br>
> > > one? The only united interface?<br>
> ><br>
> > .kra file are expected to use the toLegacyXML because this functions<br>
> > saves to an XML formated for the 1.6 .kra files. While toXML use the<br>
> > OpenRaster style, described in<br>
> > http://create.freedesktop.org/wiki/index.php?title=OpenRaster/Layers_Stac<br>
> >k_Specification.<br>
><br>
> Do you mean filter's configuration should look like that?<br>
><br>
> <filter name="perchannel" type="applications:Krita:PerChannel"><br>
> <params version=1><br>
> <param name="CurvesNumber">3</param><br>
> <param name="curve0">0,0;0.5,0.5;1,1;</param><br>
> <param name="curve1">0,0;1,1;</param><br>
> <param name="curve2">0,0;1,1;</param><br>
> </params><br>
> </filter><br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>Yes.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>><br>
> even if it will be very inspired by the SVG spec.<br>
><br>
><br>
> I'm afraid we can't be inspired by SVGspec here at all. The point is they<br>
> suggest us to use linear interpolation for the curve (i guess this type of<br>
> the curve had been implemented in KCurve before my patch) . Maybe this type<br>
> is a good idea for some ideal theoretical curve as it's quite simple and<br>
> fast, but for real users (who are "inspired" by real professional<br>
> instrumens, such as PS, PictureWindowPro, TheGimp) it's at least _strange_<br>
> curve and almost unusable, because they are used to cubic splines.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>Difference between inspiration and copy/paste is that you can make changes ;) I have no objection to adding cubic splines to the list of types, and to properly specify it.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>> Of course, we can interpolate cubic spline, drawn by the user, to the<br>
> linear one and write the latter to the file in SVG format. But the user<br>
> won't be able to change the curve later.<br>
><br>
> And the last issue against SVG. SVG's feComponentTransferis is stuck to<br>
> RGB. RGB is not the only colorspace in the world. ;)<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>Well you have shown one of the solution to that problem in the beginning of the mail ;) While I am not a fan of numbers, it might as well be better to have curve_red, curve_blue, curve_cyan, curve_l, curve_...<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>> > > KisBaseProcessor::defaultConfiguration(const KisPaintDeviceSP pd)<br>
> ><br>
> > The documentation says all.<br>
><br>
> Where can i take a look on this documentation?<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>In the header, or http://www.koffice.org/developer/apidocs/ (more specificly http://www.koffice.org/developer/apidocs/krita-image/classKisBaseProcessor.html#09a63a0fbbf406ccddd0409fe2644e77 )<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>> > It's the configuration that will be used in the filter dialog when the<br>
> > user start the dialog.<br>
> ><br>
> > You forgot that way as well:<br>
> > KisFilterConfiguration * KisBaseProcessor::factoryConfiguration(const<br>
> > KisPaintDeviceSP) const<br>
> > whose name sucks and is a bit confusing (if anyone has a better idea,<br>
><br>
> createConfiguration?<br>
I think it's a too generic name.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>> > Filter configurations are saved in KisBookmarkedConfigurationManager<br>
> > which is a generic class which can be used with all kind of<br>
> > KisSerializableConfiguration to store them, this can be<br>
> > filters/generators configuration, paintop settings, ... The only way<br>
> > KisBookmarkedConfigurationManager can create the correct subclass of<br>
> > KisSerializableConfiguration is by using the<br>
> > KisFilterConfigurationFactory.<br>
><br>
> The problem is that KisKraLoader::loadAdjustmentLayer calls<br>
> KisBaseProcessor::defaultConfiguration(...). The latter one calls<br>
> factoryConfiguration and (if factoryConfiguration isn't overridden in<br>
> KisPerChannelFilter) creates KisFilterConfiguration instead of needed<br>
> KisPerChannelFilterConfiguration. This causes adjustment layers not being<br>
> loaded properly from .kra.<br>
><br>
> The easiest (and the most illogical) workaround is to change<br>
> KisBookmarkedConfigurationManager::defaultConfiguration, that has access to<br>
> filter's configurationFactory, to call factory's createDefault if nothing<br>
> can be loaded from kritarc.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>Why not provide an implementation of KisBaseProcessor::factoryConfiguration in KisPerChannelFilter ?<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>> KisSerializableConfiguration*<br>
> KisBookmarkedConfigurationManager::defaultConfiguration() const<br>
> {<br>
> if (exists(KisBookmarkedConfigurationManager::ConfigDefault.id())) {<br>
> return load(KisBookmarkedConfigurationManager::ConfigDefault.id());<br>
> }<br>
> ///skipped///<br>
> return d->configFactory->createDefault();<br>
> }<br>
><br>
> In this case we won't need factoryDefault at all. But this change is _very<br>
> bad idea_ as it's illogical.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>Yes. Not good.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>> In my opinion Factories should somehow call Filter's defaultConfiguration<br>
> instead of having their own createDefault and create(...). In this case,<br>
> maybe, we won't even be needed to override KisConfigurationFactory for<br>
> every filter.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>Except defaultConfiguration it will break the purpose of defaultConfiguration, and will trigger an endless loop, since KisFilter::defaultConfiguration() call the BookmarkManager to get the configuration which will then call the factory. But it might be a good idea to use KisBaseProcessor::factoryConfiguration for that, instead.<br>
The main advantages, would be that there would, indeed, be no more need for overriding KisConfigurationFactory, and if the filter get new parameters they are loaded from the factoryConfiguration. And I can't see a drawback.<br>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"><br></p>-- <br>
Cyrille Berger</p></body></html>