I believe there may have been a rebasing error. Will look into it. I certainly intended this to change behavior when the filter size is odd, not when the number of subsamples is odd. Not sure if this is truly rebase screwing up, or I just mistyped an attempt to fix a rebase error.
On Mon, Apr 4, 2016 at 12:21 PM, Søren Sandmann <[email protected]> wrote: > No, it changes behavior when the number of *phases* is odd, which it is in > the example I gave. > > > Søren > > On Mon, Apr 4, 2016 at 2:51 PM, Bill Spitzak <[email protected]> wrote: > >> That's why this only changes the behavior when the number of samples is >> odd. >> >> >> On Sun, Apr 3, 2016 at 11:17 AM, Søren Sandmann <[email protected] >> > 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. >>> >>> >>> 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, <[email protected]> wrote: >>> >>>> From: Bill Spitzak <[email protected]> >>>> >>>> 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 <[email protected]> >>>> --- >>>> 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 >>>> [email protected] >>>> https://lists.freedesktop.org/mailman/listinfo/pixman >>>> >>> >>> >> >
_______________________________________________ Pixman mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/pixman
