[Pixman] [PATCH] test: Add cover-test
Pekka Paalanen
ppaalanen at gmail.com
Wed Sep 2 04:56:40 PDT 2015
Hi Ben,
here I'm only replying to the questions that are not yet completely
clear to me.
On Wed, 26 Aug 2015 21:26:57 +0100
"Ben Avison" <bavison at riscosopen.org> wrote:
> On Thu, 04 Jun 2015 14:00:30 +0100, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>
> > On Tue, 26 May 2015 23:58:30 +0100
> > Ben Avison <bavison at riscosopen.org> wrote:
> >
> >> +static pixman_fixed_t
> >> +calc_translate (int dst_size,
> >> + int src_size,
> >> + pixman_fixed_t scale,
> >> + pixman_bool_t low_align,
> >> + pixman_bool_t bilinear)
> >> +{
> >> + pixman_fixed_t ref_src, ref_dst, scaled_dst;
> >> +
> >> + if (low_align)
> >> + {
> >> + ref_src = bilinear ? pixman_fixed_1 / 2 : pixman_fixed_e;
> >
> > Why the bilinear case is not pixman_fixed_1 / 2 + pixman_fixed_e?
>
> At this point we're determining the translations required so that the
> first destination pixel, when transformed into source space, has exactly
> the lowest value it can have without straying into the repeat zones.
>
> With bilinear scaling, a coordinate of pixman_fixed_1 / 2 has pixel value
> determined only by source pixel 0. A coordinate of pixman_fixed_1 * 3 / 2
> has pixel value determined only by source pixel 1. Any value in between
> has to be assumed to contain elements from both pixels 0 and 1.
> With nearest scaling, any coordinate between pixman_fixed_e and
> pixman_fixed_1 inclusive is determined by source pixel 0. Yes, this is
> slightly counter-intuitive, but it's the way Pixman has always done it.
> See for example in bits_image_fetch_nearest_affine() in
> pixman-fast-path.c where we have the lines
>
> x0 = pixman_fixed_to_int (x - pixman_fixed_e);
> y0 = pixman_fixed_to_int (y - pixman_fixed_e);
Ok, I can take that as a given.
> There is no equivalent offset by pixman_fixed_e for bilinear scaling, for
> example in fast_bilinear_cover_iter_init() also in pixman-fast-path.c:
>
> info->x = v.vector[0] - pixman_fixed_1 / 2;
> info->y = v.vector[1] - pixman_fixed_1 / 2;
>
> All the other implementations follow suit (they have to, or they wouldn't
> pass the "make check" suite.)
Right. I looked at fast_bilinear_cover_iter_init() and
fast_fetch_bilinear_cover() -> fetch_horizontal(), and yes, that really
is how it is implemented. The leftmost pixel is chosen essentially by
n = pixman_fixed_to_int(x - pixman_fixed_1 / 2).
So, pixman_fixed_1 / 2 -> 0, not -1.
The thing that confuses me is that with nearest, x=0 will sample pixel
n=-1. However with bilinear, x=0.5 will sample pixels n=0 and n=1, not
n=-1 and n=0.
That alone is surprising, but I am further confused by the talk about
the sampling case n=-1 & n=0:
http://lists.freedesktop.org/archives/pixman/2015-August/003911.html
where you refer to the very end of:
http://lists.freedesktop.org/archives/pixman/2014-October/003457.html
It's exactly the question of whether to sample the next pixel to the
left or to the right of the sampling point.
Eh, maybe I'm trying too hard. I do believe your code is correct, but
in that case I have a follow-up question on check_transform() in my
following patch review email.
> > - Looks like it processes the whole stride, not width * bytespp, which
> > means this is going to explode on fenced images in the padding,
> > right?
>
> Yes, looks like it is. Good spot. That probably does belong in a separate
> patch; are you volunteering?
I should, shouldn't I? :-)
I'll do it.
> >> * We need to test alignment with both the extreme minimum and extreme
> >> maximum source coordinates permitted. Given that the scale factors
> >> can't be assumed to be nice numbers, I think we need to test them
> >> independently, using the translation offsets in the transformation
> >> matrix to ensure they align as desired.
> >
> > To do?
>
> No - that's covered by supporting both left- and right-aligning (and top-
> and bottom-) in calc_translate(). Translation is done entirely using the
> transformation matrix because the src_x, src_y, mask_x and mask_y
> parameters to pixman_image_composite() only allow for integer positions
> in source/mask image coordinates and we need 16.16 fixed-point precision.
Ok, I think I misunderstood. I was thinking of the range of 16.16 fp
numbers in general, but that's going a bit off-topic here, too.
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/20150902/ca5bcd2a/attachment.sig>
More information about the Pixman
mailing list