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 <spit...@gmail.com> 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 <soren.sandm...@gmail.com> > 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, <spit...@gmail.com> wrote: >> >>> From: Bill Spitzak <spit...@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 <spit...@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@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/pixman >>> >> >> >
_______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman