[Pixman] [PATCH 10/15] pixman-filter: don't range-check impulse filter

[Oded Gabbay]

On Tue, Dec 22, 2015 at 10:51 PM, Bill Spitzak <spitzak at gmail.com> wrote:
> The problem is that *Cairo* does not call this function. This is because
> Cairo already has my patches which work around the broken filtering by
> generating it's own filtering. The whole point of this series of patches is
> so that this work-around can be removed from Cairo.
>

So my response here is at the same I wrote about patch 7/15 (defer
until cairo will actually use this function), only here we are talking
about showing results of cairo test suite and not benchmarking.

Thanks,

       Oded

>
> On Tue, Dec 22, 2015 at 12:12 PM, Oded Gabbay <oded.gabbay at gmail.com> wrote:
>>
>> On Tue, Dec 22, 2015 at 9:25 PM, Bill Spitzak <spitzak at gmail.com> wrote:
>> >
>> >
>> > On Tue, Dec 22, 2015 at 2:32 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>
>> >> >
>> >> > The other filters don't range-check, so there is no need for this
>> >> > one to either. It is only called with x==0.
>> >>
>> >> Actually, I tried to stop at this function in gdb and didn't manage to
>> >> do it (using the scale demo). I then looked at the code and it seems
>> >> to me that the only way to reach this function is when both
>> >> reconstruction and sample kernels are IMPLUSE. That's because:
>> >>
>> >> 1. If both reconstruction and sample are *not* IMPLUSE, then of course
>> >> we won't reach it.
>> >> 2. If only one of them is IMPLUSE, than the code will immediately
>> >> return the value of the function of the other kernel, which is *not*
>> >> IMPLUSE.
>> >>
>> >> However, when I put both of them to IMPLUSE in the scale demo, the
>> >> picture simply disappears *and* the impluse_kernel is still not
>> >> reached. Actually, in that case, the integral() func is never reached
>> >> as well.
>> >>
>> >> What am I missing ?
>> >
>> >
>> > I believe at this point the calling code calculated a width of zero for
>> > the
>> > filter, and this caused all kinds of problems.
>> >
>> > I think you are correct that in most or all versions of this code, that
>> > impulse function is never called, and it could be a null pointer
>> > instead.
>>
>> Well, I wouldn't go that far, but what I'm implying is maybe we can
>> defer this patch until a time when pixman's code will actually call
>> this function. Then, we can re-evaluate the patch based on the inputs
>> we will see.
>>
>>        Oded
>>
>> >>
>> >>
>> >>        Oded
>> >>
>> >> > ---
>> >> >  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 fbc657d..00126cd 100644
>> >> > --- a/pixman/pixman-filter.c
>> >> > +++ b/pixman/pixman-filter.c
>> >> > @@ -45,7 +45,7 @@ typedef struct
>> >> >  static double
>> >> >  impulse_kernel (double x)
>> >> >  {
>> >> > -    return (x == 0.0)? 1.0 : 0.0;
>> >> > +    return 1;
>> >> >  }
>> >> >
>> >> >  static double
>> >> > --
>> >> > 1.9.1
>> >> >
>> >> > _______________________________________________
>> >> > Pixman mailing list
>> >> > Pixman at lists.freedesktop.org
>> >> > http://lists.freedesktop.org/mailman/listinfo/pixman
>> >
>> >
>
>