Re: [Pixman] [PATCH 03/13] pixman-image: Added GNUPLOT_OUTPUT compile-time option to view filters in gnuplot

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 7:34 PM, Bill Spitzak  wrote:
>
>
> On Mon, Jan 4, 2016 at 3:25 AM, Oded Gabbay  wrote:
>>
>> On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
>> > From: Bill Spitzak 
>> >
>> > If GNUPLOT_OUTPUT is set, then you can pipe the output of a pixman-using
>> > program
>> > to gnuplot and get a continuously-updated plot of the horizontal filter.
>> > This
>> > works well with demos/scale to test the filter generation.
>> >
>> > The plot is all the different subposition filters shuffled together.
>> > This is
>> > misleading in a few cases:
>> >
>> >   IMPULSE.BOX - goes up and down as the subfilters have different
>> > numbers of non-zero samples
>> >   IMPULSE.TRIANGLE - somewhat crooked for the same reason
>> >   1-wide filters - looks triangular, but a 1-wide box would be more
>> > accurate
>> > ---
>> >  pixman/pixman-image.c | 40 
>> >  1 file changed, 40 insertions(+)
>> >
>> > diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
>> > index 1ff1a49..69743c4 100644
>> > --- a/pixman/pixman-image.c
>> > +++ b/pixman/pixman-image.c
>> > @@ -531,6 +531,46 @@ compute_image_info (pixman_image_t *image)
>> >
>> >  image->common.flags = flags;
>> >  image->common.extended_format_code = code;
>> > +/* If GNUPLOT_OUTPUT is set, then you can pipe the output of a
>> > pixman-using program
>> > + * to gnuplot and get a continuously-updated plot of the horizontal
>> > filter. This
>> > + * works well with demos/scale to test the filter generation.
>> > + *
>> > + * The plot is all the different subposition filters shuffled together.
>> > This is
>> > + * misleading in a few cases:
>> > + *
>> > + *  IMPULSE.BOX - goes up and down as the subfilters have different
>> > numbers of non-zero samples
>> > + *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
>> > + *  1-wide filters - looks triangular, but a 1-wide box would be more
>> > accurate
>> > + */
>> > +/* #define GNUPLOT_OUTPUT 1 */
>> > +#if GNUPLOT_OUTPUT
>> > +if ((flags & FAST_PATH_SEPARABLE_CONVOLUTION_FILTER) &&
>> > image->common.filter_params) {
>> > +   const pixman_fixed_t* p = image->common.filter_params;
>> > +   int width = pixman_fixed_to_int(p[0]);
>> > +   int samples = 1 << pixman_fixed_to_int(p[2]);
>> > +   int x,y;
>> > +   p += 4;
>> > +   printf("plot '-' with linespoints\n");
>> > +   printf("%g 0\n", - width * .5);
>> > +   for (x = 0; x < width; ++x) {
>> > +   for (y = 0; y < samples; ++y) {
>> > +   int yy;
>> > +   if (width & 1)
>> > +   yy = y;
>> > +   else if (y >= samples / 2)
>> > +   yy = y - samples / 2;
>> > +   else
>> > +   yy = samples / 2 + y;
>> > +   printf("%g %g\n",
>> > +  x - width * .5 + (y + .5) * (1.0 / samples),
>> > +  pixman_fixed_to_double(p[(yy + 1) * width - x -
>> > 1]));
>> > +   }
>> > +   }
>> > +   printf("%g 0\n", width * .5);
>> > +   printf("e\n");
>> > +   fflush(stdout);
>> > +}
>> > +#endif
>> >  }
>> >
>> >  void
>> > --
>> > 1.9.1
>> >
>> > ___
>> > Pixman mailing list
>> > Pixman@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/pixman
>>
>> I get the big picture of this feature, and it seems useful, but we
>> need write this according to standards.
>> I would like to see the following changes:
>>
>> 1. We can do something more nice than a hard-coded #define. Let's add
>> a flag to configure.ac called "enable-gnuplot-output". For something
>> similar, see "enable-timers" in configure.ac. The default will be
>> "no".
>
>
> That makes sense. I actually gave up trying to figure out how to set the
> CFLAGS and was forced to edit the source code to turn this on/off, which
> meant I often mistakenly checked the code in with it turned on. I guess
> configure is the way to go.
>
>>
>> 2. Take your code and put it in a new function in pixman-utils.c, and
>> make sure the code inside the function is encompassed with
>> #ifdef/#endif around it. In case enable-gluplot-output wasn't called,
>> the function will be an empty function.
>>
>> 3. Call that function from compute_image_info. I guess you will want
>> to call it only if:
>> ((flags & FAST_PATH_SEPARABLE_CONVOLUTION_FILTER) &&
>> image->common.filter_params)
>> is true.
>> However, we don't need to put #ifdef around the call itself.
>
>
> I would prefer that there not be a call to the empty function there, and
> that instead the config option removes the call to it here. In addition the
> arguments may be non-trivial as I would like this to be able to print
> filters from a future mulit-resolution data structure, to extract this from
> the current structure I need to pass the width, subsamples, and pointer to
> array.

Re: [Pixman] [PATCH 03/13] pixman-image: Added GNUPLOT_OUTPUT compile-time option to view filters in gnuplot

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> From: Bill Spitzak 
>
> If GNUPLOT_OUTPUT is set, then you can pipe the output of a pixman-using 
> program
> to gnuplot and get a continuously-updated plot of the horizontal filter. This
> works well with demos/scale to test the filter generation.
>
> The plot is all the different subposition filters shuffled together. This is
> misleading in a few cases:
>
>   IMPULSE.BOX - goes up and down as the subfilters have different numbers of 
> non-zero samples
>   IMPULSE.TRIANGLE - somewhat crooked for the same reason
>   1-wide filters - looks triangular, but a 1-wide box would be more accurate
> ---
>  pixman/pixman-image.c | 40 
>  1 file changed, 40 insertions(+)
>
> diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
> index 1ff1a49..69743c4 100644
> --- a/pixman/pixman-image.c
> +++ b/pixman/pixman-image.c
> @@ -531,6 +531,46 @@ compute_image_info (pixman_image_t *image)
>
>  image->common.flags = flags;
>  image->common.extended_format_code = code;
> +/* If GNUPLOT_OUTPUT is set, then you can pipe the output of a pixman-using 
> program
> + * to gnuplot and get a continuously-updated plot of the horizontal filter. 
> This
> + * works well with demos/scale to test the filter generation.
> + *
> + * The plot is all the different subposition filters shuffled together. This 
> is
> + * misleading in a few cases:
> + *
> + *  IMPULSE.BOX - goes up and down as the subfilters have different numbers 
> of non-zero samples
> + *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
> + *  1-wide filters - looks triangular, but a 1-wide box would be more 
> accurate
> + */
> +/* #define GNUPLOT_OUTPUT 1 */
> +#if GNUPLOT_OUTPUT
> +if ((flags & FAST_PATH_SEPARABLE_CONVOLUTION_FILTER) && 
> image->common.filter_params) {
> +   const pixman_fixed_t* p = image->common.filter_params;
> +   int width = pixman_fixed_to_int(p[0]);
> +   int samples = 1 << pixman_fixed_to_int(p[2]);
> +   int x,y;
> +   p += 4;
> +   printf("plot '-' with linespoints\n");
> +   printf("%g 0\n", - width * .5);
> +   for (x = 0; x < width; ++x) {
> +   for (y = 0; y < samples; ++y) {
> +   int yy;
> +   if (width & 1)
> +   yy = y;
> +   else if (y >= samples / 2)
> +   yy = y - samples / 2;
> +   else
> +   yy = samples / 2 + y;
> +   printf("%g %g\n",
> +  x - width * .5 + (y + .5) * (1.0 / samples),
> +  pixman_fixed_to_double(p[(yy + 1) * width - x - 1]));
> +   }
> +   }
> +   printf("%g 0\n", width * .5);
> +   printf("e\n");
> +   fflush(stdout);
> +}
> +#endif
>  }
>
>  void
> --
> 1.9.1
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

