On Fri, Mar 18, 2016 at 9:54 PM, Søren Sandmann <soren.sandm...@gmail.com>
wrote:
> On Sun, Mar 6, 2016 at 8:06 PM, <spit...@gmail.com> wrote:
>
>> From: Bill Spitzak <spit...@gmail.com>
>>
>> This removes a high-frequency spike in the middle of some filters that is
>> caused by math errors all being in the same direction.
>>
>> Signed-off-by: Bill Spitzak <spit...@gmail.com>
>> ---
>> pixman/pixman-filter.c | 12 +++++++++++-
>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> index 36dd811..ab62e0a 100644
>> --- a/pixman/pixman-filter.c
>> +++ b/pixman/pixman-filter.c
>> @@ -282,8 +282,18 @@ create_1d_filter (int width,
>> p[x] = t;
>> }
>>
>> + /* Distribute any remaining error over all samples */
>> if (new_total != pixman_fixed_1)
>> - p[width / 2] += (pixman_fixed_1 - new_total);
>> + {
>> + pixman_fixed_t delta = new_total - pixman_fixed_1;
>> + pixman_fixed_t t = 0;
>> + for (x = 0; x < width; ++x)
>> + {
>> + pixman_fixed_t new_t = delta * (x + 1) / width;
>> + p[x] += new_t - t;
>> + t = new_t;
>> + }
>> + }
>>
>
> I think there is a sign error in this code: delta is new_total - 1, which
> is positive when new_total is bigger than 1. But this positive delta is
> then added to the samples, making the total even bigger.
>
> Also, I would write the code like this:
>
> pixman_fixed_t error = pixman_fixed_1 - new_total;
> for (x = 0; x < width; ++x)
> {
> pixman_fixed_t d = error * (x + 1) / width;
> p[x] += d;
> error -= d;
> }
>
> to get rid of the temporary and to make it more obvious that there is an
> error that is being distributed.
>
> Another possibility is to do error diffusion in the /* Normalize */ loop.
>
Also, a test that generates a bunch of filters and verifies that all the
phases sum to 1 would be useful.
Søren
_______________________________________________
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman
