<div dir="ltr"><div><div>I don't believe this patch is correct.<br><br>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<br><br></div>    [ cubic(-2), cubic(-1), cubic(0), cubic(1) ]<br><br></div><div>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].<br><br>With your code, the matrix computed would be<br><br></div><div>    [ cubic(-1.5), cubic(-0.5), cubic(0.5), cubic(1) ]<br><br></div><div>which would not be correct no matter what: With an identity transformation, the pixel at 17.5 should be multiplied with cubic(0).<br><br>There is a detailed explanation of these issues in the file pixman/rounding.txt.<br><br></div><div><br>Søren<br></div><div><br>[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.<br><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Mar 6, 2016 at 8:06 PM,  <span dir="ltr"><<a href="mailto:spitzak@gmail.com" target="_blank">spitzak@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Bill Spitzak <<a href="mailto:spitzak@gmail.com">spitzak@gmail.com</a>><br>
<br>
The position of only one subsample was wrong as ceil() was done on an integer.<br>
Use a different function for all odd numbers of subsamples that gets this right.<br>
<br>
Signed-off-by: Bill Spitzak <<a href="mailto:spitzak@gmail.com">spitzak@gmail.com</a>><br>
---<br>
 pixman/pixman-filter.c | 5 ++++-<br>
 1 file changed, 4 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c<br>
index ac89dda..f9ad45f 100644<br>
--- a/pixman/pixman-filter.c<br>
+++ b/pixman/pixman-filter.c<br>
@@ -252,7 +252,10 @@ create_1d_filter (int              width,<br>
         * and sample positions.<br>
         */<br>
<br>
-       pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;<br>
+       if (n_phases & 1)<br>
+           pos = frac - width / 2.0;<br>
+       else<br>
+           pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;<br>
<br>
        total = 0;<br>
        for (x = 0; x < width; ++x, ++pos)<br>
<span class="HOEnZb"><font color="#888888">--<br>
1.9.1<br>
<br>
_______________________________________________<br>
Pixman mailing list<br>
<a href="mailto:Pixman@lists.freedesktop.org">Pixman@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/pixman" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/pixman</a><br>
</font></span></blockquote></div><br></div>