I get the big picture of this feature, and it seems useful, but we
need write this according to standards.
I would like to see the following changes:

1. We can do something more nice than a hard-coded #define. Let's add
a flag to configure.ac called "enable-gnuplot-output". For something
similar, see "enable-timers" in configure.ac. The default will be
"no".

2. Take your code and put it in a new function in pixman-utils.c, and
make sure the code inside the function is encompassed with
#ifdef/#endif around it. In case enable-gluplot-output wasn't called,
the function will be an empty function.

3. Call that function from compute_image_info. I guess you will want
to call it only if:
((flags & FAST_PATH_SEPARABLE_CONVOLUTION_FILTER) &&
image->common.filter_params)
is true.
However, we don't need to put #ifdef around the call itself.

I think the above steps will create two patches:
- Step 1&2 should be the first patch (Adding the utility).
- Step 3 should be the second patch (Using the utility).

Is using this is simple as:
./ | gnuplot ?

or is it more complicated than that ?
If so, put an example of how to use it in the second commit message.

Thanks,

Oded
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 01/13] demos/scale: Compute filter size using boundary of xformed ellipse, not rectangle

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> From: Bill Spitzak 
>
> This is much more accurate and less blurry. In particular the filtering does
> not change as the image is rotated.

You forgot to sign-off this patch.

> ---
>  demos/scale.c | 102 
> +++---
>  1 file changed, 61 insertions(+), 41 deletions(-)
>
> diff --git a/demos/scale.c b/demos/scale.c
> index d00307e..0995ad0 100644
> --- a/demos/scale.c
> +++ b/demos/scale.c
> @@ -55,50 +55,70 @@ get_widget (app_t *app, const char *name)
>  return widget;
>  }
>
> -static double
> -min4 (double a, double b, double c, double d)
> -{
> -double m1, m2;
> -
> -m1 = MIN (a, b);
> -m2 = MIN (c, d);
> -return MIN (m1, m2);
> -}
> -
> -static double
> -max4 (double a, double b, double c, double d)
> -{
> -double m1, m2;
> -
> -m1 = MAX (a, b);
> -m2 = MAX (c, d);
> -return MAX (m1, m2);
> -}
> -
> +/* Figure out the boundary of a diameter=1 circle transformed into an ellipse
> + * by trans. Proof that this is the correct calculation:
> + *
> + * Transform x,y to u,v by this matrix calculation:
> + *
> + *  |u|   |a c| |x|
> + *  |v| = |b d|*|y|
> + *
> + * Horizontal component:
> + *
> + *  u = ax+cy (1)
> + *
> + * For each x,y on a radius-1 circle (p is angle to the point):
> + *
> + *  x^2+y^2 = 1
> + *  x = cos(p)
> + *  y = sin(p)
> + *  dx/dp = -sin(p) = -y
> + *  dy/dp = cos(p) = x
> + *
> + * Figure out derivative of (1) relative to p:
> + *
> + *  du/dp = a(dx/dp) + c(dy/dp)
> + *= -ay + cx
> + *
> + * The min and max u are when du/dp is zero:
> + *
> + *  -ay + cx = 0
> + *  cx = ay
> + *  c = ay/x  (2)
> + *  y = cx/a  (3)
> + *
> + * Substitute (2) into (1) and simplify:
> + *
> + *  u = ax + ay^2/x
> + *= a(x^2+y^2)/x
> + *= a/x (because x^2+y^2 = 1)
> + *  x = a/u (4)
> + *
> + * Substitute (4) into (3) and simplify:
> + *
> + *  y = c(a/u)/a
> + *  y = c/u (5)
> + *
> + * Square (4) and (5) and add:
> + *
> + *  x^2+y^2 = (a^2+c^2)/u^2
> + *
> + * But x^2+y^2 is 1:
> + *
> + *  1 = (a^2+c^2)/u^2
> + *  u^2 = a^2+c^2
> + *  u = hypot(a,c)
> + *
> + * Similarily the max/min of v is at:
> + *
> + *  v = hypot(b,d)
> + *
> + */
>  static void
>  compute_extents (pixman_f_transform_t *trans, double *sx, double *sy)
>  {
> -double min_x, max_x, min_y, max_y;
> -pixman_f_vector_t v[4] =
> -{
> -   { { 1, 1, 1 } },
> -   { { -1, 1, 1 } },
> -   { { -1, -1, 1 } },
> -   { { 1, -1, 1 } },
> -};
> -
> -pixman_f_transform_point (trans, [0]);
> -pixman_f_transform_point (trans, [1]);
> -pixman_f_transform_point (trans, [2]);
> -pixman_f_transform_point (trans, [3]);
> -
> -min_x = min4 (v[0].v[0], v[1].v[0], v[2].v[0], v[3].v[0]);
> -max_x = max4 (v[0].v[0], v[1].v[0], v[2].v[0], v[3].v[0]);
> -min_y = min4 (v[0].v[1], v[1].v[1], v[2].v[1], v[3].v[1]);
> -max_y = max4 (v[0].v[1], v[1].v[1], v[2].v[1], v[3].v[1]);
> -
> -*sx = (max_x - min_x) / 2.0;
> -*sy = (max_y - min_y) / 2.0;
> +*sx = hypot (trans->m[0][0], trans->m[0][1]) / trans->m[2][2];
> +*sy = hypot (trans->m[1][0], trans->m[1][1]) / trans->m[2][2];
>  }
>
>  typedef struct
> --
> 1.9.1
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Add the sign-off and my:

Reviewed-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 04/13] pixman-filter: Consistency in arg names to integral ()

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 11:25 AM, Oded Gabbay  wrote:
> On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
>> From: Bill Spitzak 
>>
>> Rename kernel1/2 to reconstruct/sample and use 1/scale as the
>> scale argument, thus matching the names in other functions.
>> ---
>>  pixman/pixman-filter.c | 28 ++--
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
>> index b2bf53f..05bc345 100644
>> --- a/pixman/pixman-filter.c
>> +++ b/pixman/pixman-filter.c
>> @@ -147,8 +147,8 @@ static const filter_info_t filters[] =
>>  { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
>>  };
>>
>> -/* This function scales @kernel2 by @scale, then
>> - * aligns @x1 in @kernel1 with @x2 in @kernel2 and
>> +/* This function scales @sample by @scale, then
>> + * aligns @x1 in @reconstruct with @x2 in @sample and
>>   * and integrates the product of the kernels across @width.
>>   *
>>   * This function assumes that the intervals are within
>> @@ -156,8 +156,8 @@ static const filter_info_t filters[] =
>>   * try to integrate a linear kernel ouside of [-1:1]
>>   */
>>  static double
>> -integral (pixman_kernel_t kernel1, double x1,
>> - pixman_kernel_t kernel2, double scale, double x2,
>> +integral (pixman_kernel_t reconstruct, double x1,
>> + pixman_kernel_t sample, double scale, double x2,
>>   double width)
>>  {
>>  /* If the integration interval crosses zero, break it into
>> @@ -168,31 +168,31 @@ integral (pixman_kernel_t kernel1, double x1,
>>  if (x1 < 0 && x1 + width > 0)
>>  {
>> return
>> -   integral (kernel1, x1, kernel2, scale, x2, - x1) +
>> -   integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
>> +   integral (reconstruct, x1, sample, scale, x2, - x1) +
>> +   integral (reconstruct, 0, sample, scale, x2 - x1, width + x1);
>>  }
>>  else if (x2 < 0 && x2 + width > 0)
>>  {
>> return
>> -   integral (kernel1, x1, kernel2, scale, x2, - x2) +
>> -   integral (kernel1, x1 - x2, kernel2, scale, 0, width + x2);
>> +   integral (reconstruct, x1, sample, scale, x2, - x2) +
>> +   integral (reconstruct, x1 - x2, sample, scale, 0, width + x2);
>>  }
>> -else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
>> +else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
>>  {
>> assert (width == 0.0);
>> -   return filters[kernel2].func (x2 * scale);
>> +   return filters[sample].func (x2 / scale);
>>  }
>> -else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
>> +else if (sample == PIXMAN_KERNEL_IMPULSE)
>>  {
>> assert (width == 0.0);
>> -   return filters[kernel1].func (x1);
>> +   return filters[reconstruct].func (x1);
>>  }
>>  else
>>  {
>> /* Integration via Simpson's rule */
>>  #define N_SEGMENTS 128
>>  #define SAMPLE(a1, a2) \
>> -   (filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))
>> +   (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / 
>> scale))
>>
>> double s = 0.0;
>> double h = width / (double)N_SEGMENTS;
>> @@ -270,7 +270,7 @@ create_1d_filter (int *width,
>> ihigh = MIN (shigh, rhigh);
>>
>> c = integral (reconstruct, ilow,
>> - sample, 1.0 / scale, ilow - pos,
>> + sample, scale, ilow - pos,
>>   ihigh - ilow);
>> }
>>
>> --
>> 1.9.1
>>
>> ___
>> Pixman mailing list
>> Pixman@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/pixman
>
> Same remarks as for patch 2 and this patch is:

