[Pixman] [PATCHv2 0/3] More general reduction of BILINEAR to NEAREST
This series addresses the comments by Bill and also changes pixman-fast-path.c so that it picks NEAREST fast paths before BILINEAR. (I noticed this because the new filter-reduction-test.c failed to detect a bug that I deliberately introduced). Søren ___ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCH v14 16/22] pixman-filter: distribute normalization error over filter
On Fri, Mar 18, 2016 at 9:54 PM, Søren Sandmannwrote: > On Sun, Mar 6, 2016 at 8:06 PM, wrote: > >> From: Bill Spitzak >> >> 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 >> --- >> 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
Re: [Pixman] [PATCH v14 16/22] pixman-filter: distribute normalization error over filter
On Sun, Mar 6, 2016 at 8:06 PM,wrote: > From: Bill Spitzak > > 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 > --- > 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. Søren ___ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman
Re: [Pixman] [PATCHv2 2/3] Add new test of filter reduction from BILINEAR to NEAREST
Looks like a good idea to me. It is unfortunate that if it fails there is not much indication which image failed but that would require a lot more checksums. I would guess the checksums are computed by running this with the fast paths disabled? ___ Pixman mailing list Pixman@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/pixman