[calligra] /: Merge remote branch 'origin/krita-colorsmudge-silvioheinrich'
plassy at web.de
Sun Mar 20 17:49:40 CET 2011
On 03/18/2011 10:32 AM, Cyrille Berger Skott wrote:
> This commit is breaking the following unit test:
> Can you have a look ?
Well... I looked at it and I have some problems with this test:
QColor blue(0, 0, 255);
QColor transparentRed(255, 0, 0, 0);
// Copying something transparent over something non-transparent
should, of course, make the dst transparent
QVERIFY(memcmp(dst.data(), src.data(), cs->pixelSize()) != 0);
copyOp->composite(dst.data(), cs->pixelSize(), src.data(),
cs->pixelSize(), 0, 0, 1, 1, OPACITY_OPAQUE_U8);
QVERIFY(memcmp(dst.data(), src.data(), cs->pixelSize()) == 0);
The test expects the color from the completely transparent source pixel
(transparentRed) to be copied to the destination.
Well.. now i guess this is a matter of definition because i would say
that a fully transparent pixel has no color (color is undefined), so no
color can be copied.
My understanding of this is using the color of a fully transparent pixel
is like using an uninitialized variable in c++.
My KoCompositeOpCopy2 never touches undefined colors, so it will not
copy the color of transparent red but just the alpha channel.
The old CompositeOp does it and i think you still know the problem of
the old smudge brush that black color was mixed into the stroke.
It was because of this behavior.
So my suggestion would be to replace this specific test and test only
that the destination pixel has a zero alpha channel, because as i stated
the color is IMHO undefined.
I tried to make my CompositeOp pass the test, but every time it passes i
have the problem again that black color is mixed into the result.
So, if you don't like my first suggestion i could restore the old
KoCompositeOpCopy2 and create a new CompositeOp. KoCompositeOpCopySmudge
But i believe that there is an optimization for the COPY CompositeOp
that uses memcopy when no blending is needed, right?
Creating a new Composite op would result in lower performance then...
More information about the kimageshop