Hi, I'm back :)

I tried to implement the proposed changes :

* the gradient walker now deals with argb_t (floats) and not uint32_t
* all gradients are WIDE because of the above change
* WIDE formats (using argb_t) can be dithered on write_back call
depending on the "dither" property of the image (set with
pixman_image_set_dither)
* The dithering is still random. I could try to implement other ones if
it's really needed for the patch to be accepted


Thanks for any feedback, please do tell if there are any more changes
needed for the patch to be accepted.

-- 
Marc
From 2cd0d0a12c9a68bdedde10ae4b10c335f8961501 Mon Sep 17 00:00:00 2001
From: Marc Jeanmougin <[email protected]>
Date: Sun, 8 Apr 2018 22:25:27 +0200
Subject: [PATCH] Adds a gradient dithering function to pixman.

This dithering is random, resulting is no artifacts and very smooth result,
and uses a very fast prng (xorshift algorithm).

Also adds a pixman_image_set_dithering to toggle the feature,
and a pixman_dither_t type for possible future other implementations.

Signed-off-by: Marc Jeanmougin <[email protected]>
---
 .gitignore                       |  2 ++
 pixman/pixman-bits-image.c       | 47 ++++++++++++++++++++++++++++++++++++++++
 pixman/pixman-conical-gradient.c |  7 ++----
 pixman/pixman-general.c          |  3 ++-
 pixman/pixman-gradient-walker.c  | 24 +++++---------------
 pixman/pixman-image.c            | 12 ++++++++++
 pixman/pixman-linear-gradient.c  | 11 ++++------
 pixman/pixman-private.h          |  7 +++++-
 pixman/pixman-radial-gradient.c  | 18 +++++++--------
 pixman/pixman-utils.c            | 10 +++++++++
 pixman/pixman.h                  |  9 ++++++++
 11 files changed, 108 insertions(+), 42 deletions(-)

diff --git a/.gitignore b/.gitignore
index a245b69..02ad685 100644
--- a/.gitignore
+++ b/.gitignore
@@ -53,3 +53,5 @@ test/trap-crasher
 *.ilk
 *.obj
 *.exe
+*.log
+*.trs
diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index dcdcc69..bdedb99 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -752,6 +752,51 @@ _pixman_bits_image_src_iter_init (pixman_image_t *image, pixman_iter_t *iter)
     iter->get_scanline = _pixman_iter_get_scanline_noop;
 }
 
