Branch: krita-colorchannels-silvioheinrich

[Boudewijn Rempt]

On Wednesday 13 April 2011 Apr, Silvio Heinrich wrote:
> On 04/13/2011 11:20 AM, Cyrille Berger Skott wrote:
> > Looks like it is breaking two unit tests:
> > http://my.cdash.org/viewTest.php?onlydelta&buildid=177252
> Phew.... this unit test stuff is starting to get really annoying :P

Yes, well... They are meant to catch changes in behaviour that can potentially cause big trouble.

> Every time i make some changes a couple of tests are breaking :D.

Run them before you commit? It's perfectly fine to actually change a test if we are sure we want the changed behaviour.

> Ok, to the first test "krita-plugin-format-xcf_test":
> It compares an imported xcf file with a png file that contains the 
> expected result.
> Thats fine so far. I wanted to look what's going wrong, so i opened the 
> xcf file in gimp and in krita...
> but i couldn't see any difference. I looked again at the output of the 
> unit test and there was written:
>      DEBUG : KisXCFTest::testFiles() Comparison failures:  
> ("subtract-multiply-masks.xcf: Pixel (28,10) has different values")
> 
> So i looked what values the Pixel 28,10 has in gimp and what value it 
> has in Krita:
> Krita  -> RGB(216,214,214)
> Gimp -> RGB(216,213,214)
> 
> Seems like the two pictures are compared too exact.
> I would say it is completely acceptable if the result differs slightly 
> to the expected result.
> I think a tolerance of 1-2 units is fine (there shouldn't be any visual 
> difference noticeable).

I can agree to that -- it's fine to change that test to have a bit of tolerance.

> Otherwise we would limit ourself to implement every blending mode 
> absolutely exact (without any optimization)
> or to do it exactly like gimp/photoshop or whatever...
> So my point is that we should compare just the visual result and not the 
> exact pixel values (what doesn't work with floating point color spaces 
> anyway).
> We should compare the images with a tolerance here.


> The second test "krita-image-KisPainterTest":
> I actually can't remember messing around with the selection...
> But maybe i don't understand the test right

> 
> it fails here -> QCOMPARE(dst2->exactBounds(), QRect(0, 0, 64, 64)); 
> with the line:
> 
> FAIL!  : KisPainterTest::testPaintDeviceBltSelection() Compared values 
> are not the same
>     Actual (dst2->exactBounds()): QRect(10,10 10x10) (bottomright 19,19)
>     Expected (QRect(0, 0, 64, 64)): QRect(0,0 64x64) (bottomright 63,63)
>     Loc: 
> [/home/silvio/kde4/src/calligra/krita/image/tests/kis_painter_test.cpp(160)]
> 
> For me it seems that the second test creates a paint device (dst2) and 
> sets a selection XYWH(10,10,20,20) then
> it blits the source device (src) which is XYWH(0,0,20,20) to dst2 with 
> the statement "painter2.bitBlt(0, 0, src, 0, 0, 30, 30);".
> but src is WH(20,20) not WH(30,30) ??
> And then at the end the bounds of dst2 are expected to be XYWH(0,0,64,64) ??
> Why should dst2 extend to WH(64,64) here?

I think this is because a selection changes the meaning of exactBounds. But we went over this issue a lot of times, and I'd really like Dmitry's opinion here.

> 
> I actually don't know what i broke here...
> 
> 


-- 
Boudewijn Rempt | http://www.valdyas.org, http://www.krita.org