[Pixman] [PATCH v11 06/14] pixman-filter: reduce amount of malloc/free/memcpy to generate filter

[Oded Gabbay]

On Fri, Feb 5, 2016 at 9:49 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.
>
> v8: small refactor to remove the filter_width function
>
> v10: Restored filter_width function but with arguments changed to
>      match later patches
>
> v11: Removed unused arg and pointer from filter_width function
>      Whitespace fixes.
>
> Signed-off-by: Bill Spitzak <spitzak at gmail.com>
> ---
>  pixman/pixman-filter.c | 55 +++++++++++++++++++++-----------------------------
>  1 file changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index b70da1f..69ac3ab 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -217,23 +217,17 @@ integral (pixman_kernel_t reconstruct, double x1,
>      }
>  }
>
> -static pixman_fixed_t *
> -create_1d_filter (int             *width,
> +static void
> +create_1d_filter (int              width,
>                   pixman_kernel_t  reconstruct,
>                   pixman_kernel_t  sample,
>                   double           size,
> -                 int              n_phases)
> +                 int              n_phases,
> +                 pixman_fixed_t *p)
>  {
> -    pixman_fixed_t *params, *p;
>      double step;
>      int i;
>
> -    *width = ceil (size * filters[sample].width + filters[reconstruct].width);
> -
> -    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)
> @@ -248,8 +242,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)
> @@ -277,7 +271,7 @@ create_1d_filter (int             *width,
>          }
>
>         /* Normalize */
> -       p -= *width;
> +       p -= width;
>          total = 1 / total;
>          new_total = 0;
>         for (x = x1; x < x2; ++x)
> @@ -289,10 +283,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;
>  }
>
>  #ifdef PIXMAN_GNUPLOT
> @@ -336,6 +328,12 @@ gnuplot_filter(int width, int samples, const pixman_fixed_t* p)
>  }
>  #endif
>
> +static int
> +filter_width (pixman_kernel_t reconstruct, pixman_kernel_t sample, double size)
> +{
> +    return ceil (filters[reconstruct].width + size * filters[sample].width);
> +}
> +
>  /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
>   * with the given kernels and size parameters
>   */
> @@ -352,42 +350,35 @@ pixman_filter_create_separable_convolution (int             *n_values,
>  {
>      double sx = fabs (pixman_fixed_to_double (size_x));
>      double sy = fabs (pixman_fixed_to_double (size_y));
> -    pixman_fixed_t *horz = NULL, *vert = NULL, *params = NULL;
> +    pixman_fixed_t *params;
>      int subsample_x, subsample_y;
>      int width, height;
>
> +    width = filter_width (reconstruct_x, sample_x, sx);
>      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);
> +    height = filter_width (reconstruct_y, sample_y, sy);
> +    subsample_y = (1 << subsample_bits_y);
>
> -    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));
> +    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);
>
>  #ifdef PIXMAN_GNUPLOT
>      gnuplot_filter(width, subsample_x, params+4);
>  #endif
>
> -out:
> -    free (horz);
> -    free (vert);
> -
>      return params;
>  }
> --
> 1.9.1
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

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