On Mon, Mar 21, 2016 at 7:41 PM, Søren Sandmann <soren.sandm...@gmail.com> wrote:
> > > On Mon, Mar 21, 2016 at 12:29 PM, Bill Spitzak <spit...@gmail.com> wrote: > >> On Sun, Mar 20, 2016 at 11:36 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> >>>> >>>> The IMPULSE special-cases did not sample the center of the of the >>>> region. This >>>> caused it to sample the filters outside their range, and produce >>>> assymetric >>>> filters and other errors. Fixing this required changing the arguments to >>>> integral() so the correct point could be determined. >>>> >>> >>> I don't understand what is wrong and why this patch fixes it. Which >>> region precisely did not have its center sampled? When IMPULSE filters are >>> involved the width of the integral is 0 so there isn't really any "region" >>> to sample. >>> >>> Can you give a concrete example where the previous code produced >>> asymmetric filters? Also, what "other errors" was produced? I think these >>> examples should be added to the commit log. >>> >> >> It sampled the *other* filter (the one that is not impulse) at the left >> edge of the region being passed, rather than at the location of the center >> of the impulse filter. This was detected by putting asserts in the filter >> functions to see if they were being called outside their width. >> > > I tried adding such asserts and I couldn't make them trigger with the > scale demo. It would be helpful if you could give a specific pair of > filters and scale factor where a filter is sampled outside its width. > > And it really doesn't make sense to talk about the "region being passed" > when one of the filters is IMPULSE. In that case, the width parameter is > always 0 so there is no "region". > Your version relies on width==0 when either filter is impulse. This is how it is being called right now, but I think it a very good idea to avoid this. This patch series does not include fixes I attempted to make the picture not vanish when impulse+impulse was selected. Those changes made the impulse filter act like it had a width of 1 and easily triggered the asserts. I think my version is a lot clearer. The center of the sampling filter is at pos, an argument to the function, and the range for the integral is separated from the alignment and scale. I think it is obvious that this reduces the size of both the implementation and the calling code, in particular the sampling only needs a single argument rather than two.
_______________________________________________ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman