Re: [Pixman] [PATCH v13 05/14] pixman-filter: Consistency in arg names to integral ()

2016-03-04 Thread Bill Spitzak
On Fri, Mar 4, 2016 at 2:51 PM, Søren Sandmann 
wrote:

> On Wed, Feb 10, 2016 at 1:25 AM,  wrote:
>
>> From: Bill Spitzak 
>>
>> Rename kernel1/2 to reconstruct/sample to match the other functions.
>> Rename "scale" to "size" to avoid confusion with the scale being applied
>> to the image, which is the reciprocol of this value.
>>
>> v10: Renamed "scale" to "size"
>>
>
> I don't really agree with this patch. The intention behind the code was
> that the integral() function shouldn't know anything about scaling or
> images; all it would do was compute integrals over functions, but it
> wouldn't know or care what those functions were used for.
>

Okay will keep the arguments as before. In fact "scale" is used as expected
in the old version of the code (larger numbers mean the picture is being
enlarged). I still think renaming kernel1/2 to be the same name used in
other functions makes things a lot easier to follow.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


Re: [Pixman] [PATCH v13 05/14] pixman-filter: Consistency in arg names to integral ()

2016-03-04 Thread Søren Sandmann
On Wed, Feb 10, 2016 at 1:25 AM,  wrote:

> From: Bill Spitzak 
>
> Rename kernel1/2 to reconstruct/sample to match the other functions.
> Rename "scale" to "size" to avoid confusion with the scale being applied
> to the image, which is the reciprocol of this value.
>
> v10: Renamed "scale" to "size"
>

I don't really agree with this patch. The intention behind the code was
that the integral() function shouldn't know anything about scaling or
images; all it would do was compute integrals over functions, but it
wouldn't know or care what those functions were used for.


Søren
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


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

2016-02-09 Thread spitzak
From: Bill Spitzak 

Rename kernel1/2 to reconstruct/sample to match the other functions.
Rename "scale" to "size" to avoid confusion with the scale being applied
to the image, which is the reciprocol of this value.

v10: Renamed "scale" to "size"

Signed-off-by: Bill Spitzak 
Reviewed-by: Oded Gabbay 
---
 pixman/pixman-filter.c | 48 +++-
 pixman/pixman.h|  6 +++---
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 3981c8b..b70da1f 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 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 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 size, 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, size, x2, - x1) +
+   integral (reconstruct, 0, sample, size, 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, size, x2, - x2) +
+   integral (reconstruct, x1 - x2, sample, size, 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 / size);
 }
-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) / size))

double s = 0.0;
double h = width / (double)N_SEGMENTS;
@@ -221,16 +221,14 @@ static pixman_fixed_t *
 create_1d_filter (int *width,
  pixman_kernel_t  reconstruct,
  pixman_kernel_t  sample,
- double   scale,
+ double   size,
  int  n_phases)
 {
 pixman_fixed_t *params, *p;
 double step;
-double size;
 int i;
 
-size = scale * filters[sample].width + filters[reconstruct].width;
-*width = ceil (size);
+*width = ceil (size * filters[sample].width + filters[reconstruct].width);
 
 p = params = malloc (*width * n_phases * sizeof (pixman_fixed_t));
 if (!params)
@@ -259,8 +257,8 @@ create_1d_filter (int *width,
double pos = x + 0.5 - frac;
double rlow = - filters[reconstruct].width / 2.0;
double rhigh = rlow + filters[reconstruct].width;
-   double slow = pos - scale * filters[sample].width / 2.0;
-   double shigh = slow + scale * filters[sample].width;
+   double slow = pos - size * filters[sample].width / 2.0;
+   double shigh = slow + size * filters[sample].width;
double c = 0.0;
double ilow, ihigh;
 
@@ -270,7 +268,7 @@ create_1d_filter (int *width,
ihigh = MIN (shigh, rhigh);
 
c = integral (reconstruct, ilow,
- sample, 1.0 / scale, ilow - pos,
+ sample, size, ilow - pos,
  ihigh - ilow);
}
 
@@ -339,12 +337,12 @@ 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
+ * with the