Sorry, hit the enter key by mistake.
This patch is:

Reviewed-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 06/13] pixman-filter: Correct Simpsons integration

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> From: Bill Spitzak 
>
> Simpsons uses cubic curve fitting, with 3 samples defining each cubic. This
> makes the weights of the samples be in a pattern of 1,4,2,4,2...4,1, and then
> dividing the result by 3.
>
> The previous code was using weights of 1,2,6,6...6,2,1. Since it divided by
> 3 this produced about 2x the desired value (the normalization fixed this).
> Also this is effectively a linear interpolation, not Simpsons integration.
>
> With this fix the integration is accurate enough that the number of samples
> could be reduced a lot. Likely even 16 samples is too many.

You forgot to sign-off

> ---
>  pixman/pixman-filter.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index 15f9069..5677431 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -189,8 +189,10 @@ integral (pixman_kernel_t reconstruct, double x1,
>  }
>  else
>  {
> -   /* Integration via Simpson's rule */
> -#define N_SEGMENTS 128
> +   /* Integration via Simpson's rule
> +* See http://www.intmath.com/integration/6-simpsons-rule.php
> +*/
> +#define N_SEGMENTS 16
>  #define SAMPLE(a1, a2) \
> (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / 
> scale))
>
> @@ -204,11 +206,14 @@ integral (pixman_kernel_t reconstruct, double x1,
> {
> double a1 = x1 + h * i;
> double a2 = x2 + h * i;
> +   s += 4 * SAMPLE(a1, a2);
> +   }
>
> -   s += 2 * SAMPLE (a1, a2);
> -
> -   if (i >= 2 && i < N_SEGMENTS - 1)
> -   s += 4 * SAMPLE (a1, a2);
> +   for (i = 2; i < N_SEGMENTS; i += 2)
> +   {
> +   double a1 = x1 + h * i;
> +   double a2 = x2 + h * i;
> +   s += 2 * SAMPLE(a1, a2);
> }
>
> s += SAMPLE (x1 + width, x2 + width);
> --
> 1.9.1
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Patch is:

Reviewed-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH pixman] V7 implement PIXMAN_FILTER_GOOD/BEST

2016-01-04 Thread Oded Gabbay 
Hi Bill,
Thanks for v7.
For future reference, when you resend a patch or a patch series, I
recommend that you follow the following guidelines (taken from the
Linux kernel community):

1. When creating the patches with git format-patch, add the following
flag: "--subject-prefix="PATCH vX". That way, the title of each patch
will contain the version information so it can be easily recognized to
which version it belongs.

2. Highlighting the major changes in the cover letter is very good.
However, you are supposed to write *every* change (maybe except
trivial spelling issues) in each of the patches commit messages, so
the person(s) reviewing your revised patches will know exactly what
was changed in each patch, instead of trying to guess from the cover
letter which patch has changed and which not. If the patch hasn't
changed at all, you can omit this part.

e.g. a sample commit message (six versions):


amdkfd: Add amdkfd skeleton driver

This patch adds the amdkfd skeleton driver. The driver does nothing except
define a /dev/kfd device.

It returns -ENODEV on all amdkfd IOCTLs.

v3:

- Move bool field to the end of structure
- Removed the pmc ioctls
- Added a meaningful error message for ioctl error.

v5:

- Create a new folder drm/amd and move amdkfd from drm/radeon/ to drm/amd/
- Remove scheduler_class from kfd_priv.h as it was never used
- Add skeleton implementation of the Get Version IOCTL

v6:

- Update module version to the correct number
- Remove the "default m" from the Kconfig file

Signed-off-by: Oded Gabbay 



If you want/have the time to resend v7 with the guidelines I wrote, it
would be highly appreciated, but its not mandatory.

Thanks,

Oded

On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> Differences from previous version:
>
> - Added compile-time option to pipe filters to gnuplot for analysis.
>
> - Fixed integral when one of the filters is IMPULSE, previous version was
> offsetting the sample from the center.
>
> - Distribute fixed-point error on all samples, rather than putting it all
> on the center.
>
> - If reconstruct=impulse and scale < 1 (or sample=impulse) then it was
> producing garbage by attempting to renormalize filters that should not
> sum to 1. Even the "correct" result is useless, so I just made this set
> the scale to 1.
>
> - Comments and formatting fixes.
>
> I consider all patches except the last one to be bug fixes. The last
> patch changes the output for GOOD or BEST filter settings. Output is
> nearly identical towhat Cairo is producing now, but has some minor
> tweaks to cutoffs between filters and handling of very small scales.
> This also detects more cases where nearest or bilinear can be used as
> a fast path.
>
> With all of these applied, the filter generation can be removed from cairo as
> pixman will do it. The fallback to image for the X11 backend could be removed
> if it is known the X11 server is using this new version.
>
> This is also a necessary step in order to introduce 2-pass image filtering
> to pixman. Currently software such as Cairo cannot use the filtering in
> pixman and this means it will bypass any such improvements.
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

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

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> From: Bill Spitzak 
>
> 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 (, reconstruct_x, sample_x, sx, 
> subsample_x);
> -vert = create_1d_filter (, 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

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

Reviewed-by: Oded Gabbay 
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 04/13] pixman-filter: Consistency in arg names to integral ()

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> From: Bill Spitzak 
>
> Rename kernel1/2 to reconstruct/sample and use 1/scale as the
> scale argument, thus matching the names in other functions.
> ---
>  pixman/pixman-filter.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
> index b2bf53f..05bc345 100644
> --- a/pixman/pixman-filter.c
> +++ b/pixman/pixman-filter.c
> @@ -147,8 +147,8 @@ static const filter_info_t filters[] =
>  { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
>  };
>
> -/* This function scales @kernel2 by @scale, then
> - * aligns @x1 in @kernel1 with @x2 in @kernel2 and
> +/* This function scales @sample by @scale, then
> + * aligns @x1 in @reconstruct with @x2 in @sample and
>   * and integrates the product of the kernels across @width.
>   *
>   * This function assumes that the intervals are within
> @@ -156,8 +156,8 @@ static const filter_info_t filters[] =
>   * try to integrate a linear kernel ouside of [-1:1]
>   */
>  static double
> -integral (pixman_kernel_t kernel1, double x1,
> - pixman_kernel_t kernel2, double scale, double x2,
> +integral (pixman_kernel_t reconstruct, double x1,
> + pixman_kernel_t sample, double scale, double x2,
>   double width)
>  {
>  /* If the integration interval crosses zero, break it into
> @@ -168,31 +168,31 @@ integral (pixman_kernel_t kernel1, double x1,
>  if (x1 < 0 && x1 + width > 0)
>  {
> return
> -   integral (kernel1, x1, kernel2, scale, x2, - x1) +
> -   integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
> +   integral (reconstruct, x1, sample, scale, x2, - x1) +
> +   integral (reconstruct, 0, sample, scale, x2 - x1, width + x1);
>  }
>  else if (x2 < 0 && x2 + width > 0)
>  {
> return
> -   integral (kernel1, x1, kernel2, scale, x2, - x2) +
> -   integral (kernel1, x1 - x2, kernel2, scale, 0, width + x2);
> +   integral (reconstruct, x1, sample, scale, x2, - x2) +
> +   integral (reconstruct, x1 - x2, sample, scale, 0, width + x2);
>  }
> -else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
> +else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
>  {
> assert (width == 0.0);
> -   return filters[kernel2].func (x2 * scale);
> +   return filters[sample].func (x2 / scale);
>  }
> -else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
> +else if (sample == PIXMAN_KERNEL_IMPULSE)
>  {
> assert (width == 0.0);
> -   return filters[kernel1].func (x1);
> +   return filters[reconstruct].func (x1);
>  }
>  else
>  {
> /* Integration via Simpson's rule */
>  #define N_SEGMENTS 128
>  #define SAMPLE(a1, a2) \
> -   (filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))
> +   (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / 
> scale))
>
> double s = 0.0;
> double h = width / (double)N_SEGMENTS;
> @@ -270,7 +270,7 @@ create_1d_filter (int *width,
> ihigh = MIN (shigh, rhigh);
>
> c = integral (reconstruct, ilow,
> - sample, 1.0 / scale, ilow - pos,
> + sample, scale, ilow - pos,
>   ihigh - ilow);
> }
>
> --
> 1.9.1
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/pixman

