Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

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

> On Tue, Feb 16, 2016 at 4:45 PM, Bill Spitzak  wrote:
>
>> I have a new version of this patch, which fixes a math error that
>> caused it to produce too many samples at small sizes (large scales). I
>> am not clear if I can submit a v14 of just this patch or I should
>> instead submit a v14 of the entire patch series.
>>
>
> I don't think this functionality belongs in pixman at all. Computing the
> number of subsamples is essentially a performance/quality tradeoff, which
> is something that cairo and other clients need to make for themselves. In
> general, pixman is not the place for heuristics; its API should be
> predictable and explicit.
>
> So, NACK from me on this patch.
>

The scale demo is almost useless without this as you have to adjust this
every time you change the scale.

Most software I am familiar with use a fixed number of samples for the
width of the filter, not per integer. This was an attempt to simulate that.

I guess the scale demo could do this math, though that makes it rather
difficult to use the scale demo to figure out the best settings.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

2016-03-04 Thread Søren Sandmann 
On Tue, Feb 16, 2016 at 4:45 PM, Bill Spitzak  wrote:

> I have a new version of this patch, which fixes a math error that
> caused it to produce too many samples at small sizes (large scales). I
> am not clear if I can submit a v14 of just this patch or I should
> instead submit a v14 of the entire patch series.
>

I don't think this functionality belongs in pixman at all. Computing the
number of subsamples is essentially a performance/quality tradeoff, which
is something that cairo and other clients need to make for themselves. In
general, pixman is not the place for heuristics; its API should be
predictable and explicit.

So, NACK from me on this patch.


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

Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

2016-03-02 Thread Oded Gabbay 
On Thu, Mar 3, 2016 at 8:43 AM, Bill Spitzak  wrote:
> On 02/16/2016 01:58 PM, Oded Gabbay wrote:
>>
>> On Tue, Feb 16, 2016 at 11:45 PM, Bill Spitzak  wrote:
>>>
>>> I have a new version of this patch, which fixes a math error that
>>> caused it to produce too many samples at small sizes (large scales). I
>>> am not clear if I can submit a v14 of just this patch or I should
>>> instead submit a v14 of the entire patch series.
>>> ___
>>> Pixman mailing list
>>> Pixman@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/pixman
>>
>>
>> My suggestion is to wait for Soren's review (I believe we will see
>> something around the start of next week) and when you fix his
>> comments, add this fix as well and then mark it as v14.
>>
>> Oded
>
>
> Have you been able to contact Søren?
>
>

Yes, I believe we will see his review in the next few days.

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

Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

2016-03-02 Thread Bill Spitzak 

On 02/16/2016 01:58 PM, Oded Gabbay wrote:

On Tue, Feb 16, 2016 at 11:45 PM, Bill Spitzak  wrote:

I have a new version of this patch, which fixes a math error that
caused it to produce too many samples at small sizes (large scales). I
am not clear if I can submit a v14 of just this patch or I should
instead submit a v14 of the entire patch series.
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman


My suggestion is to wait for Soren's review (I believe we will see
something around the start of next week) and when you fix his
comments, add this fix as well and then mark it as v14.

Oded


Have you been able to contact Søren?


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

Re: [Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

2016-02-16 Thread Oded Gabbay 
On Tue, Feb 16, 2016 at 11:45 PM, Bill Spitzak  wrote:
> I have a new version of this patch, which fixes a math error that
> caused it to produce too many samples at small sizes (large scales). I
> am not clear if I can submit a v14 of just this patch or I should
> instead submit a v14 of the entire patch series.
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman

My suggestion is to wait for Soren's review (I believe we will see
something around the start of next week) and when you fix his
comments, add this fix as well and then mark it as v14.

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

[Pixman] [PATCH v13 11/14] pixman-filter: Negative subsample values produces scaled result

2016-02-09 Thread spitzak 
From: Bill Spitzak 

The intention here is to produce approximately the same number of samples for
each filter size (ie width*samples is the same). This means the caller can
pass a constant rather than a different value for each size. To avoid conflict
with existing code, negative numbers are used to indicate that -n samples are
needed at size==1.

The original idea was to scale n by 1/size, but experiments led to it being
scaled by 2/((size+1)*min(size,1)). This approaches twice as many samples as
size goes to either zero or infinity. For larger sizes this is scaling by the
width of a BOX.BOX filter, but for smaller size I don't have a good
explanation.

This function was arrived at experimentally by testing the scaling of the zone
plate image rotated slightly. The smallest number of samples that did not
produce more artifacts was plotted for various scales which led to the above
formula and a value of 12 for -n.

The computed value is then rounded up to the next power of 2 to get the
subsample_bits.

The scale demo is modified to allow these negative numbers, and initially
uses -12.

v11: Put subsample calculation in it's own function
 Minor changes to comments
v12: More info in the commit message
v13: Corrected mistakes in the commit message and comments about the scale 
factor

Signed-off-by: Bill Spitzak 
Acked-by: Oded Gabbay 
---
 demos/scale.ui |  5 +++--
 pixman/pixman-filter.c | 37 +
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/demos/scale.ui b/demos/scale.ui
index b62cbfb..1e77f56 100644
--- a/demos/scale.ui
+++ b/demos/scale.ui
@@ -24,12 +24,12 @@
 10
   
   
-0
+-256
 12
 1
 1
 0
-4
+-12
   
   
 
@@ -321,6 +321,7 @@
   
 True
subsample_adjustment
+   -12
   
   
 1
diff --git a/pixman/pixman-filter.c b/pixman/pixman-filter.c
index 56e25b1..cb9f72b 100644
--- a/pixman/pixman-filter.c
+++ b/pixman/pixman-filter.c
@@ -346,6 +346,41 @@ filter_width (pixman_kernel_t reconstruct, pixman_kernel_t 
sample, double size)
 return ceil (filters[reconstruct].width + size * filters[sample].width);
 }
 
+/* Turn negative number into ceil(ln2(-n * 2/(size+1)*min(size,1)))
+ *
+ * The original idea was to scale n by 1/size, but experiments led to
+ * it being scaled by 2/((size+1)*min(size,1)). This approaches twice
+ * as many samples as size goes to either zero or infinity. For larger
+ * sizes this is scaling by the width of a BOX.BOX filter, but for
+ * smaller size I don't have a good explanation.
+ *
+ * This function was arrived at experimentally by testing the scaling
+ * of the zone plate image rotated slightly. The smallest number of
+ * samples that did not produce more artifacts was plotted for various
+ * scales which led to the above formula and a value of 12 for -n.
+ */
+static int
+subsample_bits (int subsample_bits, pixman_kernel_t sample, double size)
+{
+if (subsample_bits < 0)
+{
+   double desired_samples = -subsample_bits;
+   if (sample == PIXMAN_KERNEL_IMPULSE)
+   ; /* For x.IMPULSE no scaling is done */
+   else if (size >= 1.0)
+   desired_samples *= 2.0 / (size + 1.0);
+   else
+   desired_samples *= 2.0 / ((size + 1.0) * size);
+   if (desired_samples <= 1.0)
+   subsample_bits = 0;
+   else if (desired_samples >= 256.0)
+   subsample_bits = 8;
+   else
+   subsample_bits = (int) ceil (log2(desired_samples) - .01);
+}
+return subsample_bits;
+}
+
 /* Create the parameter list for a SEPARABLE_CONVOLUTION filter
  * with the given kernels and size parameters
  */
@@ -367,9 +402,11 @@ pixman_filter_create_separable_convolution (int
 *n_values,
 int width, height;
 
 width = filter_width (reconstruct_x, sample_x, sx);
+subsample_bits_x = subsample_bits (subsample_bits_x, sample_x, sx);
 subsample_x = (1 << subsample_bits_x);
 
 height = filter_width (reconstruct_y, sample_y, sy);
+subsample_bits_y = subsample_bits (subsample_bits_y, sample_y, sy);
 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
https://lists.freedesktop.org/mailman/listinfo/pixman