Ben Avison bavison at riscosopen.org
Mon Aug 24 13:42:01 PDT 2015

A number of fast paths exist which only support "cover" type plots. These
are operations where once we transform the complete set of destination
pixels required for the plot into the source image's coordinate space, all
the required source pixels fall within the source's array bounds. This
means that there is no need for bounds checking in the inner loop, and the
selected repeat type (none, normal (aka tiled), pad, or reflect) has no
impact upon the result.

Without the needs for bounds checking, a "cover" fast path will always be
faster than an equivalent fast path which uses one of the four repeat types,
so it is unfortunate that the previous test in analyze_extent() for
setting the flag was too strict, resulting in the "cover" fast paths being
passed over in some cases where they could have been used instead of the
matching "repeat" fast path.

The case of nearest neighbour scaling is a bit more straightforward than
bilinear scaling, because each output pixel corresponds to exactly one
input pixel. FAST_PATH_SAMPLES_COVER_CLIP_NEAREST only ever occurs in
fast paths (or scanline fetch iters) for affine transforms - albeit often
more specifically they also have FAST_PATH_SCALE_TRANSFORM,
scaling used) or even FAST_PATH_ID_TRANSFORM. Therefore, each destination
pixel can be mapped directly to a 16.16 fixed-point coordinate in source
space without any rounding being performed (as was inferred by the comment
deleted in the previous commit).

Here is a worked example of where the previous code would have selected
the wrong fast path. Consider a source image of size 2 x 1 being plotted
to a 3 x 2 region at the top-left of the destination using the
transformation matrix

    / 0.5 0   0.25 \
T = | 0   0.5 0.25 |
    \ 0   0   1    /

Pixman samples at the centre of pixels, so for the second row of pixels,
it would pass a y value of 1.5 into this matrix, generating a source y
coordinate of exactly 1.0 (or pixman_fixed_1 in 16.16 format). Because
Pixman's fast paths always subtract pixman_fixed_e before shifting by 16,
this is a valid coordinate for a 1-pixel high source image (in fact it's
the highest permissable such coordinate) but by adding 8 * pixman_fixed_e
first, the old code would have incorrectly deduced it to be out-of-bounds.

After this patch, cover-test still passes, at least for the ARMv7, ARMv6
and generic C fast paths and fetchers.
 pixman/pixman.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pixman/pixman.c b/pixman/pixman.c
index 982c820..e8290d9 100644
--- a/pixman/pixman.c
+++ b/pixman/pixman.c
@@ -499,10 +499,10 @@ analyze_extent (pixman_image_t       *image,
     if (image->common.type == BITS)
-	if (pixman_fixed_to_int (transformed.x1 - 8 * pixman_fixed_e) >= 0                &&
-	    pixman_fixed_to_int (transformed.y1 - 8 * pixman_fixed_e) >= 0                &&
-	    pixman_fixed_to_int (transformed.x2 + 8 * pixman_fixed_e) < image->bits.width &&
-	    pixman_fixed_to_int (transformed.y2 + 8 * pixman_fixed_e) < 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)

More information about the Pixman mailing list