[Pixman] [PATCH v14 12/22] pixman-filter: fix subsample_bits == 0
soren.sandmann at gmail.com
Mon Apr 4 19:21:15 UTC 2016
No, it changes behavior when the number of *phases* is odd, which it is in
the example I gave.
On Mon, Apr 4, 2016 at 2:51 PM, Bill Spitzak <spitzak at gmail.com> wrote:
> That's why this only changes the behavior when the number of samples is
> On Sun, Apr 3, 2016 at 11:17 AM, Søren Sandmann <soren.sandmann at gmail.com>
>> 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 .
>> 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
>>  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
>>> 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)
>>> Pixman mailing list
>>> Pixman at lists.freedesktop.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Pixman