+
+static float 
+dither_channel (float f, int n_bits, uint32_t *state)
+{
+    uint32_t u, i_rem;
+    float f_rem;
+    if (f>1.0) f = 1.0;
+    if (f<0) f = 0.;
+
+    u = f * (1 << n_bits);
+    u -= (u >> n_bits);
+    f_rem = ( (float)(1<<n_bits) * f - (float)u);
+    i_rem = f_rem * 4294967296.;
+
+    *state = pixman_prng_get(*state);
+    if (*state < i_rem)
+        return MIN (1.0, f + (1.0 / ((float)(1<<n_bits))));
+    return f;
+}
+
+
+void
+dither(bits_image_t *image, argb_t *buf, int width)
+{
+    int i;
+    int a_size, r_size, g_size, b_size;
+    uint32_t *state = &(image->common.state);
+    pixman_format_code_t format = image->format;
+
+    if(image->common.dither == PIXMAN_DITHER_NONE)
+        return;
+
+    a_size = PIXMAN_FORMAT_A (format);
+    r_size = PIXMAN_FORMAT_R (format);
+    g_size = PIXMAN_FORMAT_G (format);
+    b_size = PIXMAN_FORMAT_B (format);
+
+    for (i = 0; i < width; ++i) {
+        buf[i].a = dither_channel(buf[i].a, a_size, state);
+        buf[i].r = dither_channel(buf[i].r, r_size, state);
+        buf[i].g = dither_channel(buf[i].g, g_size, state);
+        buf[i].b = dither_channel(buf[i].b, b_size, state);
+    }
+}
+
 static uint32_t *
 dest_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 {
@@ -856,6 +901,8 @@ dest_write_back_wide (pixman_iter_t *iter)
     int             width  = iter->width;
     const uint32_t *buffer = iter->buffer;
 
+    dither (image, (argb_t*)buffer, width);
+
     image->store_scanline_float (image, x, y, width, buffer);
 
     if (image->common.alpha_map)
diff --git a/pixman/pixman-conical-gradient.c b/pixman/pixman-conical-gradient.c
index 8bb46ae..35a64f4 100644
--- a/pixman/pixman-conical-gradient.c
+++ b/pixman/pixman-conical-gradient.c
@@ -57,11 +57,11 @@ conical_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
     int x = iter->x;
     int y = iter->y;
     int width = iter->width;
-    uint32_t *buffer = iter->buffer;
+    argb_t *buffer = (argb_t*)(iter->buffer);
 
     gradient_t *gradient = (gradient_t *)image;
     conical_gradient_t *conical = (conical_gradient_t *)image;
-    uint32_t       *end = buffer + width;
+    argb_t       *end = buffer + width;
     pixman_gradient_walker_t walker;
     pixman_bool_t affine = TRUE;
     double cx = 1.;
@@ -165,9 +165,6 @@ conical_get_scanline_wide (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 buffer;
 }
 
diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
index 6141cb0..809df2c 100644
--- a/pixman/pixman-general.c
+++ b/pixman/pixman-general.c
@@ -141,7 +141,8 @@ general_composite_rect  (pixman_implementation_t *imp,
     if ((src_image->common.flags & FAST_PATH_NARROW_FORMAT)		     &&
 	(!mask_image || mask_image->common.flags & FAST_PATH_NARROW_FORMAT)  &&
 	(dest_image->common.flags & FAST_PATH_NARROW_FORMAT)		     &&
-	!(operator_needs_division (op)))
+	(!(operator_needs_division (op)))  &&   (!(src_image->type == LINEAR))&&
+	(!(src_image->type == RADIAL))    &&   (!(src_image->type == CONICAL)))
     {
 	width_flag = ITER_NARROW;
 	Bpp = 4;
diff --git a/pixman/pixman-gradient-walker.c b/pixman/pixman-gradient-walker.c
index 822f8e6..1811683 100644
--- a/pixman/pixman-gradient-walker.c
+++ b/pixman/pixman-gradient-walker.c
@@ -169,13 +169,11 @@ gradient_walker_reset (pixman_gradient_walker_t *walker,
     walker->need_reset = FALSE;
 }
 
-uint32_t
+argb_t
 _pixman_gradient_walker_pixel (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 v;
     float y;
 
     if (walker->need_reset || x < walker->left_x || x >= walker->right_x)
@@ -183,20 +181,10 @@ _pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
 
     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);
-
-    a8 = a + 0.5f;
-    r8 = r + 0.5f;
-    g8 = g + 0.5f;
-    b8 = b + 0.5f;
-
-    v = ((a8 << 24) & 0xff000000) |
-        ((r8 << 16) & 0x00ff0000) |
-        ((g8 <<  8) & 0x0000ff00) |
-        ((b8 >>  0) & 0x000000ff);
+    v.a = (walker->a_s * y + walker->a_b) / 256.0f;
+    v.r = v.a * (walker->r_s * y + walker->r_b);
+    v.g = v.a * (walker->g_s * y + walker->g_b);
+    v.b = v.a * (walker->b_s * y + walker->b_b);
 
     return v;
 }
diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 681864e..ddec4ed 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -28,6 +28,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <assert.h>
+#include <time.h>
 
 #include "pixman-private.h"
 
@@ -118,6 +119,8 @@ _pixman_image_init (pixman_image_t *image)
     common->clip_sources = FALSE;
     common->transform = NULL;
     common->repeat = PIXMAN_REPEAT_NONE;
+    common->dither = PIXMAN_DITHER_NONE;
+    common->state = rand();
     common->filter = PIXMAN_FILTER_NEAREST;
     common->filter_params = NULL;
     common->n_filter_params = 0;
@@ -684,6 +687,15 @@ pixman_image_set_repeat (pixman_image_t *image,
     image_property_changed (image);
 }
 
+PIXMAN_EXPORT void
+pixman_image_set_dither (pixman_image_t *      image,
+		         pixman_dither_t       dither)
+{
+    image_common_t *common = (image_common_t *)image;
+    common->dither = dither;
+    image_property_changed (image);
+}
+
 PIXMAN_EXPORT pixman_bool_t
 pixman_image_set_filter (pixman_image_t *      image,
                          pixman_filter_t       filter,
diff --git a/pixman/pixman-linear-gradient.c b/pixman/pixman-linear-gradient.c
index 40c8c9f..89e3318 100644
--- a/pixman/pixman-linear-gradient.c
+++ b/pixman/pixman-linear-gradient.c
@@ -96,14 +96,14 @@ linear_get_scanline_narrow (pixman_iter_t  *iter,
     int             x      = iter->x;
     int             y      = iter->y;
     int             width  = iter->width;
-    uint32_t *      buffer = iter->buffer;
+    argb_t *        buffer = (argb_t*)(iter->buffer);
 
     pixman_vector_t v, unit;
     pixman_fixed_32_32_t l;
     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;
+    argb_t *end = buffer + width;
     pixman_gradient_walker_t walker;
 
     _pixman_gradient_walker_init (&walker, gradient, image->common.repeat);
@@ -160,7 +160,7 @@ linear_get_scanline_narrow (pixman_iter_t  *iter,
 
 	if (((pixman_fixed_32_32_t )(inc * width)) == 0)
 	{
-	    register uint32_t color;
+	    register argb_t color;
 
 	    color = _pixman_gradient_walker_pixel (&walker, t);
 	    while (buffer < end)
@@ -227,16 +227,13 @@ linear_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
 {
     uint32_t *buffer = linear_get_scanline_narrow (iter, NULL);
 
-    pixman_expand_to_float (
-	(argb_t *)buffer, buffer, PIXMAN_a8r8g8b8, iter->width);
-
     return buffer;
 }
 
 void
 _pixman_linear_gradient_iter_init (pixman_image_t *image, pixman_iter_t  *iter)
 {
-    if (linear_gradient_is_horizontal (
+    if (image->common.dither == PIXMAN_DITHER_NONE && linear_gradient_is_horizontal (
 	    iter->image, iter->x, iter->y, iter->width, iter->height))
     {
 	if (iter->iter_flags & ITER_NARROW)
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 73a5414..70c91c3 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -104,6 +104,8 @@ struct image_common
     pixman_transform_t *        transform;
     pixman_repeat_t             repeat;
     pixman_filter_t             filter;
+    pixman_dither_t             dither;
+    uint32_t                    state;
     pixman_fixed_t *            filter_params;
     int                         n_filter_params;
     bits_image_t *              alpha_map;
@@ -297,6 +299,9 @@ _pixman_conical_gradient_iter_init (pixman_image_t *image, pixman_iter_t *iter);
 void
 _pixman_image_init (pixman_image_t *image);
 
+uint32_t
+pixman_prng_get(uint32_t state);
+
 pixman_bool_t
 _pixman_bits_image_init (pixman_image_t *     image,
                          pixman_format_code_t format,
@@ -363,7 +368,7 @@ void
 _pixman_gradient_walker_reset (pixman_gradient_walker_t *walker,
                                pixman_fixed_48_16_t      pos);
 
-uint32_t
+argb_t
 _pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
                                pixman_fixed_48_16_t      x);
 
diff --git a/pixman/pixman-radial-gradient.c b/pixman/pixman-radial-gradient.c
index 6a21796..4d7dd5e 100644
--- a/pixman/pixman-radial-gradient.c
+++ b/pixman/pixman-radial-gradient.c
@@ -66,7 +66,7 @@ fdot (double x1,
     return x1 * x2 + y1 * y2 + z1 * z2;
 }
 
-static uint32_t
+static argb_t
 radial_compute_color (double                    a,
 		      double                    b,
 		      double                    c,
@@ -93,13 +93,14 @@ radial_compute_color (double                    a,
      *  - the above problems are worse if a is small (as inva becomes bigger)
      */
     double discr;
+    argb_t _0; _0.a=0; _0.r=0; _0.g=0; _0.b=0;
 
     if (a == 0)
     {
 	double t;
 
 	if (b == 0)
-	    return 0;
+	    return _0;
 
 	t = pixman_fixed_1 / 2 * c / b;
 	if (repeat == PIXMAN_REPEAT_NONE)
@@ -113,7 +114,6 @@ radial_compute_color (double                    a,
 		return _pixman_gradient_walker_pixel (walker, t);
 	}
 
-	return 0;
     }
 
     discr = fdot (b, a, 0, b, -c, 0);
@@ -152,7 +152,7 @@ radial_compute_color (double                    a,
 	}
     }
 
-    return 0;
+    return _0;
 }
 
 static uint32_t *
@@ -243,13 +243,14 @@ radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
     int x = iter->x;
     int y = iter->y;
     int width = iter->width;
-    uint32_t *buffer = iter->buffer;
+    argb_t *buffer = (argb_t*)(iter->buffer);
 
     gradient_t *gradient = (gradient_t *)image;
     radial_gradient_t *radial = (radial_gradient_t *)image;
-    uint32_t *end = buffer + width;
+    argb_t *end = buffer + width;
     pixman_gradient_walker_t walker;
     pixman_vector_t v, unit;
+    argb_t _0; _0.a=0; _0.r=0; _0.g=0; _0.b=0;
 
     /* reference point is the center of the pixel */
     v.vector[0] = pixman_int_to_fixed (x) + pixman_fixed_1 / 2;
@@ -384,7 +385,7 @@ radial_get_scanline_narrow (pixman_iter_t *iter, const uint32_t *mask)
 		}
 		else
 		{
-		    *buffer = 0;
+		    *buffer = _0;
 		}
 	    }
 
@@ -405,9 +406,6 @@ radial_get_scanline_wide (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 buffer;
 }
 
diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index 4a3a835..7bf46df 100644
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -328,3 +328,13 @@ _pixman_log_error (const char *function, const char *message)
 	n_messages++;
     }
 }
+
+uint32_t
+pixman_prng_get(uint32_t x)
+{
+    if (!x) return 1;
+    x ^= x << 13;
+    x ^= x >> 17;
+    x ^= x << 15;
+    return x;
+}
diff --git a/pixman/pixman.h b/pixman/pixman.h
index 509ba5e..8701d10 100644
--- a/pixman/pixman.h
+++ b/pixman/pixman.h
@@ -316,6 +316,13 @@ typedef enum
     PIXMAN_FILTER_SEPARABLE_CONVOLUTION
 } pixman_filter_t;
 
+typedef enum
+{
+    PIXMAN_DITHER_NONE,
+    PIXMAN_DITHER_BEST
+} pixman_dither_t;
+
+
 typedef enum
 {
     PIXMAN_OP_CLEAR			= 0x00,
@@ -807,6 +814,8 @@ pixman_bool_t   pixman_image_set_transform           (pixman_image_t
 						      const pixman_transform_t     *transform);
 void            pixman_image_set_repeat              (pixman_image_t               *image,
 						      pixman_repeat_t               repeat);
+void            pixman_image_set_dither              (pixman_image_t               *image,
+		                                      pixman_dither_t              dither);
 pixman_bool_t   pixman_image_set_filter              (pixman_image_t               *image,
 						      pixman_filter_t               filter,
 						      const pixman_fixed_t         *filter_params,
-- 
2.16.3

_______________________________________________
Pixman mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/pixman

Reply via email to