Same remarks as for patch 2 and this patch is:
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 03/14] pixman-image: Added enable-gnuplot config to view filters in gnuplot

2016-01-04 Thread spitzak 
From: Bill Spitzak 

If enable-gnuplot is configured, then you can pipe the output of a pixman-using 
program
to gnuplot and get a continuously-updated plot of the horizontal filter. This
works well with demos/scale to test the filter generation.

The plot is all the different subposition filters shuffled together. This is
misleading in a few cases:

  IMPULSE.BOX - goes up and down as the subfilters have different numbers of 
non-zero samples
  IMPULSE.TRIANGLE - somewhat crooked for the same reason
  1-wide filters - looks triangular, but a 1-wide box would be more accurate

v8: Use config option
Moved code to the filter generator
Modified scale demo to not call filter generator a second time.

v7: First time this ability was included

Signed-off-by: Bill Spitzak 
---
 configure.ac   | 13 +
 demos/scale.c  | 29 +++--
 pixman/pixman-filter.c | 45 +
 3 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6b2134e..8066cb2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -834,6 +834,19 @@ fi
 AC_SUBST(PIXMAN_TIMERS)
 
 dnl ===
+dnl gnuplot
+
+AC_ARG_ENABLE(gnuplot,
+   [AC_HELP_STRING([--enable-gnuplot],
+   [enable output of filters that can be piped to gnuplot 
[default=no]])],
+   [enable_gnuplot=$enableval], [enable_gnuplot=no])
+
+if test $enable_gnuplot = yes ; then 
+   AC_DEFINE(PIXMAN_GNUPLOT, 1, [enable output that can be piped to gnuplot])
+fi
+AC_SUBST(PIXMAN_GNUPLOT)
+
+dnl ===
 dnl GTK+
 
 AC_ARG_ENABLE(gtk,
diff --git a/demos/scale.c b/demos/scale.c
index 06821e3..881004e 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -258,16 +258,25 @@ rescale (GtkWidget *may_be_null, app_t *app)
 pixman_transform_from_pixman_f_transform (, );
 pixman_image_set_transform (app->original, );
 
-params = pixman_filter_create_separable_convolution (
-_params,
-sx * 65536.0 + 0.5,
-   sy * 65536.0 + 0.5,
-   get_value (app, filters, "reconstruct_x_combo_box"),
-   get_value (app, filters, "reconstruct_y_combo_box"),
-   get_value (app, filters, "sample_x_combo_box"),
-   get_value (app, filters, "sample_y_combo_box"),
-   gtk_adjustment_get_value (app->subsample_adjustment),
-   gtk_adjustment_get_value (app->subsample_adjustment));
+if (get_value (app, filter_types, "filter_combo_box") ==
+   PIXMAN_FILTER_SEPARABLE_CONVOLUTION)
+{
+   params = pixman_filter_create_separable_convolution (
+_params,
+   sx * 65536.0 + 0.5,
+   sy * 65536.0 + 0.5,
+   get_value (app, filters, "reconstruct_x_combo_box"),
+   get_value (app, filters, "reconstruct_y_combo_box"),
+   get_value (app, filters, "sample_x_combo_box"),
+   get_value (app, filters, "sample_y_combo_box"),
+   gtk_adjustment_get_value (app->subsample_adjustment),
+   gtk_adjustment_get_value (app->subsample_adjustment));
+}
+else
+{
+   params = 0;
+   n_params = 0;
+}
 
 pixman_image_set_filter (app->original,
get_value (app, filter_types, "filter_combo_box"),
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b2bf53f..fe0d261 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -297,6 +297,47 @@ create_1d_filter (int *width,
 return params;
 }
 
+#if PIXMAN_GNUPLOT
+/* If enable-gnuplot is configured, then you can pipe the output of a
+ * pixman-using program to gnuplot and get a continuously-updated plot
+ * of the horizontal filter. This works well with demos/scale to test
+ * the filter generation.
+ *
+ * The plot is all the different subposition filters shuffled
+ * together. This is misleading in a few cases:
+ *
+ *  IMPULSE.BOX - goes up and down as the subfilters have different
+ *numbers of non-zero samples
+ *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
+ *  1-wide filters - looks triangular, but a 1-wide box would be more
+ *   accurate
+ */
+static void
+gnuplot_filter(int width, int samples, const pixman_fixed_t* p)
+{
+int x,y;
+printf("plot '-' with linespoints\n");
+printf("%g 0\n", - width * .5);
+for (x = 0; x < width; ++x) {
+   for (y = 0; y < samples; ++y) {
+   int yy;
+   if (width & 1)
+   yy = y;
+   else if (y >= samples / 2)
+   yy = y - samples / 2;
+   else
+   yy = samples / 2 + y;
+   printf("%g %g\n",
+  x - width * 0.5 + (y + 0.5) * (1.0 / samples),
+  pixman_fixed_to_double(p[(yy + 1) * width - x - 1]));
+   }
+}
+printf("%g 0\n", width * .5);
+printf("e\n");
+fflush(stdout);
+}
+#endif
+
 /* Create the parameter list for a

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

2016-01-04 Thread spitzak 
From: Bill Spitzak 

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

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 51 +-
 1 file changed, 17 insertions(+), 34 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index aaa7de9..55073c4 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -217,25 +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   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 +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)
@@ -279,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)
@@ -291,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;
 }
 
 #if PIXMAN_GNUPLOT
@@ -354,38 +344,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 (, reconstruct_x, sample_x, sx, subsample_x);
-vert = create_1d_filter (, reconstruct_y, sample_y, sy, 
subsample_y);
+width = ceil (filters[reconstruct_x].width + sx * filters[sample_x].width);
+height = ceil (filters[reconstruct_y].width + sy * 
filters[sample_y].width);
 
-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);
 
 #if PIXMAN_GNUPLOT
 gnuplot_filter(width, subsample_x, params+4);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 pixman] implement PIXMAN_FILTER_GOOD/BEST

2016-01-04 Thread spitzak 
Changes from previous version:

- gnuplot output is now a config option
- Adjustment to cutoff between BEST filters for better match
- Reduced subsampling of GOOD/BEST from extreme value mistakenly used

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 07/14] pixman-filter: Corrections to the integral() function

2016-01-04 Thread spitzak 
From: Bill Spitzak 

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).

I removed the recursion where it splits at zero. This would only be necessary
if the curve was not smooth at this point, but it is for all integrations
(even linear.linear is smooth). Plots show that removing this makes no 
difference.

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

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 126 +
 1 file changed, 55 insertions(+), 71 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 8d65e76..d057782 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,34 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales @sample by @scale, then
- * aligns @x1 in @reconstruct with @x2 in @sample and
- * and integrates the product of the kernels across @width.
+/* This function scales @sample by @scale, 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 scale, double x2,
- double width)
+integral (pixman_kernel_t reconstruct,
+ pixman_kernel_t sample, double scale, double pos,
+ double low, double high)
 {
-/* 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)
+if (high < low)
 {
-   return
-   integral (reconstruct, x1, sample, scale, x2, - x1) +
-   integral (reconstruct, 0, sample, scale, x2 - x1, width + x1);
+   return 0.0;
 }
-else if (x2 < 0 && x2 + width > 0)
+else if (sample == PIXMAN_KERNEL_IMPULSE)
 {
-   return
-   integral (reconstruct, x1, sample, scale, x2, - x2) +
-   integral (reconstruct, x1 - x2, sample, scale, 0, width + x2);
+   return filters[reconstruct].func (-pos);
 }
 else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
 {
-   assert (width == 0.0);
-   return filters[sample].func (x2 / scale);
+   return filters[sample].func (-pos / scale) / scale;
 }
-else if (sample == PIXMAN_KERNEL_IMPULSE)
+else if (reconstruct == PIXMAN_KERNEL_BOX && sample == PIXMAN_KERNEL_BOX)
 {
-   assert (width == 0.0);
-   return filters[reconstruct].func (x1);
+   assert (high <= low + 1.0);
+   return (high - low) / scale;
 }
 else
 {
@@ -193,32 +182,30 @@ integral (pixman_kernel_t reconstruct, double x1,
 * See http://www.intmath.com/integration/6-simpsons-rule.php
 */
 #define N_SEGMENTS 16
-#define SAMPLE(a1, a2) \
-   (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / scale))
+#define SAMPLE(a)  \
+   (filters[reconstruct].func ((a)) * filters[sample].func (((a) - pos) / 
scale))

double s = 0.0;
-   double h = width / (double)N_SEGMENTS;
+   double h = (high - low) / (double)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;
+

[Pixman] [PATCH v8 09/14] pixman-filter: refactor cubic polynominal and don't range check

2016-01-04 Thread spitzak 
From: Bill Spitzak 

The other filters do not check for x being in range, so there is
no reason for cubic to do so.

v1: initial version

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 8bbf394..17e93ad 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -109,18 +109,16 @@ general_cubic (double x, double B, double C)
 
 if (ax < 1)
 {
-   return ((12 - 9 * B - 6 * C) * ax * ax * ax +
-   (-18 + 12 * B + 6 * C) * ax * ax + (6 - 2 * B)) / 6;
-}
-else if (ax >= 1 && ax < 2)
-{
-   return ((-B - 6 * C) * ax * ax * ax +
-   (6 * B + 30 * C) * ax * ax + (-12 * B - 48 * C) *
-   ax + (8 * B + 24 * C)) / 6;
+   return (((12 - 9 * B - 6 * C) * ax +
+(-18 + 12 * B + 6 * C)) * ax * ax +
+   (6 - 2 * B)) / 6;
 }
 else
 {
-   return 0;
+   return -B - 6 * C) * ax +
+(6 * B + 30 * C)) * ax +
+   (-12 * B - 48 * C)) * ax +
+   (8 * B + 24 * C)) / 6;
 }
 }
 
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 11/14] pixman-filter: Turn off subsampling for width=1 filters

