[Pixman] [PATCH v3 1/2] Remove the 8e extra safety margin in COVER_CLIP analysis

[Pekka Paalanen]

On Wed, 23 Sep 2015 11:40:32 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> From: Ben Avison <bavison at riscosopen.org>
> 
> As discussed in
> http://lists.freedesktop.org/archives/pixman/2015-August/003905.html
> 
> the 8 * pixman_fixed_e (8e) adjustment which was applied to the transformed
> coordinates is a legacy of rounding errors which used to occur in old
> versions of Pixman, but which no longer apply. For any affine transform,
> you are now guaranteed to get the same result by transforming the upper
> coordinate as though you transform the lower coordinate and add (size-1)
> steps of the increment in source coordinate space. No projective
> transform routines use the COVER_CLIP flags, so they cannot be affected.

<snip>

>  pixman/pixman.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/pixman/pixman.c b/pixman/pixman.c
> index a07c577..f932eac 100644
> --- a/pixman/pixman.c
> +++ b/pixman/pixman.c
> @@ -497,21 +497,12 @@ analyze_extent (pixman_image_t       *image,
>      if (!compute_transformed_extents (transform, extents, &transformed))
>  	return FALSE;
>  
> -    /* Expand the source area by a tiny bit so account of different rounding that
> -     * may happen during sampling. Note that (8 * pixman_fixed_e) is very far from
> -     * 0.5 so this won't cause the area computed to be overly pessimistic.
> -     */
> -    transformed.x1 -= 8 * pixman_fixed_e;
> -    transformed.y1 -= 8 * pixman_fixed_e;
> -    transformed.x2 += 8 * pixman_fixed_e;
> -    transformed.y2 += 8 * pixman_fixed_e;
> -
>      if (image->common.type == BITS)
>      {
> -	if (pixman_fixed_to_int (transformed.x1) >= 0			&&
> -	    pixman_fixed_to_int (transformed.y1) >= 0			&&
> -	    pixman_fixed_to_int (transformed.x2) < image->bits.width	&&
> -	    pixman_fixed_to_int (transformed.y2) < image->bits.height)
> +	if (pixman_fixed_to_int (transformed.x1 - pixman_fixed_e) >= 0                &&
> +	    pixman_fixed_to_int (transformed.y1 - pixman_fixed_e) >= 0                &&
> +	    pixman_fixed_to_int (transformed.x2 - pixman_fixed_e) < image->bits.width &&
> +	    pixman_fixed_to_int (transformed.y2 - pixman_fixed_e) < image->bits.height)
>  	{
>  	    *flags |= FAST_PATH_SAMPLES_COVER_CLIP_NEAREST;
>  	}

Hi,

I've been running a bunch of benchmarks around this series recently,
and there is one result I can share that I am fairly confident with.

This one patch, "Remove the 8e extra safety margin in COVER_CLIP
analysis", is having the following effect:

Speedups
========
image16        t-firefox-paintball  1013.46 (1023.32 0.66%) -> 913.53 (922.78 0.71%):  1.11x speedup

Speedups
========
image        t-firefox-paintball  791.74 (799.70 0.72%) -> 691.03 (697.58 0.76%):  1.15x speedup

This was measured on x86_64 (sandybridge laptop) and with
PIXMAN_DISABLE=wholeops. The Cairo trace is from the trimmed set.

This number is the overall average from running 'cairo-perf-trace -i16'
six times. Run by run, other traces varied between +/- 6% change,
but t-firefox-paintball was the only consistent and significant change
by eye-balling the results with cairo-perf-diff-files.

I ran cairo-perf-trace six times with identical input for both image
and image16, and strangely one run of them crashed. I've no idea why.

I tried the same on rpi1 (with only 4 internal and external iterations,
so it took just 22 hours), but the results are all over the place with
some random 70% slowdowns and speedups.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20151016/88869837/attachment.sig>