[cairo] The short road left to 1.6 - EXTEND_PAD

[Bertram Felgenhauer]

Antoine Azar wrote:
> Here's my contribution to 1.6. This adds extend_pad support to pixman (as 
> in my previous patch) but also cleans up the whole fbfetchtransformed 
> function that used to be almost 700 lines long. I broke it down in separate 
> functions, and reduced some of the code duplication.

Looks good to me, code-wise. However, the indentation is messed up
badly, and there are a few other style nits.

> I made my own perf test in Cairo to stress this function and the new code 
> should be either as fast or (very slightly) faster than the previous one.

I didn't test the performance impact of this patch.

> This patch should be "apply and play" with pixman's current repository.

There's a conflict with commit 1d89bac5a7a5693911d8a74701bd1c0292160478
-- fix cairo's  a1-image-sample  test.

> diff --git a/pixman/pixman-compose.c b/pixman/pixman-compose.c
> index c3f50e2..e09c809 100644
> --- a/pixman/pixman-compose.c
> +++ b/pixman/pixman-compose.c
> @@ -124,11 +124,6 @@ SourcePictureClassify (source_image_t *pict,
>  	int offset1 = stride < 0 ? \
>  		offset0 + ((-stride) >> 1) * ((pict->height) >> 1) : \
>  		offset0 + (offset0 >> 2)
> -/* Note n trailing semicolon on the above macro; if it's there, then
> - * the typical usage of YV12_SETUP(pict); will have an extra trailing ;
> - * that some compilers will interpret as a statement -- and then any further
> - * variable declarations will cause an error.
> - */

Is there any good reason to remove that comment?

> @@ -3678,65 +3673,70 @@ static void pixmanFetchSourcePict(source_image_t * pict, int x, int y, int width
>      }
>  }
>  
> -static void fbFetchTransformed(bits_image_t * pict, int x, int y, int width, uint32_t *buffer, uint32_t *mask, uint32_t maskBits)
> +//FETCH FROM REGION STRATEGIES//////////////////////////////////////////////////////////////////////////////////////////////////////////

Please use C89 comments - /* ... */.

/*
 * Fetching from from regions
 */

perhaps?

> -    /* reference point is the center of the pixel */
> -    v.vector[0] = pixman_int_to_fixed(x) + pixman_fixed_1 / 2;
> -    v.vector[1] = pixman_int_to_fixed(y) + pixman_fixed_1 / 2;

The conflict is here - these lines now read

| -    v.vector[0] = pixman_int_to_fixed(x) + pixman_fixed_1 / 2 - 1;
| -    v.vector[1] = pixman_int_to_fixed(y) + pixman_fixed_1 / 2 - 1;

> +    //intialize the two function pointers

Typo, which is repeated several times in the code.

> +    /* reference point is the center of the pixel */
> +    v.vector[0] = pixman_int_to_fixed(x) + pixman_fixed_1 / 2;
> +    v.vector[1] = pixman_int_to_fixed(y) + pixman_fixed_1 / 2;

You'll have to add the - 1; here.

Bertram