2016-01-04 Thread spitzak 
From: Bill Spitzak 

Due to normalization these filters must all be identical (a single 1.0).

Also make IMPULSE.IMPULSE produce a width=1 filter, rather than zero
(which did not work).

v7: Replaced earlier tests for BOX.IMPULSE

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index b49ee32..9e50d5c 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -332,11 +332,21 @@ pixman_filter_create_separable_convolution (int   
  *n_values,
 int subsample_x, subsample_y;
 int width, height;
 
+width = ceil (filters[reconstruct_x].width + sx * filters[sample_x].width);
+if (width <= 1)
+{
+   width = 1;
+   subsample_bits_x = 0;
+}
 subsample_x = (1 << subsample_bits_x);
-subsample_y = (1 << subsample_bits_y);
 
-width = ceil (filters[reconstruct_x].width + sx * filters[sample_x].width);
 height = ceil (filters[reconstruct_y].width + sy * 
filters[sample_y].width);
+if (height <= 1)
+{
+   height = 1;
+   subsample_bits_y = 0;
+}
+subsample_y = (1 << subsample_bits_y);
 
 *n_values = 4 + width * subsample_x + height * subsample_y;
 
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 02/14] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

2016-01-04 Thread spitzak 
From: Bill Spitzak 

This allows testing of GOOD/BEST and to do comparisons between
the basic filters and PIXMAN_FILTER_SEPARABLE_CONVOLUTION settings.

Signed-off-by: Bill Spitzak 
---
 demos/scale.c  | 14 +-
 demos/scale.ui | 40 ++--
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index 0995ad0..06821e3 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -127,6 +127,15 @@ typedef struct
 intvalue;
 } named_int_t;
 
+static const named_int_t filter_types[] =
+{
+{ "Separable", PIXMAN_FILTER_SEPARABLE_CONVOLUTION },
+{ "Nearest",   PIXMAN_FILTER_NEAREST },
+{ "Bilinear",  PIXMAN_FILTER_BILINEAR },
+{ "Good",  PIXMAN_FILTER_GOOD },
+{ "Best",  PIXMAN_FILTER_BEST },
+};
+
 static const named_int_t filters[] =
 {
 { "Box",   PIXMAN_KERNEL_BOX },
@@ -260,7 +269,9 @@ rescale (GtkWidget *may_be_null, app_t *app)
gtk_adjustment_get_value (app->subsample_adjustment),
gtk_adjustment_get_value (app->subsample_adjustment));
 
-pixman_image_set_filter (app->original, 
PIXMAN_FILTER_SEPARABLE_CONVOLUTION, params, n_params);
+pixman_image_set_filter (app->original,
+   get_value (app, filter_types, "filter_combo_box"),
+   params, n_params);
 
 pixman_image_set_repeat (
 app->original, get_value (app, repeats, "repeat_combo_box"));
@@ -402,6 +413,7 @@ app_new (pixman_image_t *original)
 widget = get_widget (app, "drawing_area");
 g_signal_connect (widget, "expose_event", G_CALLBACK (on_expose), app);
 
+set_up_combo_box (app, "filter_combo_box", G_N_ELEMENTS (filter_types), 
filter_types);
 set_up_filter_box (app, "reconstruct_x_combo_box");
 set_up_filter_box (app, "reconstruct_y_combo_box");
 set_up_filter_box (app, "sample_x_combo_box");
diff --git a/demos/scale.ui b/demos/scale.ui
index ee985dd..b62cbfb 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -191,12 +191,23 @@
 8
 6
 
+  
+True
+1
+bFilter:/b
+True
+  
+
+
   
 True
 1
 bReconstruct X:/b
 True
   
+  
+1
+  
 
 
   
@@ -206,7 +217,7 @@
 True
   
   
-1
+2
   
 
 
@@ -217,7 +228,7 @@
 True
   
   
-2
+3
   
 
 
@@ -228,7 +239,7 @@
 True
   
   
-3
+4
   
 
 
@@ -239,7 +250,7 @@
 True
   
   
-4
+5
   
 
 
@@ -250,7 +261,15 @@
 True
   
   
-5
+6
+  
+
+
+  
+True
+  
+  
+1
   
 
 
@@ -259,6 +278,7 @@
   
   
 1
+1
   
 
 
@@ -267,7 +287,7 @@
   
   
 1
-1
+2
   
 
 
@@ -276,7 +296,7 @@
   
   
 1
-2
+3
   
 
 
@@ -285,7 +305,7 @@
   
   
 1
-3
+4
   
 
 
@@ -294,7 +314,7 @@
   
   
 1
-4
+

[Pixman] [PATCH v8 08/14] pixman-filter: don't range-check impulse filter

2016-01-04 Thread spitzak 
From: Bill Spitzak 

The other filters don't range-check, so there is no need for this
one to either. It is only called with x==0.

