[Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0

[Søren Sandmann]

I don't believe this patch is correct.

As an example, consider an image with an identity transformation and a
cubic filter (which has width 4) with subsample_bits = 0. The current code
will compute a matrix

    [ cubic(-2), cubic(-1), cubic(0), cubic(1) ]

Now suppose we are filtering a pixel located at x = 17.5. The code in
bits_image_fetch_pixel_separable_convolution() will eventually end up
convolving with the pixels at locations 15.5, 16.5, 17.5 and 18.5, which is
the correct result since cubic(-2) = 0 [1].

With your code, the matrix computed would be

    [ cubic(-1.5), cubic(-0.5), cubic(0.5), cubic(1) ]

which would not be correct no matter what: With an identity transformation,
the pixel at 17.5 should be multiplied with cubic(0).

There is a detailed explanation of these issues in the file
pixman/rounding.txt.


Søren

[1] You might consider optimizing this case away and generate a matrix with
width 3, but since this would only work with subsample_bits=0, it's not
worth the special-casing.


On Sun, Mar 6, 2016 at 8:06 PM, <spitzak at gmail.com> wrote:

> From: Bill Spitzak <spitzak at gmail.com>
>
> The position of only one subsample was wrong as ceil() was done on an
> integer.
> Use a different function for all odd numbers of subsamples that gets this
> right.
>
> Signed-off-by: Bill Spitzak <spitzak at gmail.com>
> ---
>  pixman/pixman-filter.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index ac89dda..f9ad45f 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -252,7 +252,10 @@ create_1d_filter (int              width,
>          * and sample positions.
>          */
>
> -       pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
> +       if (n_phases & 1)
> +           pos = frac - width / 2.0;
> +       else
> +           pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
>
>         total = 0;
>         for (x = 0; x < width; ++x, ++pos)
> --
> 1.9.1
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pixman/attachments/20160403/acb87365/attachment-0001.html>