[Pixman] [PATCH] test: Add cover-test

[Pekka Paalanen]

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>