v1: initial version

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index d057782..8bbf394 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -45,7 +45,7 @@ typedef struct
 static double
 impulse_kernel (double x)
 {
-return (x == 0.0)? 1.0 : 0.0;
+return 1;
 }
 
 static double
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 04/14] pixman-filter: Consistency in arg names to integral ()

2016-01-04 Thread spitzak 
From: Bill Spitzak 

Rename kernel1/2 to reconstruct/sample and use 1/scale as the
scale argument, thus matching the names in other functions.

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index fe0d261..aaa7de9 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -147,8 +147,8 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
 };
 
-/* This function scales @kernel2 by @scale, then
- * aligns @x1 in @kernel1 with @x2 in @kernel2 and
+/* This function scales @sample by @scale, then
+ * aligns @x1 in @reconstruct with @x2 in @sample and
  * and integrates the product of the kernels across @width.
  *
  * This function assumes that the intervals are within
@@ -156,8 +156,8 @@ static const filter_info_t filters[] =
  * try to integrate a linear kernel ouside of [-1:1]
  */
 static double
-integral (pixman_kernel_t kernel1, double x1,
- pixman_kernel_t kernel2, double scale, double x2,
+integral (pixman_kernel_t reconstruct, double x1,
+ pixman_kernel_t sample, double scale, double x2,
  double width)
 {
 /* If the integration interval crosses zero, break it into
@@ -168,31 +168,31 @@ integral (pixman_kernel_t kernel1, double x1,
 if (x1 < 0 && x1 + width > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x1) +
-   integral (kernel1, 0, kernel2, scale, x2 - x1, width + x1);
+   integral (reconstruct, x1, sample, scale, x2, - x1) +
+   integral (reconstruct, 0, sample, scale, x2 - x1, width + x1);
 }
 else if (x2 < 0 && x2 + width > 0)
 {
return
-   integral (kernel1, x1, kernel2, scale, x2, - x2) +
-   integral (kernel1, x1 - x2, kernel2, scale, 0, width + x2);
+   integral (reconstruct, x1, sample, scale, x2, - x2) +
+   integral (reconstruct, x1 - x2, sample, scale, 0, width + x2);
 }
-else if (kernel1 == PIXMAN_KERNEL_IMPULSE)
+else if (reconstruct == PIXMAN_KERNEL_IMPULSE)
 {
assert (width == 0.0);
-   return filters[kernel2].func (x2 * scale);
+   return filters[sample].func (x2 / scale);
 }
-else if (kernel2 == PIXMAN_KERNEL_IMPULSE)
+else if (sample == PIXMAN_KERNEL_IMPULSE)
 {
assert (width == 0.0);
-   return filters[kernel1].func (x1);
+   return filters[reconstruct].func (x1);
 }
 else
 {
/* Integration via Simpson's rule */
 #define N_SEGMENTS 128
 #define SAMPLE(a1, a2) \
-   (filters[kernel1].func ((a1)) * filters[kernel2].func ((a2) * scale))
+   (filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / scale))

double s = 0.0;
double h = width / (double)N_SEGMENTS;
@@ -270,7 +270,7 @@ create_1d_filter (int *width,
ihigh = MIN (shigh, rhigh);
 
c = integral (reconstruct, ilow,
- sample, 1.0 / scale, ilow - pos,
+ sample, scale, ilow - pos,
  ihigh - ilow);
}
 
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 01/14] demos/scale: Compute filter size using boundary of xformed ellipse, not rectangle

2016-01-04 Thread spitzak 
From: Bill Spitzak 

This is much more accurate and less blurry. In particular the filtering does
not change as the image is rotated.

Signed-off-by: Bill Spitzak 
---
 demos/scale.c | 102 +++---
 1 file changed, 61 insertions(+), 41 deletions(-)

diff --git a/demos/scale.c b/demos/scale.c
index d00307e..0995ad0 100644
--- a/demos/scale.c
+++ b/demos/scale.c
@@ -55,50 +55,70 @@ get_widget (app_t *app, const char *name)
 return widget;
 }
 
-static double
-min4 (double a, double b, double c, double d)
-{
-double m1, m2;
-
-m1 = MIN (a, b);
-m2 = MIN (c, d);
-return MIN (m1, m2);
-}
-
-static double
-max4 (double a, double b, double c, double d)
-{
-double m1, m2;
-
-m1 = MAX (a, b);
-m2 = MAX (c, d);
-return MAX (m1, m2);
-}
-
+/* Figure out the boundary of a diameter=1 circle transformed into an ellipse
+ * by trans. Proof that this is the correct calculation:
+ *
+ * Transform x,y to u,v by this matrix calculation:
+ *
+ *  |u|   |a c| |x|
+ *  |v| = |b d|*|y|
+ *
+ * Horizontal component:
+ *
+ *  u = ax+cy (1)
+ *
+ * For each x,y on a radius-1 circle (p is angle to the point):
+ *
+ *  x^2+y^2 = 1
+ *  x = cos(p)
+ *  y = sin(p)
+ *  dx/dp = -sin(p) = -y
+ *  dy/dp = cos(p) = x
+ *
+ * Figure out derivative of (1) relative to p:
+ *
+ *  du/dp = a(dx/dp) + c(dy/dp)
+ *= -ay + cx
+ *
+ * The min and max u are when du/dp is zero:
+ *
+ *  -ay + cx = 0
+ *  cx = ay
+ *  c = ay/x  (2)
+ *  y = cx/a  (3)
+ *
+ * Substitute (2) into (1) and simplify:
+ *
+ *  u = ax + ay^2/x
+ *= a(x^2+y^2)/x
+ *= a/x (because x^2+y^2 = 1)
+ *  x = a/u (4)
+ *
+ * Substitute (4) into (3) and simplify:
+ *
+ *  y = c(a/u)/a
+ *  y = c/u (5)
+ *
+ * Square (4) and (5) and add:
+ *
+ *  x^2+y^2 = (a^2+c^2)/u^2
+ *
+ * But x^2+y^2 is 1:
+ *
+ *  1 = (a^2+c^2)/u^2
+ *  u^2 = a^2+c^2
+ *  u = hypot(a,c)
+ *
+ * Similarily the max/min of v is at:
+ *
+ *  v = hypot(b,d)
+ *
+ */
 static void
 compute_extents (pixman_f_transform_t *trans, double *sx, double *sy)
 {
-double min_x, max_x, min_y, max_y;
-pixman_f_vector_t v[4] =
-{
-   { { 1, 1, 1 } },
-   { { -1, 1, 1 } },
-   { { -1, -1, 1 } },
-   { { 1, -1, 1 } },
-};
-
-pixman_f_transform_point (trans, [0]);
-pixman_f_transform_point (trans, [1]);
-pixman_f_transform_point (trans, [2]);
-pixman_f_transform_point (trans, [3]);
-
-min_x = min4 (v[0].v[0], v[1].v[0], v[2].v[0], v[3].v[0]);
-max_x = max4 (v[0].v[0], v[1].v[0], v[2].v[0], v[3].v[0]);
-min_y = min4 (v[0].v[1], v[1].v[1], v[2].v[1], v[3].v[1]);
-max_y = max4 (v[0].v[1], v[1].v[1], v[2].v[1], v[3].v[1]);
-
-*sx = (max_x - min_x) / 2.0;
-*sy = (max_y - min_y) / 2.0;
+*sx = hypot (trans->m[0][0], trans->m[0][1]) / trans->m[2][2];
+*sy = hypot (trans->m[1][0], trans->m[1][1]) / trans->m[2][2];
 }
 
 typedef struct
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 10/14] pixman-filter: Gaussian fixes

2016-01-04 Thread spitzak 
From: Bill Spitzak 

The SIGMA term drops out on simplification.

Expanded the size slightly (from ~4.25 to 5) to make the cutoff less noticable.
The filter is truncated at a value of .001 instead of .006, this new
value is less than 1/2 of 1/255, rather than greater than it.

