[Pixman] [PATCH v10 08/15] pixman-filter: Corrections to the integral() function

Oded Gabbay oded.gabbay at gmail.com
Thu Feb 4 02:11:33 PST 2016


On Tue, Feb 2, 2016 at 8:28 AM,  <spitzak at gmail.com> wrote:
> From: Bill Spitzak <spitzak at 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 fixed the nice filter and the integration to directly produce normalized
> values. Re-normalization is still needed for impulse.box or impulse.triangle
> so I did not remove it.
>
> Distribute fixed error over all filter samples, to remove a high-frequency
> bit of noise in the center of some filters (lancoz at large scale value).
>
> box.box, which I expect will be very common as it is the proposed "good" filter,
> was made a lot faster and more accurate. This is easy as the caller already
> intersected the two boxes, so the width is the integral.
>
> v7: This is a merge of 4 patches and lots of new code cleanup and fixes
>  determined by examining the gnuplot output
>
> v9: Restored the recursion splitting at zero for linear filter
>
> v10: Small change from here moved to previous Simpsons patch so it compiles
>
>      Merged patch to get correct subsample positions when subsample_bits==0
>
> Signed-off-by: Bill Spitzak <spitzak at gmail.com>
> ---
>  pixman/pixman-filter.c | 144 +++++++++++++++++++++++++------------------------
>  1 file changed, 74 insertions(+), 70 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index 13ee024..bd4e174 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -79,7 +79,7 @@ sinc (double x)
>  }
>
>  static double
> -lanczos (double x, int n)
> +lanczos (double x, double n)
>  {
>      return sinc (x) * sinc (x * (1.0 / n));
>  }
> @@ -99,7 +99,7 @@ lanczos3_kernel (double x)
>  static double
>  nice_kernel (double x)
>  {
> -    return lanczos3_kernel (x * 0.75);
> +    return lanczos3_kernel (x * 0.75) * 0.75;
>  }
>
>  static double
> @@ -147,45 +147,51 @@ static const filter_info_t filters[] =
>      { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,      8.0 },
>  };
>
> -/* This function scales the @sample filter by @size, then
> - * aligns @x1 in @reconstruct with @x2 in @sample and
> - * and integrates the product of the kernels across @width.
> +/* This function scales the @sample filter by @size, shifts it by @pos,
> + * and then integrates the product of the kernels across low..high
>   *
>   * This function assumes that the intervals are within
>   * the kernels in question. E.g., the caller must not
>   * try to integrate a linear kernel ouside of [-1:1]
>   */
>  static double
> -integral (pixman_kernel_t reconstruct, double x1,
> -         pixman_kernel_t sample, double size, double x2,
> -         double width)
> +integral (pixman_kernel_t reconstruct,
> +         pixman_kernel_t sample, double size, double pos,
> +         double low, double high)
>  {
> +    if (high < low)
> +    {
> +       return 0.0;
> +    }
> +    else if (sample == PIXMAN_KERNEL_IMPULSE)
> +    {
> +       return filters[reconstruct].func (-pos);
> +    }
> +    else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
> +    {
> +       return filters[sample].func (-pos / size) / size;
> +    }
> +    else if (reconstruct == PIXMAN_KERNEL_BOX && sample == PIXMAN_KERNEL_BOX)
> +    {
> +       assert (high <= low + 1.0);
> +       return (high - low) / size;
> +    }
>      /* If the integration interval crosses zero, break it into
>       * two separate integrals. This ensures that filters such
>       * as LINEAR that are not differentiable at 0 will still
>       * integrate properly.
>       */
> -    if (x1 < 0 && x1 + width > 0)
> +    else if (reconstruct == PIXMAN_KERNEL_LINEAR && low < 0 && high > 0)
>      {
>         return
> -           integral (reconstruct, x1, sample, size, x2, - x1) +
> -           integral (reconstruct, 0, sample, size, x2 - x1, width + x1);
> +           integral (reconstruct, sample, size, pos, low, 0) +
> +           integral (reconstruct, sample, size, pos, 0, high);
>      }
> -    else if (x2 < 0 && x2 + width > 0)
> +    else if (sample == PIXMAN_KERNEL_LINEAR && low < pos && high > pos)
>      {
>         return
> -           integral (reconstruct, x1, sample, size, x2, - x2) +
> -           integral (reconstruct, x1 - x2, sample, size, 0, width + x2);
> -    }
> -    else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
> -    {
> -       assert (width == 0.0);
> -       return filters[sample].func (x2 / size);
> -    }
> -    else if (sample == PIXMAN_KERNEL_IMPULSE)
> -    {
> -       assert (width == 0.0);
> -       return filters[reconstruct].func (x1);
> +           integral (reconstruct, sample, size, pos, low, pos) +
> +           integral (reconstruct, sample, size, pos, pos, high);
>      }
>      else
>      {
> @@ -197,32 +203,30 @@ integral (pixman_kernel_t reconstruct, double x1,
>          * filter is 6 wide.
>          */
>  #define N_SEGMENTS 12
> -#define SAMPLE(a1, a2)                                                 \
> -       (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / size))
> +#define SAMPLE(a)                                                      \
> +        (filters[reconstruct].func ((a)) * filters[sample].func (((a) - pos) / size))
>
>         double s = 0.0;
> -       double h = width / N_SEGMENTS;
> +       double h = (high - low) / N_SEGMENTS;
>         int i;
>
> -       s = SAMPLE (x1, x2);
> +       s = SAMPLE (low);
>
>         for (i = 1; i < N_SEGMENTS; i += 2)
>         {
> -           double a1 = x1 + h * i;
> -           double a2 = x2 + h * i;
> -           s += 4 * SAMPLE(a1, a2);
> +           double a1 = low + h * i;
> +           s += 4 * SAMPLE(a1);
>         }
>
>         for (i = 2; i < N_SEGMENTS; i += 2)
>         {
> -           double a1 = x1 + h * i;
> -           double a2 = x2 + h * i;
> -           s += 2 * SAMPLE(a1, a2);
> +           double a1 = low + h * i;
> +           s += 2 * SAMPLE(a1);
>         }
>
> -       s += SAMPLE (x1 + width, x2 + width);
> +       s += SAMPLE (high);
>
> -       return h * s * (1.0 / 3.0);
> +       return h * s * (1.0 / 3.0) / size;
>      }
>  }
>
> @@ -234,65 +238,65 @@ create_1d_filter (int              width,
>                   int              n_phases,
>                   pixman_fixed_t *p)
>  {
> -    double step;
> +    double step = 1.0 / n_phases;
> +    double rwidth2 = filters[reconstruct].width / 2.0;
> +    double swidth2 = size * filters[sample].width / 2.0;
>      int i;
>
> -    step = 1.0 / n_phases;
> -
>      for (i = 0; i < n_phases; ++i)
>      {
>          double frac = step / 2.0 + i * step;
>         pixman_fixed_t new_total;
> -        int x, x1, x2;
> -       double total;
> +        int x;
> +       double pos, total;
>
>         /* Sample convolution of reconstruction and sampling
>          * filter. See rounding.txt regarding the rounding
>          * and sample positions.
>          */
>
> -       x1 = ceil (frac - width / 2.0 - 0.5);
> -        x2 = x1 + width;
> +       if (n_phases & 1)
> +           pos = frac - width / 2.0;
> +       else
> +           pos = ceil (frac - width / 2.0 - 0.5) + 0.5 - frac;
>
>         total = 0;
> -        for (x = x1; x < x2; ++x)
> +        for (x = 0; x < width; ++x)
>          {
> -           double pos = x + 0.5 - frac;
> -           double rlow = - filters[reconstruct].width / 2.0;
> -           double rhigh = rlow + filters[reconstruct].width;
> -           double slow = pos - size * filters[sample].width / 2.0;
> -           double shigh = slow + size * filters[sample].width;
> -           double c = 0.0;
> -           double ilow, ihigh;
> -
> -           if (rhigh >= slow && rlow <= shigh)
> -           {
> -               ilow = MAX (slow, rlow);
> -               ihigh = MIN (shigh, rhigh);
> -
> -               c = integral (reconstruct, ilow,
> -                             sample, size, ilow - pos,
> -                             ihigh - ilow);
> -           }
> -
> +           double ilow = MAX (pos - swidth2, -rwidth2);
> +           double ihigh = MIN (pos + swidth2, rwidth2);
> +           double c = integral (reconstruct,
> +                                sample, size, pos,
> +                                ilow, ihigh);
>             total += c;
> -            *p++ = (pixman_fixed_t)(c * 65536.0 + 0.5);
> +            p[x] = (pixman_fixed_t)(c * 65536.0 + 0.5);
> +           pos++;
>          }
>
>         /* Normalize */
> -       p -= width;
>          total = 1 / total;
>          new_total = 0;
> -       for (x = x1; x < x2; ++x)
> +       for (x = 0; x < width; ++x)
>         {
> -           pixman_fixed_t t = (*p) * total + 0.5;
> -
> +           pixman_fixed_t t = p[x] * total + 0.5;
>             new_total += t;
> -           *p++ = t;
> +           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;
> +           }
> +       }
> +
> +       p += width;
>      }
>  }
>
> @@ -337,7 +341,7 @@ gnuplot_filter(int width, int samples, const pixman_fixed_t* p)
>  }
>  #endif
>
> -/* Besides calculating the width, this can modify the scale and subsample_bits */
> +/* Besides calculating the width, this can modify the size and subsample_bits */
>  static int
>  filter_width(pixman_kernel_t reconstruct, pixman_kernel_t sample,
>              double* size, int* subsample_bits)
> --
> 1.9.1
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Just one remark about consistency in indentation.
e.g. (from your patch above):
> +        int x;
> +       double pos, total;

The first line (int x) is padded with spaces, while the second line is
padded with a tab.
>From going over pixman's code, I see that the convention is for the
first indentation to be with 4 spaces, the second with a tab, the
third with a tab and 4 spaces and so on.

Please go over this patch and correct this.

With that fixed, this patch is:
Acked-by: Oded Gabbay <oded.gabbay at gmail.com>


More information about the Pixman mailing list