[Pixman] [PATCH 1/2] Remove the 8e extra safety margin in COVER_CLIP analysis
Pekka Paalanen
ppaalanen at gmail.com
Fri Sep 11 04:30:23 PDT 2015
From: Ben Avison <bavison at riscosopen.org>
As discussed in
http://lists.freedesktop.org/archives/pixman/2015August/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 (size1)
steps of the increment in source coordinate space. No projective
transform routines use the COVER_CLIP flags, so they cannot be affected.
However, for COVER_CLIP_NEAREST, the actual margins added were not 8e.
Because the halfway cases round down, that is, coordinate 0 hits pixel
index 1 while coordinate e hits pixel index 0, the extra safety margins
were actually 7e to the left and up, and 9e to the right and down. This
patch removes the 7e and 9e margins and restores the e adjustment
required for NEAREST sampling in Pixman. For reference, see
pixman/rounding.txt.
For COVER_CLIP_BILINEAR, the margins were exactly 8e as there are no
additional offsets to be restored, so simply removing the 8e additions
is enough.
Proof:
All implementations must give the same numerical results as
bits_image_fetch_pixel_nearest() / bits_image_fetch_pixel_bilinear().
The former does
int x0 = pixman_fixed_to_int (x  pixman_fixed_e);
which maps directly to the new test for the nearest flag, when you consider
that x0 must fall in the interval [0,width).
The latter does
x1 = x  pixman_fixed_1 / 2;
x1 = pixman_fixed_to_int (x1);
x2 = x1 + 1;
but then x2 is brought back in range by the repeat() function, so it can't
stray beyond the end of the source line. When you write a COVER path, you
are effectively omitting the repeat() function.
As samplers are allowed to fetch the pixel at x2 unconditionally, we
require
x1 >= 0
x2 < width
so
x  pixman_fixed_1 / 2 >= 0
x  pixman_fixed_1 / 2 + pixman_fixed_1 < width * pixman_fixed_1
so
pixman_fixed_to_int (x  pixman_fixed_1 / 2) >= 0
pixman_fixed_to_int (x + pixman_fixed_1 / 2) < width
which matches the source code lines for the bilinear case, once you delete
the lines that add the 8e margin.
Signedoffby: Ben Avison <bavison at riscosopen.org>
[Pekka: adjusted commit message, left affinebench changes for another patch]
Signedoffby: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

This is v2 part 1/2 of the patch "Change conditions for setting
FAST_PATH_SAMPLES_COVER_CLIP flags".

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;
}

2.4.6
More information about the Pixman
mailing list