Hi, I'm Marc, I'm an Inkscape contributor, and I would like to improve pixman by providing a patch to allow gradient dithering, to eliminate all gradient banding. This "banding" problem is hugely visible in many Inkscape files, and I hope you could help me putting it into pixman, then in exposing the functionality into cairo.
I tried to very closely follow the existing code style, and I came up with the proof of concept (attached Proof_of_concept.patch ), which applies to pixman/master to enable the feature directly. Since Inkscape also uses pixman to render files when exporting to png, and that kind of dithering affects very significantly the file size (PNG compression algorithm and randomness don't mix well) we also need a switch to enable/disable it "at will". I tried to understand how it could be done by looking at pixman_image_set_* functions and finally came up with the other patch attached 0001-Adds-a-gradient-dithering-function-to-pixman.patch (which cannot directly be tested, since it would require changes in cairo to call pixman_image_set_dithering, but changing "gradient->dithering = PIXMAN_DITHERING_NONE;" to _BEST in pixman-image.c works) Thanks for any help in merging this, or any pointer to how to improve it, -- Marc
diff --git a/pixman/pixman-gradient-walker.c b/pixman/pixman-gradient-walker.c
index 822f8e6..c0fbebb 100644
--- a/pixman/pixman-gradient-walker.c
+++ b/pixman/pixman-gradient-walker.c
@@ -27,6 +27,9 @@
#include <config.h>
#endif
#include "pixman-private.h"
+#include <math.h>
+#include <time.h>
+#include <stdlib.h>
void
_pixman_gradient_walker_init (pixman_gradient_walker_t *walker,
@@ -48,6 +51,8 @@ _pixman_gradient_walker_init (pixman_gradient_walker_t *walker,
walker->repeat = repeat;
walker->need_reset = TRUE;
+ walker->dithering = PIXMAN_DITHERING_BEST;
+ walker->prng_state = rand();
}
static void
@@ -169,6 +174,42 @@ gradient_walker_reset (pixman_gradient_walker_t *walker,
walker->need_reset = FALSE;
}
+static inline uint8_t
+_pixman_dither (pixman_gradient_walker_t *walker, float *in)
+{
+ float f_rem = (*in-(float)((int)(*in)));
+ uint32_t i_rem = f_rem * 4294967296.;
+
+ switch (walker->dithering)
+ {
+ case PIXMAN_DITHERING_NONE:
+ /* round */
+ return *in + 0.5f;
+ case PIXMAN_DITHERING_GOOD:
+ /* only dither between 1/4 and 3/4 */
+ if (f_rem < 0.25 || f_rem >= 0.75)
+ return *in + 0.5f;
+ else
+ i_rem += (i_rem - (1<<31));
+ break;
+ /*case PIXMAN_DITHERING_EDGES:
+ if (f_rem < 0.48 || f_rem >= 0.52)
+ return *in + 0.5f;
+ else
+ i_rem += (24*(((int)i_rem) - (1<<31)));
+ break;*/
+ case PIXMAN_DITHERING_BEST:
+ break;
+ }
+ /* we want a8 = a with probability (1-(a-int(a))) and
+ * a8=a+1 with probability a-int(a)
+ */
+ if ((walker->prng_state = pixman_prng_get(walker->prng_state)) < i_rem)
+ return *in +1;
+ else
+ return *in;
+}
+
uint32_t
_pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
pixman_fixed_48_16_t x)
@@ -188,10 +229,10 @@ _pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
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;
+ a8 = _pixman_dither(walker, &a);
+ r8 = _pixman_dither(walker, &r);
+ g8 = _pixman_dither(walker, &g);
+ b8 = _pixman_dither(walker, &b);
v = ((a8 << 24) & 0xff000000) |
((r8 << 16) & 0x00ff0000) |
diff --git a/pixman/pixman-linear-gradient.c b/pixman/pixman-linear-gradient.c
index 40c8c9f..5ad6f45 100644
--- a/pixman/pixman-linear-gradient.c
+++ b/pixman/pixman-linear-gradient.c
@@ -160,11 +160,8 @@ 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;
+ *buffer++ = _pixman_gradient_walker_pixel (&walker, t);
}
else
{
@@ -236,6 +233,7 @@ linear_get_scanline_wide (pixman_iter_t *iter, const uint32_t *mask)
void
_pixman_linear_gradient_iter_init (pixman_image_t *image, pixman_iter_t *iter)
{
+ /*
if (linear_gradient_is_horizontal (
iter->image, iter->x, iter->y, iter->width, iter->height))
{
@@ -246,7 +244,7 @@ _pixman_linear_gradient_iter_init (pixman_image_t *image, pixman_iter_t *iter)
iter->get_scanline = _pixman_iter_get_scanline_noop;
}
- else
+ else*/
{
if (iter->iter_flags & ITER_NARROW)
iter->get_scanline = linear_get_scanline_narrow;
diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h
index 73a5414..a3e026a 100644
--- a/pixman/pixman-private.h
+++ b/pixman/pixman-private.h
@@ -352,6 +352,9 @@ typedef struct
pixman_repeat_t repeat;
pixman_bool_t need_reset;
+
+ pixman_dithering_t dithering;
+ uint32_t prng_state;
} pixman_gradient_walker_t;
void
@@ -367,6 +370,8 @@ uint32_t
_pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker,
pixman_fixed_48_16_t x);
+uint32_t
+pixman_prng_get(uint32_t state);
/*
* Edges
*/
@@ -1121,6 +1126,12 @@ extern int timer_defined;
void pixman_timer_register (pixman_timer_t *timer);
+struct pixman_prng_state
+{
+ int n;
+}
+
+
#define TIMER_BEGIN(tname) \
{ \
static pixman_timer_t timer ## tname; \
diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c
index 4a3a835..aaeb50a 100644
--- a/pixman/pixman-utils.c
+++ b/pixman/pixman-utils.c
@@ -328,3 +328,12 @@ _pixman_log_error (const char *function, const char *message)
n_messages++;
}
}
+
+uint32_t
+pixman_prng_get(uint32_t x)
+{
+ x ^= x << 13;
+ x ^= x >> 17;
+ x ^= x << 15;
+ return x;
+}
diff --git a/pixman/pixman.h b/pixman/pixman.h
index 509ba5e..12612e8 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_DITHERING_NONE,
+ PIXMAN_DITHERING_GOOD,
+ PIXMAN_DITHERING_BEST
+} pixman_dithering_t;
+
typedef enum
{
PIXMAN_OP_CLEAR = 0x00,
From f3405510ed7070cc2d0e903b3561ca287efbb41e Mon Sep 17 00:00:00 2001 From: Marc Jeanmougin <[email protected]> Date: Tue, 27 Mar 2018 00:05:37 +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_dithering_t type for possible future other implementations. Signed-off-by: Marc Jeanmougin <[email protected]> --- pixman/pixman-gradient-walker.c | 36 ++++++++++++++++++++++++++++++++---- pixman/pixman-image.c | 15 +++++++++++++++ pixman/pixman-linear-gradient.c | 8 +++----- pixman/pixman-private.h | 12 ++++++++++++ pixman/pixman-utils.c | 9 +++++++++ pixman/pixman.h | 9 +++++++++ 6 files changed, 80 insertions(+), 9 deletions(-) diff --git a/pixman/pixman-gradient-walker.c b/pixman/pixman-gradient-walker.c index 822f8e6..dbd7a92 100644 --- a/pixman/pixman-gradient-walker.c +++ b/pixman/pixman-gradient-walker.c @@ -27,6 +27,9 @@ #include <config.h> #endif #include "pixman-private.h" +#include <math.h> +#include <time.h> +#include <stdlib.h> void _pixman_gradient_walker_init (pixman_gradient_walker_t *walker, @@ -48,6 +51,8 @@ _pixman_gradient_walker_init (pixman_gradient_walker_t *walker, walker->repeat = repeat; walker->need_reset = TRUE; + walker->dithering = gradient->dithering; + walker->prng_state = rand(); } static void @@ -169,6 +174,29 @@ gradient_walker_reset (pixman_gradient_walker_t *walker, walker->need_reset = FALSE; } +static inline uint8_t +_pixman_dither (pixman_gradient_walker_t *walker, float *in) +{ + float f_rem = (*in-(float)((int)(*in))); + uint32_t i_rem = f_rem * 4294967296.; + + switch (walker->dithering) + { + case PIXMAN_DITHERING_BEST: + /* we want a8 = a with probability (1-(a-int(a))) and + * a8=a+1 with probability a-int(a) + */ + if ((walker->prng_state = pixman_prng_get(walker->prng_state)) < i_rem) + return *in +1; + else + return *in; + case PIXMAN_DITHERING_NONE: + default: + /* round */ + return *in + 0.5f; + } +} + uint32_t _pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker, pixman_fixed_48_16_t x) @@ -188,10 +216,10 @@ _pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker, 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; + a8 = _pixman_dither(walker, &a); + r8 = _pixman_dither(walker, &r); + g8 = _pixman_dither(walker, &g); + b8 = _pixman_dither(walker, &b); v = ((a8 << 24) & 0xff000000) | ((r8 << 16) & 0x00ff0000) | diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c index 681864e..75f168e 100644 --- a/pixman/pixman-image.c +++ b/pixman/pixman-image.c @@ -102,6 +102,7 @@ _pixman_init_gradient (gradient_t * gradient, gradient->n_stops = n_stops; gradient->common.property_changed = gradient_property_changed; + gradient->dithering = PIXMAN_DITHERING_NONE; return TRUE; } @@ -684,6 +685,20 @@ pixman_image_set_repeat (pixman_image_t *image, image_property_changed (image); } +PIXMAN_EXPORT void +pixman_image_set_dithering (pixman_image_t * image, + pixman_dithering_t dither) +{ + gradient_t *gradient = &image->gradient; + + if (gradient->dithering == dither) + return; + + gradient->dithering = 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..cc4eb76 100644 --- a/pixman/pixman-linear-gradient.c +++ b/pixman/pixman-linear-gradient.c @@ -160,11 +160,8 @@ 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; + *buffer++ = _pixman_gradient_walker_pixel (&walker, t); } else { @@ -237,7 +234,8 @@ void _pixman_linear_gradient_iter_init (pixman_image_t *image, pixman_iter_t *iter) { if (linear_gradient_is_horizontal ( - iter->image, iter->x, iter->y, iter->width, iter->height)) + iter->image, iter->x, iter->y, iter->width, iter->height) + && (image->gradient.dithering == PIXMAN_DITHERING_NONE)) { if (iter->iter_flags & ITER_NARROW) linear_get_scanline_narrow (iter, NULL); diff --git a/pixman/pixman-private.h b/pixman/pixman-private.h index 73a5414..8546be2 100644 --- a/pixman/pixman-private.h +++ b/pixman/pixman-private.h @@ -133,6 +133,7 @@ struct gradient image_common_t common; int n_stops; pixman_gradient_stop_t *stops; + pixman_dithering_t dithering; }; struct linear_gradient @@ -352,6 +353,9 @@ typedef struct pixman_repeat_t repeat; pixman_bool_t need_reset; + + pixman_dithering_t dithering; + uint32_t prng_state; } pixman_gradient_walker_t; void @@ -367,6 +371,8 @@ uint32_t _pixman_gradient_walker_pixel (pixman_gradient_walker_t *walker, pixman_fixed_48_16_t x); +uint32_t +pixman_prng_get(uint32_t state); /* * Edges */ @@ -1121,6 +1127,12 @@ extern int timer_defined; void pixman_timer_register (pixman_timer_t *timer); +struct pixman_prng_state +{ + int n; +} + + #define TIMER_BEGIN(tname) \ { \ static pixman_timer_t timer ## tname; \ diff --git a/pixman/pixman-utils.c b/pixman/pixman-utils.c index 4a3a835..aaeb50a 100644 --- a/pixman/pixman-utils.c +++ b/pixman/pixman-utils.c @@ -328,3 +328,12 @@ _pixman_log_error (const char *function, const char *message) n_messages++; } } + +uint32_t +pixman_prng_get(uint32_t x) +{ + x ^= x << 13; + x ^= x >> 17; + x ^= x << 15; + return x; +} diff --git a/pixman/pixman.h b/pixman/pixman.h index 509ba5e..5ac0db6 100644 --- a/pixman/pixman.h +++ b/pixman/pixman.h @@ -316,6 +316,12 @@ typedef enum PIXMAN_FILTER_SEPARABLE_CONVOLUTION } pixman_filter_t; +typedef enum +{ + PIXMAN_DITHERING_NONE, + PIXMAN_DITHERING_BEST +} pixman_dithering_t; + typedef enum { PIXMAN_OP_CLEAR = 0x00, @@ -811,6 +817,9 @@ pixman_bool_t pixman_image_set_filter (pixman_image_t pixman_filter_t filter, const pixman_fixed_t *filter_params, int n_filter_params); +void pixman_image_set_dithering (pixman_image_t *image, + pixman_dithering_t dither +); void pixman_image_set_source_clipping (pixman_image_t *image, pixman_bool_t source_clipping); void pixman_image_set_alpha_map (pixman_image_t *image, -- 2.16.2
0xF882B07A.asc
Description: application/pgp-keys
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Pixman mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/pixman
