[cairo] [PATCH] pixman: fast path for nearest neighbour scaled compositing operations.

[Jeff Muizelaar]

On Thu, May 28, 2009 at 05:05:55AM +0300, Siarhei Siamashka wrote:
> Hello,
> 
> This is more complete implementation of nearest neighbor scaling fastpath
> functions. OVER 8888x8888, OVER 8888x0565, SRC 8888x8888,
> SRC 8888x0565 and SRC 0565x0565 cases are supported.
> 

Great to see this work. Here are some quick comments:

> +static void fbCompositeTransformNearestNonrotatedAffineTrivialclipOver_8888x0565 (
> +    pixman_image_t *pSrc, pixman_image_t *pDst, int xSrc, int ySrc, int xDst, int yDst,
> +    int width, int height, int32_t vv0, int32_t vv1, int32_t uv0, int32_t uv1)
> +{

I think these fuctions would be easier to understand if the variables
were renamed:
vv0 -> vx;
vv1 -> vy;
uv0 -> unit_x;
uv1 -> unit_y;

I think x and y a are clearer then 0 and 1

> +    uint16_t *dstLine;
> +    uint32_t *xbits;
> +    uint32_t  d;
> +    uint32_t  s1, s2;
> +    uint8_t   a1, a2;

Overall more explanatory variable names would make understanding easier.
Perhaps alpha1 instead of just a1?

> +            if (a1 == 0xff)
> +                WRITE(pDst, dst, cvt8888to0565(s1));
> +            else if (s1) {
> +                d = cvt0565to0888(READ(pDst, dst));
> +                a1 ^= 0xff;

things like this could also maybe use a comment or a macro:
e.g. //equivalent to 255 - a1
in fact is a1 ^= 0xff actually faster than a1 = 255 - a1?

> +#define FbByteMulAdd_store_a8r8g8b8(x, a, y) do {                   \
> +        uint32_t t = ((x & 0xff00ff) * a) + 0x800080;               \
> +        t = (t + ((t >> 8) & 0xff00ff)) >> 8;                       \
> +        t &= 0xff00ff;                                              \
> +        t += y & 0xff00ff;                                          \
> +        t |= 0x1000100 - ((t >> 8) & 0xff00ff);                     \
> +        t &= 0xff00ff;                                              \
> +                                                                    \
> +        x = (((x >> 8) & 0xff00ff) * a) + 0x800080;                 \
> +        x = (x + ((x >> 8) & 0xff00ff)) >> 8;                       \
> +        x &= 0xff00ff;                                              \
> +        x += (y >> 8) & 0xff00ff;                                   \
> +        x |= 0x1000100 - ((x >> 8) & 0xff00ff);                     \
> +        x &= 0xff00ff;                                              \
> +        x <<= 8;                                                    \
> +        x += t;                                                     \
> +    } while (0)

This looks like a copy of FbByteMulAdd

-Jeff