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

[Bill Spitzak]

On 04/09/2016 01:13 PM, Søren Sandmann wrote:
> On Sat, Apr 9, 2016 at 3:45 PM, Bill Spitzak <spitzak at gmail.com
> <mailto:spitzak at gmail.com>> wrote:
>
>     On 04/03/2016 11:17 AM, Søren Sandmann wrote:
>
>         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.
>
>
>     I have finally checked this and it is producing the correct value in
>     the above example. pos is not the left edge of the range to
>     integrate, it is the center of it, due to the changes I made to the
>     integrate function.
>
>
> No, it really doesn't produce the correct values. If you apply the patch
> below on top of your series and run demos/scale with
>
>      subsample set to 0,
>      reconstruct set to 'Cubic',
>      sample set to 'Impulse'
>      scale factor set to 1.0,
>
> it prints out this:
>
>      cubic(1.500000) = -0.034722
>      cubic(0.500000) = 0.534722
>      cubic(-0.500000) = 0.534722
>      cubic(-1.500000) = -0.034722
>      [ -0.035, 0.535, 0.535, -0.035, ]
>
> which is not the correct result. The first value should be cubic(-2) = 0.
>
> And if you toggle back and forth between subsamples = 0 and 1, you can
> see the image shift slightly.
>
> (The *gnuplot* graph looks correct, but that's because the gnuplot
> function is also generating the wrong coordinates).

It does look like there is something really wrong. I compared and 
(except for the subsample_bits==0 case) my version produces the same 
output as the current git head.

I think your intention is that there is a sample at offset=0 whether the 
filter width is even or odd. However (except when subsample_bits==0) the 
filter generator makes a symmetric filter for even sizes, with two equal 
samples around the maximum center value. If a sample was at offset==0 
then it would be unique and larger than all the other samples.

Comparing box+box with subsample_bits==1 with bilinear, it does appear 
there is an offset and the filter is wrong. It matches exactly (in your 
version) when subsample_bits==0.

The gnuplot output is wrong because I adjusted it to make this output 
look correct.

I then further broke the only case that actually worked (when 
subsample_bits==0) with this patch, since it did not match the others.

I need to look at the actual filter sampling code and at your paper I 
guess. I think that is doing the right thing, and the generator and 
gnuplot have to be adjusted to match.