v1: initial version

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 17e93ad..b49ee32 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -63,10 +63,7 @@ linear_kernel (double x)
 static double
 gaussian_kernel (double x)
 {
-#define SQRT2 (1.4142135623730950488016887242096980785696718753769480)
-#define SIGMA (SQRT2 / 2.0)
-
-return exp (- x * x / (2 * SIGMA * SIGMA)) / (SIGMA * sqrt (2.0 * M_PI));
+return exp (- x * x) / sqrt (M_PI);
 }
 
 static double
@@ -139,7 +136,7 @@ static const filter_info_t filters[] =
 { PIXMAN_KERNEL_BOX,   box_kernel,   1.0 },
 { PIXMAN_KERNEL_LINEAR,linear_kernel,2.0 },
 { PIXMAN_KERNEL_CUBIC, cubic_kernel, 4.0 },
-{ PIXMAN_KERNEL_GAUSSIAN,  gaussian_kernel,  6 * SIGMA },
+{ PIXMAN_KERNEL_GAUSSIAN,  gaussian_kernel,  5.0 },
 { PIXMAN_KERNEL_LANCZOS2,  lanczos2_kernel,  4.0 },
 { PIXMAN_KERNEL_LANCZOS3,  lanczos3_kernel,  6.0 },
 { PIXMAN_KERNEL_LANCZOS3_STRETCHED, nice_kernel,  8.0 },
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 06/14] pixman-filter: Correct Simpsons integration

2016-01-04 Thread spitzak 
From: Bill Spitzak 

Simpsons uses cubic curve fitting, with 3 samples defining each cubic. This
makes the weights of the samples be in a pattern of 1,4,2,4,2...4,1, and then
dividing the result by 3.

The previous code was using weights of 1,2,6,6...6,2,1. Since it divided by
3 this produced about 2x the desired value (the normalization fixed this).
Also this is effectively a linear interpolation, not Simpsons integration.

With this fix the integration is accurate enough that the number of samples
could be reduced a lot. Likely even 16 samples is too many.

