Patch: Many composite/blend modes mostly compatible to Adobe Photoshop (c)

Cyrille Berger Skott cberger at cberger.net
Sun Feb 6 09:46:31 CET 2011


Hi,

Here is some code review:

* I did some more testing and it is indeed a regression for this bug 
https://bugs.kde.org/show_bug.cgi?id=176536 , since I am assuming that you 
wanted it that way, having transparent pixel being erased.

For instance look at the result of the following file:

http://cyrille.diwi.org/tmp/krita/colored_leaves.kra
Before patch:
http://cyrille.diwi.org/tmp/krita/colored_leaves_before.png
After patch:
http://cyrille.diwi.org/tmp/krita/colored_leaves_after.png

Since I am not convinced that "alpha lock" should be used for that, since it 
is there to prevent editing of the alpha channel, if the having transparent 
pixel erased is a wanted behaviour we should probably introduce a "keep 
transparency when compositing" and have it on by default

* the optimized integers operations do provide a significant boost in speed, 
about 10% on the composite operations, that translate to about 5% when 
drawing. I don't know at what size of brush you tried, but even on high-end 
hardware it makes a noticeable difference when using large size brushes. On 
the other hand, it does provide an unnoticeable difference in the results. 
This is why those optimizwd integers operations are used in Gimp, imagemagick 
and many other graphics applications. So I am against dropping them. If it is 
really a problem for you, I would suggest the use of a compile switch, 
ENABLE_EXACT_PRECISION_COMPOSITE_OPS , defaulted to off.

* KoCompositeOpFunction.h should not duplicate the content of 
KoColorSpaceMath.h (especially lerp, mul) and the functions that does not 
exist in KoColorSpaceMath should be added (like inv) Similary use "clamp" from 
KoColorSpaceMath, since your version clamp values for floating points as well, 
we should not happen.

* have you cheked if compositeFunc is inlined inside KoCompositeOpGenericSC ?

-- 
Cyrille Berger Skott


More information about the kimageshop mailing list