Fixed bug in KisPerChannelFilter that prevented it from loading/saving bookmarks v.3

[Cyrille Berger]

On Thursday 05 March 2009, Dmitry Kazakov wrote:
> PS:
> And an issue for the dispute: why do we have such a strange API?
>
> The first issue is toXML/toLegacyXML stuff. These functions are implemented
> in _different_ classes although they do the same things. More than that,
> both of them are called from different parts of krita: bookmark manager
> uses toXML, kis_kra_ uses toLegacyXML Why not have the only one? The only
> united interface?
.kra file are expected to use the toLegacyXML because this functions saves to 
an XML formated for the 1.6 .kra files. While toXML use the OpenRaster style, 
described in 
http://create.freedesktop.org/wiki/index.php?title=OpenRaster/Layers_Stack_Specification 
.

I suppose we could have used the OpenRaster style in .kra files as well, but 
then we would have had to make migration path from the old style to the OR 
style, which also have the drawback to be not finished, since only the XML is 
specified, but the values and parameters isn't specified at the moment, even if 
it will be very inspired by the SVG spec.

> And the second issue is defaultConfigurations of the filter. The specific
> Kis<smth>FilterConfiguration class for the filter can be (and is) created
> in two different places:

I will reorder the question :)

> Speaking truly i can't understand the meaning of these factory-classes at
> all =(
If you are not familiar with the factory method pattern, I suggest to read 
http://en.wikipedia.org/wiki/Factory_method_pattern . The main idea here is 
that 95% of filters can live happilly with the KisFilterConfiguration class, 
they just have to read a few integers, and then they can filter a paint device. 
But for some filters it's not good enough, and they need to have a 'cache' of 
some information, like the transfert curve (or the KoColorTransformation, but 
that cache was removed sometime ago since it triggered issue with 
multithreading, but it might be worth to restore it, sometime), hence the need 
of the custom KisFilterConfiguration.

> KisBaseProcessor::defaultConfiguration(const KisPaintDeviceSP pd)
The documentation says all. It's the configuration that will be used in the 
filter dialog when the user start the dialog.

You forgot that way as well:
KisFilterConfiguration * KisBaseProcessor::factoryConfiguration(const 
KisPaintDeviceSP) const
whose name sucks and is a bit confusing (if anyone has a better idea, the name  
comes from what you can sometime see on some devices: "reset to factory 
settings", but here it might be confusing in the middle of the factory 
pattern), it allows filters developers to provide the user of the filter with 
some sane initial values for the first run.

> and
> KisFilterConfigurationFactory::createDefault(). Why?! The same thing with
> KisFilterConfigurationFactory::create(...). It's a clone. Why is it so?
Filter configurations are saved in KisBookmarkedConfigurationManager which is a 
generic class which can be used with all kind of KisSerializableConfiguration 
to store them, this can be filters/generators configuration, paintop settings, 
... The only way KisBookmarkedConfigurationManager can creates the correct 
subclass of KisSerializableConfiguration is by using the 
KisFilterConfigurationFactory.

The difference between createDefault and create, is that create is used to 
unserialized configuration, and createDefault is used to create the first to use 
default configuration (now that I have said that, I note that there is a bug in 
the implementation, since it means KisBaseProcessor::factoryConfiguration is 
never called, and I think that KisFilterConfigurationFactory::createDefault 
should call KisBaseProcessor::factoryConfiguration).

I hope it makes things a little bit more clear :)

-- 
Cyrille Berger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kimageshop/attachments/20090309/b30c82c1/attachment.htm