[cairo] [PATCH] Image scaling regression test script and 'fbCompositeSrcScaleNearest' bugfixes

[Jeff Muizelaar]

On Wed, Apr 01, 2009 at 12:20:45PM +0300, Siarhei Siamashka wrote:
> On Wednesday 01 April 2009 04:53:23 ext Soeren Sandmann wrote:
> > Hi,
> >
> > > I'm currently working on porting performance optimizations from Xomap
> > > (X server used in Nokia internet tablets N800 and N810) into the latest
> > > version of pixman and now also have a permission to submit these patches
> > > directly upstream :)
> >
> > Great! Upstream much appreciates this.
> >
> > > Xomap has optimizations which include fastpath functions for nearest
> > > neighbour scaling with the support of SRC and OVER operations for
> > > NONE and NORMAL repeat in the case when no rotation is involved and
> > > only a single clipping rectangle is used. Also it has a set of ARMv6
> > > assembly optimizations (they partially overlap with the ARM optimizations
> > > that pixman also got recently, but still have some useful stuff).
> >
> > Jeff also has some ARM optimizations here:
> >
> >     http://cgit.freedesktop.org/~jrmuizel/pixman/log/?h=neon
> 
> Yes, I have seen and tried this patch. Are there any immediate plans for
> merging it into the main branch? 

I hope to merge it soon.

> > >
> > > 'fbCompositeSrcScaleNearest' function had a problem with alpha channel
> > > in the destination image. Also NORMAL repeat was broken (the optimized
> > > function can handle repeat operation itself and can be screwed up if
> > > 'pixman_walk_composite_region' tries to help it by splitting the work
> > > into handling multiple separate areas).
> >
> > The srcRepeat = FALSE change looks correct and should give a good
> > speedup, but does it actually fix a *bug* or just a performance
> > problem?
> >
> > If fbCompositeSrcScaleNearest() actually renders incorrectly when it
> > is given 1x1 rectangles, then that's a bug in its own right that we
> > can't work around by setting srcRepeat to FALSE.
> 
> Yes, it's a rendering bug. Splitting work into handling different areas does
> not work right for the transform case (and it is never used for generic path).
> The point is that this splitting only has full pixel precision at the moment,
> while correct blitting needs to preserve some fractional part in calculations
> when moving from one "tile" to another.

I've committed this. Thanks for catching it.

> 
> This splitting actually might need to be improved later so that it could be
> used for the transform case too. It may be needed for the performance
> reasons (in order to have fast scaling functions which can handle clipping
> efficiently).
> 
> > The alpha change I don't understand. Note this line:
> >
> >          && pSrc->bits.format == pDst->bits.format
> >
> > down in the if statement; fbCompositeSrcScaleNearest() only gets
> > called when source and destination have the same format, so copying
> > pixels directly between them should be correct, and there should be no
> > need for masking in 0xff in the alpha channel.
> 
> This is a bit of gray area. This change is needed to provide exactly the same
> rendering results as the generic path. Generic path always sets the (unneeded)
> alpha bits to 0xFF if the source picture had them set to something different.
> But if the values of these bits are actually supposed to be "undefined", then
> the test program needs to be modified not to take them into account when
> checking correctness of rendering.

The alpha bits are defined as 'undefined' with x8r8g8b8 so this change
is unneeed. See fbCompositeSrc_8888xx888 as an example.

-Jeff