Re: [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  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 

> On Tue, Dec 22, 2015 at 2:41 AM, Oded Gabbay  wrote:
>>
>> On Sat, Dec 12, 2015 at 8:06 PM,   wrote:
>> > From: Bill Spitzak 
>> >
>> > 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@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
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

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

[Bill Spitzak]

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.

On Tue, Dec 22, 2015 at 2:41 AM, Oded Gabbay  wrote:

> On Sat, Dec 12, 2015 at 8:06 PM,   wrote:
> > From: Bill Spitzak 
> >
> > 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@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
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

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

[Oded Gabbay]

On Sat, Dec 12, 2015 at 8:06 PM,   wrote:
> From: Bill Spitzak 
>
> 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@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
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

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

[spitzak]

From: Bill Spitzak 

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@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman