[1/4] Implement floating point gradient computation

Submitted by Basile Clement on Dec. 3, 2018, 2:55 p.m.

Details

Message ID 20181203145530.9649-1-basile-pixman@clement.pm
State New
Series "Series without cover letter"
Headers show

Commit Message

Basile Clement Dec. 3, 2018, 2:55 p.m.
This patch modifies the gradient walker to be able to generate floating
point values directly in addition to a8r8g8b8 32 bit values.  This is
then used by the various gradient implementations to render in floating
point when asked to do so, instead of rendering to a8r8g8b8 and then
expanding to floating point as they were doing previously.
---
 pixman/pixman-conical-gradient.c |  36 ++++++----
 pixman/pixman-gradient-walker.c  |  92 ++++++++++++++++++-------
 pixman/pixman-linear-gradient.c  |  49 ++++++++------
 pixman/pixman-private.h          |  65 ++++++++++++++----
 pixman/pixman-radial-gradient.c  | 112 ++++++++++++++++++-------------
 5 files changed, 234 insertions(+), 120 deletions(-)

Patch hide | download patch | download mbox

diff --git a/pixman/pixman-conical-gradient.c b/pixman/pixman-conical-gradient.c
index 8bb46ae..a39e20c 100644
--- a/pixman/pixman-conical-gradient.c
+++ b/pixman/pixman-conical-gradient.c
@@ -51,7 +51,10 @@  coordinates_to_parameter (double x, double y, double angle)
 }
 
 static uint32_t *
-conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
+conical_get_scanline (pixman_iter_t                 *iter,
+		      const uint32_t                *mask,
+		      int                            Bpp,
+		      pixman_gradient_walker_write_t write_pixel)
 {
     pixman_image_t *image = iter->image;
     int x = iter->x;
@@ -61,7 +64,7 @@  conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 
     gradient_t *gradient = (gradient_t *)image;
     conical_gradient_t *conical = (conical_gradient_t *)image;
-    uint32_t       *end = buffer + width;
+    uint32_t       *end = buffer + width * (Bpp / 4);
     pixman_gradient_walker_t walker;
     pixman_bool_t affine = TRUE;
     double cx = 1.;
@@ -109,11 +112,12 @@  conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 	    {
 		double t = coordinates_to_parameter (rx, ry, conical->angle);
 
-		*buffer = _pixman_gradient_walker_pixel (
-		    &walker, (pixman_fixed_48_16_t)pixman_double_to_fixed (t));
+		write_pixel (&walker,
+			     (pixman_fixed_48_16_t)pixman_double_to_fixed (t),
+			     buffer);
 	    }
 
-	    ++buffer;
+	    buffer += (Bpp / 4);
 
 	    rx += cx;
 	    ry += cy;
@@ -144,11 +148,12 @@  conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 
 		t = coordinates_to_parameter (x, y, conical->angle);
 
-		*buffer = _pixman_gradient_walker_pixel (
-		    &walker, (pixman_fixed_48_16_t)pixman_double_to_fixed (t));
+		write_pixel (&walker,
+			     (pixman_fixed_48_16_t)pixman_double_to_fixed (t),
+			     buffer);
 	    }
 
-	    ++buffer;
+	    buffer += (Bpp / 4);
 
 	    rx += cx;
 	    ry += cy;
@@ -161,14 +166,17 @@  conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 }
 
 static uint32_t *
-conical_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
+conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 {
-    uint32_t *buffer = conical_get_scanline_narrow (iter, NULL);
-
-    pixman_expand_to_float (
-	(argb_t *)buffer, buffer, PIXMAN_a8r8g8b8, iter->width);
+    return conical_get_scanline (iter, mask, 4,
+				 _pixman_gradient_walker_write_narrow);
+}
 
-    return buffer;
+static uint32_t *
+conical_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
+{
+    return conical_get_scanline (iter, NULL, 16,
+				 _pixman_gradient_walker_write_wide);
 }
 
 void
diff --git a/pixman/pixman-gradient-walker.c b/pixman/pixman-gradient-walker.c
index 822f8e6..9f11cf4 100644
--- a/pixman/pixman-gradient-walker.c
+++ b/pixman/pixman-gradient-walker.c
@@ -122,10 +122,10 @@  gradient_walker_reset (pixman_gradient_walker_t *walker,
 	    left_c = right_c;
     }
 
-    /* The alpha channel is scaled to be in the [0, 255] interval,
+    /* The alpha channel is scaled to be in the [0, 1] interval,
      * and the red/green/blue channels are scaled to be in [0, 1].
      * This ensures that after premultiplication all channels will
-     * be in the [0, 255] interval.
+     * be in the [0, 1] interval.
      */
     la = (left_c->alpha * (1.0f/257.0f));
     lr = (left_c->red * (1.0f/257.0f));
@@ -143,7 +143,7 @@  gradient_walker_reset (pixman_gradient_walker_t *walker,
     if (FLOAT_IS_ZERO (rx - lx) || left_x == INT32_MIN || right_x == INT32_MAX)
     {
 	walker->a_s = walker->r_s = walker->g_s = walker->b_s = 0.0f;
-	walker->a_b = (la + ra) / 2.0f;
+	walker->a_b = (la + ra) / 510.0f;
 	walker->r_b = (lr + rr) / 510.0f;
 	walker->g_b = (lg + rg) / 510.0f;
 	walker->b_b = (lb + rb) / 510.0f;
@@ -152,12 +152,12 @@  gradient_walker_reset (pixman_gradient_walker_t *walker,
     {
 	float w_rec = 1.0f / (rx - lx);
 
-	walker->a_b = (la * rx - ra * lx) * w_rec;
+	walker->a_b = (la * rx - ra * lx) * w_rec * (1.0f/255.0f);
 	walker->r_b = (lr * rx - rr * lx) * w_rec * (1.0f/255.0f);
 	walker->g_b = (lg * rx - rg * lx) * w_rec * (1.0f/255.0f);
 	walker->b_b = (lb * rx - rb * lx) * w_rec * (1.0f/255.0f);
 
-	walker->a_s = (ra - la) * w_rec;
+	walker->a_s = (ra - la) * w_rec * (1.0f/255.0f);
 	walker->r_s = (rr - lr) * w_rec * (1.0f/255.0f);
 	walker->g_s = (rg - lg) * w_rec * (1.0f/255.0f);
 	walker->b_s = (rb - lb) * w_rec * (1.0f/255.0f);
@@ -169,34 +169,78 @@  gradient_walker_reset (pixman_gradient_walker_t *walker,
     walker->need_reset = FALSE;
 }
 
-uint32_t
-_pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
-                               pixman_fixed_48_16_t      x)
+static argb_t
+pixman_gradient_walker_pixel_float (pixman_gradient_walker_t *walker,
+				    pixman_fixed_48_16_t      x)
 {
-    float a, r, g, b;
-    uint8_t a8, r8, g8, b8;
-    uint32_t v;
+    argb_t f;
     float y;
 
     if (walker->need_reset || x < walker->left_x || x >= walker->right_x)
-        gradient_walker_reset (walker, x);
+	gradient_walker_reset (walker, x);
 
     y = x * (1.0f / 65536.0f);
 
-    a = walker->a_s * y + walker->a_b;
-    r = a * (walker->r_s * y + walker->r_b);
-    g = a * (walker->g_s * y + walker->g_b);
-    b = a * (walker->b_s * y + walker->b_b);
+    f.a = walker->a_s * y + walker->a_b;
+    f.r = f.a * (walker->r_s * y + walker->r_b);
+    f.g = f.a * (walker->g_s * y + walker->g_b);
+    f.b = f.a * (walker->b_s * y + walker->b_b);
+
+    return f;
+}
 
-    a8 = a + 0.5f;
-    r8 = r + 0.5f;
-    g8 = g + 0.5f;
-    b8 = b + 0.5f;
+static uint32_t
+pixman_gradient_walker_pixel_32 (pixman_gradient_walker_t *walker,
+				 pixman_fixed_48_16_t      x)
+{
+    argb_t f = pixman_gradient_walker_pixel_float (walker, x);
+    uint32_t v;
 
-    v = ((a8 << 24) & 0xff000000) |
-        ((r8 << 16) & 0x00ff0000) |
-        ((g8 <<  8) & 0x0000ff00) |
-        ((b8 >>  0) & 0x000000ff);
+    pixman_contract_from_float (&v, &f, 1);
 
     return v;
 }
+
+void
+_pixman_gradient_walker_write_narrow (pixman_gradient_walker_t *walker,
+				      pixman_fixed_48_16_t      x,
+				      uint32_t                 *buffer)
+{
+    *buffer = pixman_gradient_walker_pixel_32 (walker, x);
+}
+
+void
+_pixman_gradient_walker_write_wide (pixman_gradient_walker_t *walker,
+				    pixman_fixed_48_16_t      x,
+				    uint32_t                 *buffer)
+{
+    *(argb_t *)buffer = pixman_gradient_walker_pixel_float (walker, x);
+}
+
+void
+_pixman_gradient_walker_fill_narrow (pixman_gradient_walker_t *walker,
+				     pixman_fixed_48_16_t      x,
+				     uint32_t                 *buffer,
+				     uint32_t                 *end)
+{
+    register uint32_t color;
+
+    color = pixman_gradient_walker_pixel_32 (walker, x);
+    while (buffer < end)
+	*buffer++ = color;
+}
+
+void
+_pixman_gradient_walker_fill_wide (pixman_gradient_walker_t *walker,
+				   pixman_fixed_48_16_t      x,
+				   uint32_t                 *buffer,
+				   uint32_t                 *end)
+{
+    register argb_t color;
+    argb_t *buffer_wide = (argb_t *)buffer;
+    argb_t *end_wide    = (argb_t *)end;
+
+    color = pixman_gradient_walker_pixel_float (walker, x);
+    while (buffer_wide < end_wide)
+	*buffer_wide++ = color;
+}
diff --git a/pixman/pixman-linear-gradient.c b/pixman/pixman-linear-gradient.c
index 40c8c9f..a5d8be9 100644
--- a/pixman/pixman-linear-gradient.c
+++ b/pixman/pixman-linear-gradient.c
@@ -89,8 +89,11 @@  linear_gradient_is_horizontal (pixman_image_t *image,
 }
 
 static uint32_t *
-linear_get_scanline_narrow (pixman_iter_t  *iter,
-			    const uint32_t *mask)
+linear_get_scanline (pixman_iter_t                 *iter,
+		     const uint32_t                *mask,
+		     int                            Bpp,
+		     pixman_gradient_walker_write_t write_pixel,
+		     pixman_gradient_walker_fill_t  fill_pixel)
 {
     pixman_image_t *image  = iter->image;
     int             x      = iter->x;
@@ -103,10 +106,10 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
     pixman_fixed_48_16_t dx, dy;
     gradient_t *gradient = (gradient_t *)image;
     linear_gradient_t *linear = (linear_gradient_t *)image;
-    uint32_t *end = buffer + width;
+    uint32_t *end = buffer + width * (Bpp / 4);
     pixman_gradient_walker_t walker;
 
-    _pixman_gradient_walker_init (&walker, gradient, image->common.repeat);
+    _pixman_gradient_walker_init (&walker,gradient, image->common.repeat);
 
     /* reference point is the center of the pixel */
     v.vector[0] = pixman_int_to_fixed (x) + pixman_fixed_1 / 2;
@@ -137,7 +140,7 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
     if (l == 0 || unit.vector[2] == 0)
     {
 	/* affine transformation only */
-        pixman_fixed_32_32_t t, next_inc;
+	pixman_fixed_32_32_t t, next_inc;
 	double inc;
 
 	if (l == 0 || v.vector[2] == 0)
@@ -152,7 +155,7 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
 	    invden = pixman_fixed_1 * (double) pixman_fixed_1 /
 		(l * (double) v.vector[2]);
 	    v2 = v.vector[2] * (1. / pixman_fixed_1);
-	    t = ((dx * v.vector[0] + dy * v.vector[1]) - 
+	    t = ((dx * v.vector[0] + dy * v.vector[1]) -
 		 (dx * linear->p1.x + dy * linear->p1.y) * v2) * invden;
 	    inc = (dx * unit.vector[0] + dy * unit.vector[1]) * invden;
 	}
@@ -160,11 +163,7 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
 
 	if (((pixman_fixed_32_32_t )(inc * width)) == 0)
 	{
-	    register uint32_t color;
-
-	    color = _pixman_gradient_walker_pixel (&walker, t);
-	    while (buffer < end)
-		*buffer++ = color;
+	    fill_pixel (&walker, t, buffer, end);
 	}
 	else
 	{
@@ -175,12 +174,11 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
 	    {
 		if (!mask || *mask++)
 		{
-		    *buffer = _pixman_gradient_walker_pixel (&walker,
-							     t + next_inc);
+		    write_pixel (&walker, t + next_inc, buffer);
 		}
 		i++;
 		next_inc = inc * i;
-		buffer++;
+		buffer += (Bpp / 4);
 	    }
 	}
     }
@@ -202,14 +200,14 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
 		    invden = pixman_fixed_1 * (double) pixman_fixed_1 /
 			(l * (double) v.vector[2]);
 		    v2 = v.vector[2] * (1. / pixman_fixed_1);
-		    t = ((dx * v.vector[0] + dy * v.vector[1]) - 
+		    t = ((dx * v.vector[0] + dy * v.vector[1]) -
 			 (dx * linear->p1.x + dy * linear->p1.y) * v2) * invden;
 		}
 
-		*buffer = _pixman_gradient_walker_pixel (&walker, t);
+		write_pixel (&walker, t, buffer);
 	    }
 
-	    ++buffer;
+	    buffer += (Bpp / 4);
 
 	    v.vector[0] += unit.vector[0];
 	    v.vector[1] += unit.vector[1];
@@ -223,14 +221,21 @@  linear_get_scanline_narrow (pixman_iter_t  *iter,
 }
 
 static uint32_t *
-linear_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
+linear_get_scanline_narrow (pixman_iter_t  *iter,
+			    const uint32_t *mask)
 {
-    uint32_t *buffer = linear_get_scanline_narrow (iter, NULL);
+    return linear_get_scanline (iter, mask, 4,
+				_pixman_gradient_walker_write_narrow,
+				_pixman_gradient_walker_fill_narrow);
+}
 
-    pixman_expand_to_float (
-	(argb_t *)buffer, buffer, PIXMAN_a8r8g8b8, iter->width);
 
-    return buffer;
+static uint32_t *
+linear_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
+{
+    return linear_get_scanline (iter, NULL, 16,
+				_pixman_gradient_walker_write_wide,
+				_pixman_gradient_walker_fill_wide);
 }
 
 void
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 73a5414..0a07467 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -340,18 +340,18 @@  _pixman_image_validate (pixman_image_t *image);
  */
 typedef struct
 {
-    float		    a_s, a_b;
-    float		    r_s, r_b;
-    float		    g_s, g_b;
-    float		    b_s, b_b;
-    pixman_fixed_48_16_t    left_x;
-    pixman_fixed_48_16_t    right_x;
-
-    pixman_gradient_stop_t *stops;
-    int                     num_stops;
-    pixman_repeat_t	    repeat;
-
-    pixman_bool_t           need_reset;
+    float		     a_s, a_b;
+    float		     r_s, r_b;
+    float		     g_s, g_b;
+    float		     b_s, b_b;
+    pixman_fixed_48_16_t     left_x;
+    pixman_fixed_48_16_t     right_x;
+
+    pixman_gradient_stop_t  *stops;
+    int                      num_stops;
+    pixman_repeat_t	     repeat;
+
+    pixman_bool_t            need_reset;
 } pixman_gradient_walker_t;
 
 void
@@ -363,9 +363,46 @@  void
 _pixman_gradient_walker_reset (pixman_gradient_walker_t *walker,
                                pixman_fixed_48_16_t      pos);
 
+argb_t
+_pixman_gradient_walker_pixel_float (pixman_gradient_walker_t *walker,
+				     pixman_fixed_48_16_t      x);
+
 uint32_t
-_pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
-                               pixman_fixed_48_16_t      x);
+_pixman_gradient_walker_pixel_32 (pixman_gradient_walker_t *walker,
+				  pixman_fixed_48_16_t      x);
+
+typedef void (*pixman_gradient_walker_write_t) (
+    pixman_gradient_walker_t *walker,
+    pixman_fixed_48_16_t      x,
+    uint32_t                 *buffer);
+
+void
+_pixman_gradient_walker_write_narrow(pixman_gradient_walker_t *walker,
+				     pixman_fixed_48_16_t      x,
+				     uint32_t                 *buffer);
+
+void
+_pixman_gradient_walker_write_wide(pixman_gradient_walker_t *walker,
+				   pixman_fixed_48_16_t      x,
+				   uint32_t                 *buffer);
+
+typedef void (*pixman_gradient_walker_fill_t) (
+    pixman_gradient_walker_t *walker,
+    pixman_fixed_48_16_t      x,
+    uint32_t                 *buffer,
+    uint32_t                 *end);
+
+void
+_pixman_gradient_walker_fill_narrow(pixman_gradient_walker_t *walker,
+				    pixman_fixed_48_16_t      x,
+				    uint32_t                 *buffer,
+				    uint32_t                 *end);
+
+void
+_pixman_gradient_walker_fill_wide(pixman_gradient_walker_t *walker,
+				  pixman_fixed_48_16_t      x,
+				  uint32_t                 *buffer,
+				  uint32_t                 *end);
 
 /*
  * Edges
diff --git a/pixman/pixman-radial-gradient.c b/pixman/pixman-radial-gradient.c
index 6a21796..0367d78 100644
--- a/pixman/pixman-radial-gradient.c
+++ b/pixman/pixman-radial-gradient.c
@@ -66,15 +66,18 @@  fdot (double x1,
     return x1 * x2 + y1 * y2 + z1 * z2;
 }
 
-static uint32_t
-radial_compute_color (double                    a,
-		      double                    b,
-		      double                    c,
-		      double                    inva,
-		      double                    dr,
-		      double                    mindr,
-		      pixman_gradient_walker_t *walker,
-		      pixman_repeat_t           repeat)
+static void
+radial_write_color (double                         a,
+		    double                         b,
+		    double                         c,
+		    double                         inva,
+		    double                         dr,
+		    double                         mindr,
+		    pixman_gradient_walker_t      *walker,
+		    pixman_repeat_t                repeat,
+		    int                            Bpp,
+		    pixman_gradient_walker_write_t write_pixel,
+		    uint32_t                      *buffer)
 {
     /*
      * In this function error propagation can lead to bad results:
@@ -99,21 +102,25 @@  radial_compute_color (double                    a,
 	double t;
 
 	if (b == 0)
-	    return 0;
+	{
+	    memset (buffer, 0, Bpp);
+	    return;
+	}
 
 	t = pixman_fixed_1 / 2 * c / b;
 	if (repeat == PIXMAN_REPEAT_NONE)
 	{
 	    if (0 <= t && t <= pixman_fixed_1)
-		return _pixman_gradient_walker_pixel (walker, t);
+		return write_pixel (walker, t, buffer);
 	}
 	else
 	{
 	    if (t * dr >= mindr)
-		return _pixman_gradient_walker_pixel (walker, t);
+		return write_pixel (walker, t, buffer);
 	}
 
-	return 0;
+	memset (buffer, 0, Bpp);
+	return;
     }
 
     discr = fdot (b, a, 0, b, -c, 0);
@@ -139,24 +146,28 @@  radial_compute_color (double                    a,
 	if (repeat == PIXMAN_REPEAT_NONE)
 	{
 	    if (0 <= t0 && t0 <= pixman_fixed_1)
-		return _pixman_gradient_walker_pixel (walker, t0);
+		return write_pixel (walker, t0, buffer);
 	    else if (0 <= t1 && t1 <= pixman_fixed_1)
-		return _pixman_gradient_walker_pixel (walker, t1);
+		return write_pixel (walker, t1, buffer);
 	}
 	else
 	{
 	    if (t0 * dr >= mindr)
-		return _pixman_gradient_walker_pixel (walker, t0);
+		return write_pixel (walker, t0, buffer);
 	    else if (t1 * dr >= mindr)
-		return _pixman_gradient_walker_pixel (walker, t1);
+		return write_pixel (walker, t1, buffer);
 	}
     }
 
-    return 0;
+    memset (buffer, 0, Bpp);
+    return;
 }
 
 static uint32_t *
-radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
+radial_get_scanline (pixman_iter_t                 *iter,
+		     const uint32_t                *mask,
+		     int                            Bpp,
+		     pixman_gradient_walker_write_t write_pixel)
 {
     /*
      * Implementation of radial gradients following the PDF specification.
@@ -247,7 +258,7 @@  radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 
     gradient_t *gradient = (gradient_t *)image;
     radial_gradient_t *radial = (radial_gradient_t *)image;
-    uint32_t *end = buffer + width;
+    uint32_t *end = buffer + width * (Bpp / 4);
     pixman_gradient_walker_t walker;
     pixman_vector_t v, unit;
 
@@ -330,18 +341,21 @@  radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 	{
 	    if (!mask || *mask++)
 	    {
-		*buffer = radial_compute_color (radial->a, b, c,
-						radial->inva,
-						radial->delta.radius,
-						radial->mindr,
-						&walker,
-						image->common.repeat);
+		radial_write_color (radial->a, b, c,
+				    radial->inva,
+				    radial->delta.radius,
+				    radial->mindr,
+				    &walker,
+				    image->common.repeat,
+				    Bpp,
+				    write_pixel,
+				    buffer);
 	    }
 
 	    b += db;
 	    c += dc;
 	    dc += ddc;
-	    ++buffer;
+	    buffer += (Bpp / 4);
 	}
     }
     else
@@ -375,20 +389,23 @@  radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 			      pdx, pdy, radial->c1.radius);
 		    /*  / pixman_fixed_1 / pixman_fixed_1 */
 
-		    *buffer = radial_compute_color (radial->a, b, c,
-						    radial->inva,
-						    radial->delta.radius,
-						    radial->mindr,
-						    &walker,
-						    image->common.repeat);
+		    radial_write_color (radial->a, b, c,
+					radial->inva,
+					radial->delta.radius,
+					radial->mindr,
+					&walker,
+					image->common.repeat,
+					Bpp,
+					write_pixel,
+					buffer);
 		}
 		else
 		{
-		    *buffer = 0;
+		    memset (buffer, 0, Bpp);
 		}
 	    }
 
-	    ++buffer;
+	    buffer += (Bpp / 4);
 
 	    v.vector[0] += unit.vector[0];
 	    v.vector[1] += unit.vector[1];
@@ -401,14 +418,17 @@  radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 }
 
 static uint32_t *
-radial_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
+radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 {
-    uint32_t *buffer = radial_get_scanline_narrow (iter, NULL);
-
-    pixman_expand_to_float (
-	(argb_t *)buffer, buffer, PIXMAN_a8r8g8b8, iter->width);
+    return radial_get_scanline (iter, mask, 4,
+				_pixman_gradient_walker_write_narrow);
+}
 
-    return buffer;
+static uint32_t *
+radial_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
+{
+    return radial_get_scanline (iter, NULL, 16,
+				_pixman_gradient_walker_write_wide);
 }
 
 void
@@ -422,11 +442,11 @@  _pixman_radial_gradient_iter_init (pixman_image_t *image, pixman_iter_t *iter)
 
 PIXMAN_EXPORT pixman_image_t *
 pixman_image_create_radial_gradient (const pixman_point_fixed_t *  inner,
-                                     const pixman_point_fixed_t *  outer,
-                                     pixman_fixed_t                inner_radius,
-                                     pixman_fixed_t                outer_radius,
-                                     const pixman_gradient_stop_t *stops,
-                                     int                           n_stops)
+				     const pixman_point_fixed_t *  outer,
+				     pixman_fixed_t                inner_radius,
+				     pixman_fixed_t                outer_radius,
+				     const pixman_gradient_stop_t *stops,
+				     int                           n_stops)
 {
     pixman_image_t *image;
     radial_gradient_t *radial;

Comments

Maarten Lankhorst Dec. 4, 2018, 4:36 p.m.
Hey,

Op 03-12-2018 om 16:01 schreef Basile Clement:
>
> This is essentially a duplicate of the recent patch by Maarten Lankhorst (which spurred me into finally posting these, which laid on my laptop for a couple months), with a slightly different approach.  It is mostly included as it is a prerequisite for the subsequent patches, but they should work just as well with Maarten's patch.
>
> This series of patches is driven by a discussion with Marc Jeanmougin and the need of having dithering in pixman for inkscape, and tries to implement the recommendations given by Soren in previous threads on the topic.
>
Series looks sane, 1/4 is cleaner than my version. I would change Bpp to cpp, or multiply by 8, since Bpp usually means bits per pixel.

Do you happen to have a patch to fix pixman-bits-image.c falling back to 8-bit paths as well?

~Maarten
Pekka Paalanen Dec. 5, 2018, 8:08 a.m.
On Tue, 4 Dec 2018 17:36:18 +0100
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:

> Hey,
> 
> Op 03-12-2018 om 16:01 schreef Basile Clement:
> >
> > This is essentially a duplicate of the recent patch by Maarten
> > Lankhorst (which spurred me into finally posting these, which laid
> > on my laptop for a couple months), with a slightly different
> > approach.  It is mostly included as it is a prerequisite for the
> > subsequent patches, but they should work just as well with
> > Maarten's patch.
> >
> > This series of patches is driven by a discussion with Marc
> > Jeanmougin and the need of having dithering in pixman for inkscape,
> > and tries to implement the recommendations given by Soren in
> > previous threads on the topic. 

> Series looks sane, 1/4 is cleaner than my version. I would change Bpp
> to cpp, or multiply by 8, since Bpp usually means bits per pixel.

And cpp means channels-per-pixel?

B usually means bytes, b often means bits. But not always, so you can
never assume.

I'd just write out bits_per_pixel or bytes_per_pixel, just like
pitch_bytes or stride_pixels or stride_uint32s or whatever makes it
totally obvious.


2c, thanks,
pq
Basile Clement Dec. 5, 2018, 10:24 a.m.
Hi,

On 12/5/18 9:08 AM, Pekka Paalanen wrote:
> On Tue, 4 Dec 2018 17:36:18 +0100
> Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>> Series looks sane, 1/4 is cleaner than my version. I would change Bpp
>> to cpp, or multiply by 8, since Bpp usually means bits per pixel.
> And cpp means channels-per-pixel?
>
> B usually means bytes, b often means bits. But not always, so you can
> never assume.
>
> I'd just write out bits_per_pixel or bytes_per_pixel, just like
> pitch_bytes or stride_pixels or stride_uint32s or whatever makes it
> totally obvious.

This was indeed Bpp as in Bytes-per-pixel; I reused the variable name 
from pixman-general.c.  Happy to change it to the more explicit 
bytes_per_pixel.

On 12/4/18 5:36 PM, Maarten Lankhorst wrote:
> Do you happen to have a patch to fix pixman-bits-image.c falling back to 8-bit paths as well?

Are you referring to the fact that `get_scanline_float` gets data 
through `get_scanline_32` first?  If so, I have not -- I understand this 
is only an issue with floating point formats, which were added after I 
wrote the patches.

- Basile
Maarten Lankhorst Dec. 6, 2018, 11:03 a.m.
Op 05-12-2018 om 11:24 schreef Basile Clement:
> Hi,
>
> On 12/5/18 9:08 AM, Pekka Paalanen wrote:
>> On Tue, 4 Dec 2018 17:36:18 +0100
>> Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>>> Series looks sane, 1/4 is cleaner than my version. I would change Bpp
>>> to cpp, or multiply by 8, since Bpp usually means bits per pixel.
>> And cpp means channels-per-pixel?
>>
>> B usually means bytes, b often means bits. But not always, so you can
>> never assume.
>>
>> I'd just write out bits_per_pixel or bytes_per_pixel, just like
>> pitch_bytes or stride_pixels or stride_uint32s or whatever makes it
>> totally obvious.
>
> This was indeed Bpp as in Bytes-per-pixel; I reused the variable name from pixman-general.c.  Happy to change it to the more explicit bytes_per_pixel.
>
> On 12/4/18 5:36 PM, Maarten Lankhorst wrote:
>> Do you happen to have a patch to fix pixman-bits-image.c falling back to 8-bit paths as well?
>
> Are you referring to the fact that `get_scanline_float` gets data through `get_scanline_32` first?  If so, I have not -- I understand this is only an issue with floating point formats, which were added after I wrote the patches. 

This is also an issue with any format with >8 bpp, see PIXMAN_FORMAT_IS_WIDE(). This also affects the 8-bit sRGB format, and PIXMAN_x2r10g10b10 and its variations.

~Maarten
Maarten Lankhorst Dec. 6, 2018, 2:46 p.m.
Op 06-12-2018 om 12:03 schreef Maarten Lankhorst:
> Op 05-12-2018 om 11:24 schreef Basile Clement:
>> Hi,
>>
>> On 12/5/18 9:08 AM, Pekka Paalanen wrote:
>>> On Tue, 4 Dec 2018 17:36:18 +0100
>>> Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>>>> Series looks sane, 1/4 is cleaner than my version. I would change Bpp
>>>> to cpp, or multiply by 8, since Bpp usually means bits per pixel.
>>> And cpp means channels-per-pixel?
>>>
>>> B usually means bytes, b often means bits. But not always, so you can
>>> never assume.
>>>
>>> I'd just write out bits_per_pixel or bytes_per_pixel, just like
>>> pitch_bytes or stride_pixels or stride_uint32s or whatever makes it
>>> totally obvious.
>> This was indeed Bpp as in Bytes-per-pixel; I reused the variable name from pixman-general.c.  Happy to change it to the more explicit bytes_per_pixel.
>>
>> On 12/4/18 5:36 PM, Maarten Lankhorst wrote:
>>> Do you happen to have a patch to fix pixman-bits-image.c falling back to 8-bit paths as well?
>> Are you referring to the fact that `get_scanline_float` gets data through `get_scanline_32` first?  If so, I have not -- I understand this is only an issue with floating point formats, which were added after I wrote the patches. 
> This is also an issue with any format with >8 bpp, see PIXMAN_FORMAT_IS_WIDE(). This also affects the 8-bit sRGB format, and PIXMAN_x2r10g10b10 and its variations.
>
> ~Maarten
>
> _______________________________________________
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman

Looks easier to add than expected. I've mostly completed my commit that does the same for pixman-bits-image, but might conflict with this series..
Adam Jackson Dec. 11, 2018, 8:52 p.m.
On Wed, 2018-12-05 at 10:08 +0200, Pekka Paalanen wrote:
> On Tue, 4 Dec 2018 17:36:18 +0100
> Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> > Series looks sane, 1/4 is cleaner than my version. I would change Bpp
> > to cpp, or multiply by 8, since Bpp usually means bits per pixel.
> 
> And cpp means channels-per-pixel?

"chars per pixel" in this context. Not always a great unit.

- ajax