<div dir="ltr">I believe there may have been a rebasing error. Will look into it.<div><br></div><div>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.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 4, 2016 at 12:21 PM, Søren Sandmann <span dir="ltr"><<a href="mailto:soren.sandmann@gmail.com" target="_blank">soren.sandmann@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>No, it changes behavior when the number of *phases* is odd, which it is in the example I gave.<span class="HOEnZb"><font color="#888888"><br><br><br></font></span></div><span class="HOEnZb"><font color="#888888">Søren<br></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Apr 4, 2016 at 2:51 PM, Bill Spitzak <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"><div dir="ltr">That's why this only changes the behavior when the number of samples is odd.<div><br></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Apr 3, 2016 at 11:17 AM, Søren Sandmann <span dir="ltr"><<a href="mailto:soren.sandmann@gmail.com" target="_blank">soren.sandmann@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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"><div><div>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></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>From: Bill Spitzak <<a href="mailto:spitzak@gmail.com" target="_blank">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" target="_blank">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>
</div></div><span><font color="#888888"><span><font color="#888888">--<br>
1.9.1<br>
<br>
_______________________________________________<br>
Pixman mailing list<br>
<a href="mailto:Pixman@lists.freedesktop.org" target="_blank">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></font></span></blockquote></div><br></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>