v7: Merged with patch to reduce from 128 samples to 16

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 55073c4..8d65e76 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -189,8 +189,10 @@ integral (pixman_kernel_t reconstruct, double x1,
 }
 else
 {
-   /* Integration via Simpson's rule */
-#define N_SEGMENTS 128
+   /* Integration via Simpson's rule
+* See http://www.intmath.com/integration/6-simpsons-rule.php
+*/
+#define N_SEGMENTS 16
 #define SAMPLE(a1, a2) \
(filters[reconstruct].func ((a1)) * filters[sample].func ((a2) / scale))

@@ -204,11 +206,14 @@ integral (pixman_kernel_t reconstruct, double x1,
{
double a1 = x1 + h * i;
double a2 = x2 + h * i;
+   s += 4 * SAMPLE(a1, a2);
+   }
 
-   s += 2 * SAMPLE (a1, a2);
-
-   if (i >= 2 && i < N_SEGMENTS - 1)
-   s += 4 * SAMPLE (a1, a2);
+   for (i = 2; i < N_SEGMENTS; i += 2)
+   {
+   double a1 = x1 + h * i;
+   double a2 = x2 + h * i;
+   s += 2 * SAMPLE(a1, a2);
}
 
s += SAMPLE (x1 + width, x2 + width);
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 13/14] Added big comment to filter

2016-01-04 Thread spitzak 
From: Bill Spitzak 

v8: first version

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 3237036..0961cfc 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -313,7 +313,28 @@ gnuplot_filter(int width, int samples, const 
pixman_fixed_t* p)
 #endif
 
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
- * with the given kernels and scale parameters
+ *
+ * The reconstruct filter is scaled by 1, while the sample filter is
+ * scaled by scale, and they are convolved together. Note that scale
+ * is the inverse of how the image is being scaled, if the image is
+ * being made smaller, the filter gets larger.
+ *
+ * Some interesting reconstruct.sample combinations:
+ *
+ *  IMPULSE.x - Uses the sample function only. Only works for scale >= 1. This
+ *  is useful for matching other software that does not convolve,
+ *  for large scale it is almost identical to BOX.x but the filter
+ *  is slightly smaller and thus faster.
+ *  x.IMPULSE - Uses the reconstruct function only, scale is ignored.
+ *  LINEAR.IMPULSE - Matches bilinear filtering
+ *  BOX.BOX - Produces a trapazoid-shape. Narrowest possible filter with 
antialiasing.
+ *This is equal to LINEAR.IMPULSE at scale == 1. Matches a lot of 
other
+ *software, some call it "box", others call it "linear"
+ *  IMPULSE.LINEAR - Some software calls this "linear" or "triangle". Not a 
good filter.
+ *  LINEAR.LINEAR - non-negative cubic. What some software calls "cubic 
interpolation".
+ *  IMPULSE.LANCZOS2 - Close to what a lot of other software calls "cubic 
interpolation"
+ *  IMPULSE.CUBIC - Often called "mitchell" in other software
+ *  IMPULSE.GAUSSIAN - Technically accurate but usually considered too blurry
  */
 PIXMAN_EXPORT pixman_fixed_t *
 pixman_filter_create_separable_convolution (int *n_values,
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [PATCH v8 12/14] pixman-filter: Made reconstruct==impulse and scale < 1 set scale to 1

2016-01-04 Thread spitzak 
From: Bill Spitzak 

This replaces settings that don't work (because the filter cannot be normalized)
with something that produces an image.

v7: First version with this. Previously you got lots of strange garbage filters
 that depended on the implementation.

Signed-off-by: Bill Spitzak 
---
 pixman/pixman-filter.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 9e50d5c..3237036 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -332,6 +332,8 @@ pixman_filter_create_separable_convolution (int 
*n_values,
 int subsample_x, subsample_y;
 int width, height;
 
+if (reconstruct_x == PIXMAN_KERNEL_IMPULSE && sx < 1.0)
+   sx = 1.0;
 width = ceil (filters[reconstruct_x].width + sx * filters[sample_x].width);
 if (width <= 1)
 {
@@ -340,6 +342,8 @@ pixman_filter_create_separable_convolution (int 
*n_values,
 }
 subsample_x = (1 << subsample_bits_x);
 
+if (reconstruct_y == PIXMAN_KERNEL_IMPULSE && sy < 1.0)
+   sy = 1.0;
 height = ceil (filters[reconstruct_y].width + sy * 
filters[sample_y].width);
 if (height <= 1)
 {
-- 
1.9.1

___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] Cleaning patchwork

2016-01-04 Thread Ben Avison 

On Tue, 22 Dec 2015 13:14:21 -, Oded Gabbay  wrote:


Hi Ben,

I would like to clean pixman's patchwork and you have about 20
outstanding patches.

[...]


[4/4] pixman-fast-path: Make bilinear cover fetcher use COVER_CLIP_TIGHT flag
[3/4] armv7/mips/sse2: Make bilinear cover fast paths use COVER_CLIP_TIGHT flag
[2/4] Introduce FAST_PATH_SAMPLES_COVER_CLIP_TIGHT_BILINEAR flag


I still think these are a worthwhile improvement and have described some
conditions under which it should be measurable using affine-bench. I believe
Pekka wanted to prove it using real-world traces though, and since there was
no measurable change for better or worse using the normal Cairo traces, he
was attempting to capture some new ones that would exercise the cases I
described. Last I heard though, he had found that Cairo's tracer was broken,
so he was unable to make progress. Under the circumstances, do you think
affine-bench results alone would be acceptable?


Resolve implementation-defined behaviour for division rounded to -infinity


This one got bogged down in an argument about whether C89 should still be
supported or not, which I'm not sure was ever resolved?


[05/37,v2] composite flags: Early detection of fully opaque 1x1
repeating source images
[10/37,v2] pixman-fast-path: Add in__8 fast path
[11/37,v2] armv6: Add in__8 fast path
[23/37,v2] armv6: Add optimised scanline fetchers and writeback for
r5g6b5 and a8
[24/37,v2] armv6: Add optimised scanline fetcher for a1r5g5b5
[25/37,v2] armv6: Add src_1555_ fast path


v1 of these patches were reviewed (excluding the ARM assembly parts) by
Søren on 05 Oct 2014, and v2 addressed the issue he raised. There haven't
been any comments on the reposted versions.


armv7: Re-use existing fast paths in more cases
armv7: Re-use existing fast paths in more cases


These apply the same rules that Søren suggested for my ARMv6 paths in the v2
patches above to the pre-existing ARMv7 paths as well. The only review
comment received so far was that the two patches needed different names.


[2/5,v2] armv7: Add in__8 fast path


The only difference for v2 here was again just a matter of defining the
widest possible set of pixel formats to match the fast path.


[5/5] armv7: Add src_1555_ fast path
[4/5] armv7: Add in_reverse__ fast path
[3/5] armv7: Add in_n_ fast path
[1/5] armv7: Use prefetch for small-width images too
[3/3] armv7: Use VLD-to-all-lanes
[2/3] armv7: Faster fill operations
[1/3] armv7: Coalesce scalar accesses where possible
Require destination images to be bitmaps


None of these have received any comments to date. In many cases, I suspect
it's the fact that they involve ARM assembly, and we don't have many (any?)
active reviewers who know it. I'm not sure what we can do about that.


I also suggest that for the relevant ones (if there are any), you
would rebase them on the latest master and re-send them as a single
series in order to restart the review process.


I can certainly do that, but I had previously been asked to split my
postings into smaller series that were independent. I have a lot more
patches waiting to post that depend on the ones that are stuck in limbo -
the complete set can be seen at

https://github.com/bavison/pixman/commits/arm-neon-release1

if you're interested. Or I could just post/repost the whole lot (72 patches
now), like I did with my 37-patch series from 2014.

Ben
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 03/13] pixman-image: Added GNUPLOT_OUTPUT compile-time option to view filters in gnuplot

2016-01-04 Thread Bill Spitzak 
On Mon, Jan 4, 2016 at 3:25 AM, Oded Gabbay  wrote:

> On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> > From: Bill Spitzak 
> >
> > If GNUPLOT_OUTPUT is set, then you can pipe the output of a pixman-using
> program
> > to gnuplot and get a continuously-updated plot of the horizontal filter.
> This
> > works well with demos/scale to test the filter generation.
> >
> > The plot is all the different subposition filters shuffled together.
> This is
> > misleading in a few cases:
> >
> >   IMPULSE.BOX - goes up and down as the subfilters have different
> numbers of non-zero samples
> >   IMPULSE.TRIANGLE - somewhat crooked for the same reason
> >   1-wide filters - looks triangular, but a 1-wide box would be more
> accurate
> > ---
> >  pixman/pixman-image.c | 40 
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
> > index 1ff1a49..69743c4 100644
> > --- a/pixman/pixman-image.c
> > +++ b/pixman/pixman-image.c
> > @@ -531,6 +531,46 @@ compute_image_info (pixman_image_t *image)
> >
> >  image->common.flags = flags;
> >  image->common.extended_format_code = code;
> > +/* If GNUPLOT_OUTPUT is set, then you can pipe the output of a
> pixman-using program
> > + * to gnuplot and get a continuously-updated plot of the horizontal
> filter. This
> > + * works well with demos/scale to test the filter generation.
> > + *
> > + * The plot is all the different subposition filters shuffled together.
> This is
> > + * misleading in a few cases:
> > + *
> > + *  IMPULSE.BOX - goes up and down as the subfilters have different
> numbers of non-zero samples
> > + *  IMPULSE.TRIANGLE - somewhat crooked for the same reason
> > + *  1-wide filters - looks triangular, but a 1-wide box would be more
> accurate
> > + */
> > +/* #define GNUPLOT_OUTPUT 1 */
> > +#if GNUPLOT_OUTPUT
> > +if ((flags & FAST_PATH_SEPARABLE_CONVOLUTION_FILTER) &&
> image->common.filter_params) {
> > +   const pixman_fixed_t* p = image->common.filter_params;
> > +   int width = pixman_fixed_to_int(p[0]);
> > +   int samples = 1 << pixman_fixed_to_int(p[2]);
> > +   int x,y;
> > +   p += 4;
> > +   printf("plot '-' with linespoints\n");
> > +   printf("%g 0\n", - width * .5);
> > +   for (x = 0; x < width; ++x) {
> > +   for (y = 0; y < samples; ++y) {
> > +   int yy;
> > +   if (width & 1)
> > +   yy = y;
> > +   else if (y >= samples / 2)
> > +   yy = y - samples / 2;
> > +   else
> > +   yy = samples / 2 + y;
> > +   printf("%g %g\n",
> > +  x - width * .5 + (y + .5) * (1.0 / samples),
> > +  pixman_fixed_to_double(p[(yy + 1) * width - x -
> 1]));
> > +   }
> > +   }
> > +   printf("%g 0\n", width * .5);
> > +   printf("e\n");
> > +   fflush(stdout);
> > +}
> > +#endif
> >  }
> >
> >  void
> > --
> > 1.9.1
> >
> > ___
> > Pixman mailing list
> > Pixman@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/pixman
>
> I get the big picture of this feature, and it seems useful, but we
> need write this according to standards.
> I would like to see the following changes:
>
> 1. We can do something more nice than a hard-coded #define. Let's add
> a flag to configure.ac called "enable-gnuplot-output". For something
> similar, see "enable-timers" in configure.ac. The default will be
> "no".
>

That makes sense. I actually gave up trying to figure out how to set the
CFLAGS and was forced to edit the source code to turn this on/off, which
meant I often mistakenly checked the code in with it turned on. I guess
configure is the way to go.


> 2. Take your code and put it in a new function in pixman-utils.c, and
> make sure the code inside the function is encompassed with
> #ifdef/#endif around it. In case enable-gluplot-output wasn't called,
> the function will be an empty function.
>
> 3. Call that function from compute_image_info. I guess you will want
> to call it only if:
> ((flags & FAST_PATH_SEPARABLE_CONVOLUTION_FILTER) &&
> image->common.filter_params)
> is true.
> However, we don't need to put #ifdef around the call itself.
>

I would prefer that there not be a call to the empty function there, and
that instead the config option removes the call to it here. In addition the
arguments may be non-trivial as I would like this to be able to print
filters from a future mulit-resolution data structure, to extract this from
the current structure I need to pass the width, subsamples, and pointer to
array.


> I think the above steps will create two patches:
> - Step 1&2 should be the first patch (Adding the utility).
> - Step 3 should be the second patch (Using the utility).
>

Ok.


> Is using this is simple as:
> ./ | gnuplot ?
>

Re: [Pixman] [PATCH 02/13] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

2016-01-04 Thread Bill Spitzak 
On Mon, Jan 4, 2016 at 1:24 AM, Oded Gabbay  wrote:

> On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
> > From: Bill Spitzak 
> >
> > This allows testing of GOOD/BEST and to do comparisons between
> > the basic filters and PIXMAN_FILTER_SEPARABLE_CONVOLUTION settings.
>
> You forgot to sign-off
>

Sorry, not sure what this means. Should I add something to the commit
message?
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH 02/13] demos/scale: Added pulldown to choose PIXMAN_FILTER_* value

2016-01-04 Thread Oded Gabbay 
On Mon, Jan 4, 2016 at 7:37 PM, Bill Spitzak  wrote:
>
>
> On Mon, Jan 4, 2016 at 1:24 AM, Oded Gabbay  wrote:
>>
>> On Mon, Jan 4, 2016 at 5:12 AM,   wrote:
>> > From: Bill Spitzak 
>> >
>> > This allows testing of GOOD/BEST and to do comparisons between
>> > the basic filters and PIXMAN_FILTER_SEPARABLE_CONVOLUTION settings.
>>
>> You forgot to sign-off
>
>
> Sorry, not sure what this means. Should I add something to the commit
> message?
>
>

Oh, sorry.
When you do a "git commit" command, you can add a flag "--signoff".
That flag will automatically add a line to the end of the commit
message which looks like this:

Signed-off-by: Your Name 

if you have an existing commit, you can either rewrite it and add this
line manually, or do:
"git commit --amend --signoff"

Oded
___
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman