[Pixman] [PATCH 09/15] pixman-filter: put filter error on center pixel

[Oded Gabbay]

On Tue, Dec 22, 2015 at 9:23 PM, Bill Spitzak <spitzak at gmail.com> wrote:
> This is forcing the filter to be normalized (sum to 1.0) despite any math
> errors.
>
> p is a post-increment pointer used to store the filter values, so it now
> points after the last filter value. The filter is summed and the difference
> from 1.0 is then added to the middle pixel with this code.
>
> If the width is an odd number such as 5, the filter is stored in f[0]..f[4],
> and p points at f[5]. The old code would add the extra error value to f[3],
> which is not the center sample. The new code adds it to f[2]. (If the width
> is even such as 4, then both the old and new code add the error to the f[2]
> which is the pixel to the right of the center.)
>
> The mistake was only visible in the width==1 case. Some combinations of
> filters produced a sample of 0, and then calculated the error as 1.0. The
> old code would put the error on f[1] (past the end). The new code puts it on
> f[0], thus producing a value of 1.0.
>

ok, so please add this to the commit msg and this patch is:

Acked-by: Oded Gabbay <oded.gabbay at gmail.com>

> On Tue, Dec 22, 2015 at 2:41 AM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>>
>> On Sat, Dec 12, 2015 at 8:06 PM,  <spitzak at gmail.com> wrote:
>> > From: Bill Spitzak <spitzak at gmail.com>
>> >
>> > Any error in filter normalization is placed on the center of odd-sized
>> > filters,
>> > rather than 1 pixel to the right.
>> >
>> > In particular this fixes the 1-wide filters produced by impulse sampling
>> > so they are 1.0 rather than 0.0.
>> > ---
>> >  pixman/pixman-filter.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> > index 0cd4a68..fbc657d 100644
>> > --- a/pixman/pixman-filter.c
>> > +++ b/pixman/pixman-filter.c
>> > @@ -299,7 +299,7 @@ create_1d_filter (int              width,
>> >         }
>> >
>> >         if (new_total != pixman_fixed_1)
>> > -           *(p - width / 2) += (pixman_fixed_1 - new_total);
>> > +           *(p - (width + 1) / 2) += (pixman_fixed_1 - new_total);
>> >      }
>> >  }
>> >
>> > --
>> > 1.9.1
>> >
>> > _______________________________________________
>> > Pixman mailing list
>> > Pixman at lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/pixman
>>
>>
>> I see that the result is indeed better (in the scale demo), but do you
>> mind explaining a bit more about the underlying of this patch ?
>> I indeed see that the width is 1, so without your patch, the new value
>> is written to *p and with your patch, it is written to *(p-1). But I
>> have no idea what that means :(
>>
>>         Oded
>
>