Submitted by Søren Sandmann on April 12, 2016, 8:55 p.m.

Message ID | CAERVUMp4r0m6Nc7=yhgUGhCJCbvoKfPToNtnL9eRLf4nfUG8Sg@mail.gmail.com |
---|---|

State | Under Review |

Series | "Series without cover letter" |

Headers | show |

diff --git a/pixman/rounding.txt b/pixman/rounding.txt > >> index b52b084..1c00019 100644 >> --- a/pixman/rounding.txt >> +++ b/pixman/rounding.txt >> @@ -160,6 +160,7 @@ which means the contents of the matrix corresponding >> to (frac) should >> contain width samplings of the function, with the first sample at: >> >> floor (frac - (width - 1) / 2.0 - e) + 0.5 - frac >> + = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac >> > > Unfortunately this produces numbers that don't agree with the filter > generator or filtering code. > > For a width==4 filter with n_phases==1, the generator seems to produce > values at -1, 0, 1, 2, so the first sample is at -1. It also appears the > filtering sampler is using the same rule, otherwise these filters would > produce an obvious shift in the image. > If you add printf's on top of this series diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c index 176dfae..4a8743e 100644 --- a/pixman/pixman-filter.c +++ b/pixman/pixman-filter.c @@ -129,12 +129,15 @@ general_cubic (double x, double B, double C) static double cubic_kernel (double x) { + double v = general_cubic (x, 1/3.0, 1/3.0); + + printf ("cubic(%f) => %f\n", x, v); /* This is the Mitchell-Netravali filter. * * (0.0, 0.5) would give us the Catmull-Rom spline, * but that one seems to be indistinguishable from Lanczos2. */ - return general_cubic (x, 1/3.0, 1/3.0); + return v; } and run scale with Reconstruction=Cubic, Sample=Impulse, and Subsample=0,

On Tue, Apr 12, 2016 at 1:55 PM, Søren Sandmann <soren.sandmann@gmail.com> wrote: > > 1-wide filters - looks triangular, but a 1-wide box would be more >>> accurate >>> >> >> Because you are not plotting the two dummy points at (0,±width/2), a >> 1-wide filter is actually just a single point. >> >> You may be right that leaving the dummy points off the plot may make it >> easier to figure out what is going on. >> > > Okay, I will update the comment. > > I don't like to make up fake data points, but maybe I should add > [x=-width/2:width/2] to the gnuplot command line. > Yes that sounds like a very good idea. > > diff --git a/pixman/rounding.txt b/pixman/rounding.txt >> >>> index b52b084..1c00019 100644 >>> --- a/pixman/rounding.txt >>> +++ b/pixman/rounding.txt >>> @@ -160,6 +160,7 @@ which means the contents of the matrix corresponding >>> to (frac) should >>> contain width samplings of the function, with the first sample at: >>> >>> floor (frac - (width - 1) / 2.0 - e) + 0.5 - frac >>> + = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac >>> >> >> Unfortunately this produces numbers that don't agree with the filter >> generator or filtering code. >> >> For a width==4 filter with n_phases==1, the generator seems to produce >> values at -1, 0, 1, 2, so the first sample is at -1. It also appears the >> filtering sampler is using the same rule, otherwise these filters would >> produce an obvious shift in the image. >> > > If you add printf's on top of this series > > diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c > index 176dfae..4a8743e 100644 > --- a/pixman/pixman-filter.c > +++ b/pixman/pixman-filter.c > @@ -129,12 +129,15 @@ general_cubic (double x, double B, double C) > static double > cubic_kernel (double x) > { > + double v = general_cubic (x, 1/3.0, 1/3.0); > + > + printf ("cubic(%f) => %f\n", x, v); > /* This is the Mitchell-Netravali filter. > * > * (0.0, 0.5) would give us the Catmull-Rom spline, > * but that one seems to be indistinguishable from Lanczos2. > */ > - return general_cubic (x, 1/3.0, 1/3.0); > + return v; > } > > and run scale with Reconstruction=Cubic, Sample=Impulse, and Subsample=0, > it prints > > cubic(-2.000000) => 0.000000 > cubic(-1.000000) => 0.055556 > cubic(0.000000) => 0.888889 > cubic(1.000000) => 0.055556 > > so the filter generator *does* generate values at [ -2, -1, 0, 1 ]. And as > mentioned, the sampling code, if given the value 17.5 will convolve with > the pixels at 15.5, 16.5, 17.5, 18.5, so I'm pretty sure this matrix is > correct. > > (I suspect your changes to the integral() arguments caused it to generate > different values, so you should check without them included. > Looks like you are correct, toggling between this and nearest shows that the filtering code is reading the arrays this way. > > This series is available as a git repository here: > > > https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master > ) > > > Søren > >

On 04/12/2016 01:55 PM, Søren Sandmann wrote: > This series is available as a git repository here: > > https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master Not having much luck with that url: fatal: repository 'https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master/' not found

Never mind, I should have just clicked on that rather than thinking it was a repository name. I am able to compile your version and run it and the output looks correct. Changing the plot line to this: printf ("plot [x=%g:%g] '-' with linespoints ls 1\n", -width*0.5, width*0.5); does make it work better, especially for subsample=0, by making the range stay fixed. Also if you run it on an older gnuplot that does not understand "pi" you get a lot of warning messages on stderr. It would help a *lot* if you added the pulldown to change the type of filter to the scale demo, so I would recommend adding those patches as well. Being able to directly compare to NEAREST and BILINEAR helps a lot with debugging filters. As is I was only able to confirm the handling of subsample==0 was correct by typing '8' into the subsample field to get a hi-res filter. subsample=1 produces a quite visible shift that I think is a good indication the sampling locations will need to be fixed. On Wed, Apr 13, 2016 at 7:24 AM, Bill Spitzak <spitzak@gmail.com> wrote: > On 04/12/2016 01:55 PM, Søren Sandmann wrote: > > This series is available as a git repository here: >> >> https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master >> > > Not having much luck with that url: > > fatal: repository ' > https://cgit.freedesktop.org/~sandmann/pixman/log/?h=spitzak-for-master/' > not found > >