[Pixman] [PATCH 05/13] pixman-filter: reduce amount of malloc/free/memcpy to generate filter

[Oded Gabbay]

On Mon, Jan 4, 2016 at 5:12 AM,  <spitzak at gmail.com> wrote:
> From: Bill Spitzak <spitzak at gmail.com>
>
> Rearranged so that the entire block of memory for the filter pair
> is allocated first, and then filled in. Previous version allocated
> and freed two temporary buffers for each filter and did an extra
> memcpy.
> ---
>  pixman/pixman-filter.c | 59 +++++++++++++++++++++-----------------------------
>  1 file changed, 25 insertions(+), 34 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index 05bc345..15f9069 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -217,25 +217,25 @@ integral (pixman_kernel_t reconstruct, double x1,
>      }
>  }
>
> -static pixman_fixed_t *
> -create_1d_filter (int             *width,
> +static int
> +filter_width (pixman_kernel_t reconstruct,
> +             pixman_kernel_t sample,
> +             double scale)
> +{
> +    return ceil (scale * filters[sample].width + filters[reconstruct].width);
> +}
> +
> +static void
> +create_1d_filter (int              width,
>                   pixman_kernel_t  reconstruct,
>                   pixman_kernel_t  sample,
>                   double           scale,
> -                 int              n_phases)
> +                 int              n_phases,
> +                 pixman_fixed_t *p)
>  {
> -    pixman_fixed_t *params, *p;
>      double step;
> -    double size;
>      int i;
>
> -    size = scale * filters[sample].width + filters[reconstruct].width;
> -    *width = ceil (size);
> -
> -    p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
> -    if (!params)
> -        return NULL;
> -
>      step = 1.0 / n_phases;
>
>      for (i = 0; i < n_phases; ++i)
> @@ -250,8 +250,8 @@ create_1d_filter (int             *width,
>          * and sample positions.
>          */
>
> -       x1 = ceil (frac - *width / 2.0 - 0.5);
> -        x2 = x1 + *width;
> +       x1 = ceil (frac - width / 2.0 - 0.5);
> +        x2 = x1 + width;
>
>         total = 0;
>          for (x = x1; x < x2; ++x)
> @@ -279,7 +279,7 @@ create_1d_filter (int             *width,
>          }
>
>         /* Normalize */
> -       p -= *width;
> +       p -= width;
>          total = 1 / total;
>          new_total = 0;
>         for (x = x1; x < x2; ++x)
> @@ -291,10 +291,8 @@ create_1d_filter (int             *width,
>         }
>
>         if (new_total != pixman_fixed_1)
> -           *(p - *width / 2) += (pixman_fixed_1 - new_total);
> +           *(p - width / 2) += (pixman_fixed_1 - new_total);
>      }
> -
> -    return params;
>  }
>
>  /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
> @@ -313,38 +311,31 @@ pixman_filter_create_separable_convolution (int             *n_values,
>  {
>      double sx = fabs (pixman_fixed_to_double (scale_x));
>      double sy = fabs (pixman_fixed_to_double (scale_y));
> -    pixman_fixed_t *horz = NULL, *vert = NULL, *params = NULL;
> +    pixman_fixed_t *params;
>      int subsample_x, subsample_y;
>      int width, height;
>
>      subsample_x = (1 << subsample_bits_x);
>      subsample_y = (1 << subsample_bits_y);
>
> -    horz = create_1d_filter (&width, reconstruct_x, sample_x, sx, subsample_x);
> -    vert = create_1d_filter (&height, reconstruct_y, sample_y, sy, subsample_y);
> +    width = filter_width (reconstruct_x, sample_x, sx);
> +    height = filter_width (reconstruct_y, sample_y, sy);
>
> -    if (!horz || !vert)
> -        goto out;
> -
>      *n_values = 4 + width * subsample_x + height * subsample_y;
> -
> +
>      params = malloc (*n_values * sizeof (pixman_fixed_t));
>      if (!params)
> -        goto out;
> +        return NULL;
>
>      params[0] = pixman_int_to_fixed (width);
>      params[1] = pixman_int_to_fixed (height);
>      params[2] = pixman_int_to_fixed (subsample_bits_x);
>      params[3] = pixman_int_to_fixed (subsample_bits_y);
>
> -    memcpy (params + 4, horz,
> -           width * subsample_x * sizeof (pixman_fixed_t));
> -    memcpy (params + 4 + width * subsample_x, vert,
> -           height * subsample_y * sizeof (pixman_fixed_t));
> -
> -out:
> -    free (horz);
> -    free (vert);
> +    create_1d_filter (width, reconstruct_x, sample_x, sx, subsample_x,
> +                     params + 4);
> +    create_1d_filter (height, reconstruct_y, sample_y, sy, subsample_y,
> +                     params + 4 + width * subsample_x);
>
>      return params;
>  }
> --
> 1.9.1
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Same remark as for patches 2&4.
This patch is:

Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com>