Re: [Pixman] [patch] Gradient dithering into pixman

[Bill Spitzak]

I don't understand why you would want to disable it when writing .png
files. There will be banding in the .png file, which I would think is worse
than the increased size. Also kind of fools the user if they did not look
at the .png file and only at InkScape's display.


On Mon, Mar 26, 2018 at 5:04 PM, Søren Sandmann 
wrote:

> Hi,
>
> A long time ago I wrote this:
>
> https://lists.freedesktop.org/archives/pixman/2012-July/002175.html
>
> about how dithering could be added to pixman. The basic idea is that
> "dithering" is a property of the destination image, not of the gradient.  I
> still think this is the right way to do it.
>
>
> Søren
>
> On Mon, Mar 26, 2018 at 6:37 PM, Marc Jeanmougin 
> wrote:
>
>> 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
>>
>> ___
>> Pixman mailing list
>> Pixman@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/pixman
>>
>>
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

Re: [Pixman] [patch] Gradient dithering into pixman

[Søren Sandmann]

Hi,

A long time ago I wrote this:

https://lists.freedesktop.org/archives/pixman/2012-July/002175.html

about how dithering could be added to pixman. The basic idea is that
"dithering" is a property of the destination image, not of the gradient.  I
still think this is the right way to do it.


Søren

On Mon, Mar 26, 2018 at 6:37 PM, Marc Jeanmougin  wrote:

> 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
>
> ___
> Pixman mailing list
> Pixman@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman
>
>
___
Pixman mailing list
Pixman@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pixman

[Pixman] [patch] Gradient dithering into pixman

[Marc Jeanmougin]

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 
 #endif
 #include "pixman-private.h"
+#include 
+#include 
+#include 
 
 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, );
+r8 = _pixman_dither(walker, );
+g8 = _pixman_dither(walker, );
+b8 = _pixman_dither(walker, );
 
 v = ((a8 << 24) & 0xff00) |
 ((r8 << 16) & 0x00ff) |
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 (, t);
 	while (buffer < end)
-		*buffer++ = color;
+		*buffer++ = _pixman_gradient_walker_pixel (, 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