[Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0
Bill Spitzak
spitzak at gmail.com
Mon Apr 11 03:34:30 UTC 2016
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.
More information about the Pixman
mailing list