Re: [Pixman] Dithering patches, v2

[Basile Clement]

There is less calculation, but the noise matrix takes up cache space
(about half a modern CPU's L1) and hence increases memory pressure.  The
actual performance relationship is probably hardware dependent, but on
my laptop doing a quick check with patched lowlevel-blt-bench:

 - Bayer is ~8% faster than blue noise
 - Bayer is only ~5% faster if I use `dither_blue_noise_64x64[((y &
0x3f) << 6) | (x & 0x3f)]` in the index computation, which I would have
expected GCC to do automatically but oh well.
 - Both are about the same speed with uint16_t array for the blue noise
(halves memory consumption).

I will make those two changes (they are clearly desirable) and re-post
the blue noise patch.  Since this is in a write-back loop which should
be memory-bound in general, I think it makes sense to keep Bayer as FAST
dithering -- but I'm not opposed to removing it.

- Basile

On 4/22/19 8:12 PM, Bill Spitzak wrote:
> Is the "blue noise" version actually slower than the Bayer? Seems to
> be doing a bit less calculation but it reading the array of weights.
> The removal of the need for the pattern offset seems like a win.
>
> On Mon, Apr 22, 2019 at 9:27 AM Matt Turner  > wrote:
>
> On Fri, Apr 19, 2019 at 4:52 PM Bryce Harrington
> mailto:br...@bryceharrington.org>> wrote:
> > Inkscape would love to see Basile's dithering patches included.  Our
> > testing shows that they make a huge quality difference for our
> users;
> > this solves a critical need.
> >
> > Mc and I have done some preliminary investigation into how to
> plumb this
> > into Cairo, and would love to hear your review of Basile's
> approach to
> > the problem.
>
> I don't feel like I'm experienced enough with that side of pixman to
> offer meaningful comments. I've Cc'd Søren in the hopes that he
> remains interested enough in the project to review the patches that
> Basile says implement the approach Søren described.
> ___
> 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] Dithering patches, v2

[Basile Clement]

I'm not sure how, but I badly broke the formatting on that email -- my
apologies.  Below is the previous email formatted as intended.

On 4/10/19 1:04 AM, Bill Spitzak wrote:

> How difficult is it to just make the gradients use dithering, so
> they produce 8-bit (or whatever) results directly but with the dithering
> pattern in them?

We don't have the destination bpp when we compute the gradients.  Søren
Sandmann explained the various issues with this approach earlier
https://lists.freedesktop.org/archives/pixman/2018-April/004711.html

Note that it is always possible to add fast paths for gradient+dithering
if performance is an issue.

> What other operations would need dithering (a large blur comes to
> mind, also zooming way in on an image in bilinear)?

Any type of filter (eg blur) or zoom could benefit from dithering, as
well as any high bpp to low bpp compositing (compositing to r5g6b5 was
mentioned in one of the previous discussions).  Although for that one
proper gamma correction may be required.

> I think the blue noise large pattern is a great idea.

All credits go to Wikipedia :)

By the way re-reading the email I linked reminded me --

Søren, I don't know if you are still reading this mailing list, but you
said in that email one year ago:

> I could elaborate on that subject and have done so several times in
> the past, but in my experience those long emails about pixman
> internals were mostly just a waste of time. I don't recall a single 
> one of them ever resulting in a useful patch.

Your emails (especially the two I linked above, but also a couple
others) were instrumental in getting a good enough grasp on the pixman
internals for writing the floating point gradient computation and
dithering patches, so thank you for taking the time to share your
knowledge and insights.  Hopefully mine will be (are?) those
long-awaited useful patches.

- Basile


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

Re: [Pixman] Dithering patches, v2

[Basile Clement]

On 4/10/19 1:04 AM, Bill Spitzak wrote:
> How difficult is it to just make the gradients use dithering, so > they 
> produce 8-bit (or whatever) results directly but with the >
dithering pattern in them? >

We don't have the destination bpp when we compute the gradients.  Søren
Sandmann explained the various issues with this approach earlier
https://lists.freedesktop.org/archives/pixman/2018-April/004711.html

Note that it is always possible to add fast paths for gradient+dithering
if performance is an issue.

> What other operations would need dithering (a large blur comes to > mind, 
> also zooming way in on an image in bilinear)?

Any type of filter (eg blur) or zoom could benefit from dithering, as
well as any high bpp to low bpp compositing (compositing to r5g6b5 was
mentioned in one of the previous discussions).  Although for that one
proper gamma correction may be required.

> I think the blue noise large pattern is a great idea.

All credits go to Wikipedia :)

By the way re-reading the email I linked reminded me --

Søren, I don't know if you are still reading this mailing list, but you
said in that email one year ago:

> I could elaborate on that subject and have done so several times in > the 
> past, but in my experience those long emails about pixman >
internals were mostly just a waste of time. I don't recall a single >
one of them ever resulting in a useful patch.
Your emails (especially the two I linked above, but also a couple
others) were instrumental in getting a good enough grasp on the pixman
internals for writing the floating point gradient computation and
dithering patches, so thank you for taking the time to share your
knowledge and insights.  Hopefully mine will be (are?) those
long-awaited useful patches.

- Basile


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

Re: [Pixman] Dithering patches, v2

[Basile Clement]

I forgot to mention two things:

 - Enabling dithering makes processing much slower, since it forces the
wide pipeline to be used (and some optimisations are disabled).  This
could be solved through the use of fast paths where needed (e.g. when
blitting images from an equal or lower bits-per-pixels format, or with
fused gradient+dithering paths).  In any case, the design of the
implementation allows setting the `dither` property of a destination
image in between calls to `pixman_image_composite` where dithering is
required, allowing users to pay the cost of the slower wide pipeline
only for those calls where dithering is actually required.

 - The dithering is not gamma corrected, which doesn't matter much for
8bpp formats -- but makes a noticeable difference for lower bpp formats,
producing images which are way too luminous (this is particularily
visible in the dithering demos when using 2bpp and 1bpp formats).  Since
the main use case for now is a8r8g8b8 gradient export in Inkscape (and
since the series is starting to be a bit large), I did not include gamma
correction in this series.  If needed, it can be added in subsequent
patches, and should probably be configurable since the proper curve to
use depend on the characteristic of the target device, which pixman may
not know about.

- Basile

On 4/9/19 11:29 PM, basile-pix...@clement.pm wrote:
> I am resubmitting for review and inclusion my series of patches on
> dithering from October, which I ended up not having the time to finish
> then.  There are only a couple changes to make the patches compatible
> with the recently added floating point formats, and I have also added a
> dithering matrix based on blue noise which provides more pleasant
> results than the Bayer matrix from the first series.
>
> Maarten, I would appreciate if you can take a second look (although as
> noted above not much has changed since your first review -- I don't
> think you had comments on the dithering part?).
>
> - Basile
>
>
> ___
> 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 2/2] pixman: Use maximum precision for pixman-bits-image, v2.

[Basile Clement]

On 1/8/19 11:42 AM, Maarten Lankhorst wrote:

> pixman-bits-image's wide helpers first obtains the 8-bits image,
> then converts it to float. This destroys all the precision that
> the wide path was offering.
>
> Fix this by making get_pixel() take a pointer instead of returning
> a value. Floating point will fill in a argb_t, while the 8-bits path
> will fill a 32-bits ARGB value. This also requires writing a floating
> point bilinear interpolator. With this change pixman can use the full
> floating point precision internally in all paths.
>
> Changes since v1:
> - Make accum and reduce an argument to convolution functions,
>   to remove duplication.
>
> Signed-off-by: Maarten Lankhorst 

Apologies for the late reply; I have been quite busy with other things
recently.

The patch looks good to me now!  I have not figured out how to visually
test the filters however.

> ---
>  pixman/pixman-bits-image.c | 414 +++--
>  pixman/pixman-inlines.h|  25 +++
>  2 files changed, 328 insertions(+), 111 deletions(-)
>
> diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
> index 9fb91ff5831d..564789e9c685 100644
> --- a/pixman/pixman-bits-image.c
> +++ b/pixman/pixman-bits-image.c
> @@ -36,43 +36,45 @@
>  #include "pixman-combine32.h"
>  #include "pixman-inlines.h"
>  
> -static uint32_t *
> -_pixman_image_get_scanline_generic_float (pixman_iter_t * iter,
> -   const uint32_t *mask)
> -{
> -pixman_iter_get_scanline_t fetch_32 = iter->data;
> -uint32_t *buffer = iter->buffer;
> -
> -fetch_32 (iter, NULL);
> +/* Fetch functions */
>  
> -pixman_expand_to_float ((argb_t *)buffer, buffer, PIXMAN_a8r8g8b8, 
> iter->width);
> +static force_inline void
> +fetch_pixel_no_alpha_32 (bits_image_t *image,
> +  int x, int y, pixman_bool_t check_bounds,
> +  void *out)
> +{
> +uint32_t *ret = out;
>  
> -return iter->buffer;
> +if (check_bounds &&
> + (x < 0 || x >= image->width || y < 0 || y >= image->height))
> + *ret = 0;
> +else
> + *ret = image->fetch_pixel_32 (image, x, y);
>  }
>  
> -/* Fetch functions */
> -
> -static force_inline uint32_t
> -fetch_pixel_no_alpha (bits_image_t *image,
> -   int x, int y, pixman_bool_t check_bounds)
> +static force_inline void
> +fetch_pixel_no_alpha_float (bits_image_t *image,
> + int x, int y, pixman_bool_t check_bounds,
> + void *out)
>  {
> +argb_t *ret = out;
> +
>  if (check_bounds &&
>   (x < 0 || x >= image->width || y < 0 || y >= image->height))
> -{
> - return 0;
> -}
> -
> -return image->fetch_pixel_32 (image, x, y);
> + ret->a = ret->r = ret->g = ret->b = 0.f;
> +else
> + *ret = image->fetch_pixel_float (image, x, y);
>  }
>  
> -typedef uint32_t (* get_pixel_t) (bits_image_t *image,
> -   int x, int y, pixman_bool_t check_bounds);
> +typedef void (* get_pixel_t) (bits_image_t *image,
> +   int x, int y, pixman_bool_t check_bounds, void 
> *out);
>  
> -static force_inline uint32_t
> +static force_inline void
>  bits_image_fetch_pixel_nearest (bits_image_t   *image,
>   pixman_fixed_t  x,
>   pixman_fixed_t  y,
> - get_pixel_t get_pixel)
> + get_pixel_t get_pixel,
> + void   *out)
>  {
>  int x0 = pixman_fixed_to_int (x - pixman_fixed_e);
>  int y0 = pixman_fixed_to_int (y - pixman_fixed_e);
> @@ -82,19 +84,20 @@ bits_image_fetch_pixel_nearest (bits_image_t   *image,
>   repeat (image->common.repeat, , image->width);
>   repeat (image->common.repeat, , image->height);
>  
> - return get_pixel (image, x0, y0, FALSE);
> + get_pixel (image, x0, y0, FALSE, out);
>  }
>  else
>  {
> - return get_pixel (image, x0, y0, TRUE);
> + get_pixel (image, x0, y0, TRUE, out);
>  }
>  }
>  
> -static force_inline uint32_t
> -bits_image_fetch_pixel_bilinear (bits_image_t   *image,
> -  pixman_fixed_t  x,
> -  pixman_fixed_t  y,
> -  get_pixel_t get_pixel)
> +static force_inline void
> +bits_image_fetch_pixel_bilinear_32 (bits_image_t   *image,
> + pixman_fixed_t  x,
> + pixman_fixed_t  y,
> + get_pixel_t get_pixel,
> + void   *out)
>  {
>  pixman_repeat_t repeat_mode = image->common.repeat;
>  int width = image->width;
> @@ -102,6 +105,7 @@ bits_image_fetch_pixel_bilinear (bits_image_t   *image,
>  int x1, y1, x2, y2;
>  uint32_t tl, tr, bl, br;
>  int32_t distx, disty;
> +uint32_t *ret = out;
>  
>  x1 = x - 

Re: [Pixman] [PATCH 1/2] Implement floating point gradient computation, v2.

[Basile Clement]

On 1/14/19 2:52 PM, Maarten Lankhorst wrote:

> 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.
>
> Changes since v1 (mlankhorst):
> - Implement pixman_gradient_walker_pixel_32 without calling
>   pixman_gradient_walker_pixel_float, to prevent performance degradation.
>   Suggested by Adam Jackson.
> - Fix whitespace errors.
> - Remove unnecessary function prototypes in pixman-private.h
> 
> Signed-off-by: Maarten Lankhorst 

Thanks for fixing the performance degradation.  I'd add a comment inline
explaining that we use this instead of expand_from_float due to
performance issues, but maybe that is not necessary.

> ---
> diff --git a/pixman/pixman-conical-gradient.c 
> b/pixman/pixman-conical-gradient.c
> index 8bb46aecdcab..a39e20c4eb68 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,
> +   intBpp,
> +   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 (
> - , (pixman_fixed_48_16_t)pixman_double_to_fixed (t));
> + write_pixel (,
> +  (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 (
> - , (pixman_fixed_48_16_t)pixman_double_to_fixed (t));
> + write_pixel (,
> +  (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 822f8e62bae7..58db4f3cc63a 100644
> --- a/pixman/pixman-gradient-walker.c
> +++ b/pixman/pixman-gradient-walker.c
> @@ -122,10 +122,9 @@ gradient_walker_reset (pixman_gradient_walker_t *walker,
>   left_c = right_c;
>  }
>  
> -/* The alpha channel is scaled to be in the [0, 255] interval,
> - * and the red/green/blue channels are scaled to be in [0, 1].
> +/* The alpha/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 +142,7 @@ gradient_walker_reset (pixman_gradient_walker_t *walker,
>  if (FLOAT_IS_ZERO (rx - lx) || left_x == INT32_MIN 

Re: [Pixman] [PATCH 2/2] pixman: Use maximum precision for pixman-bits-image

[Basile Clement]

I am far from qualified for doing a proper review on this, but this
looks reasonable overall.  I only have a few concerns outlined below.

Is there a way to visually test this?  Adding an example in demos/ would
be great, and would help ensuring there are no scaling issues in the
conv/sepconv filters (see inline comments below).

I am also a bit concerned by the duplication in the convolution and
separable_convolution functions.  I understand why it's there, but
(unlike the bilinear case) those functions are quite large by themselves
already.  If not too complex, I'd merge them by using type-erased
`accumulate(int *satot, int *srtot, int *sgtot, int *sbtot, void *pixel,
pixman_fixed_t params)` and `reduce(int *satot, int *srtot, int *sgtot,
int *sbtot, void *out)` argument functions.  They should get inlined by
the compiler anyways.

- Basile

On 1/3/19 12:18 PM, Maarten Lankhorst wrote:
> +static force_inline void
> +bits_image_fetch_pixel_convolution_float (bits_image_t   *image,
> +   pixman_fixed_t  x,
> +   pixman_fixed_t  y,
> +   get_pixel_t get_pixel,
> +   void*out)
> +{
> +pixman_fixed_t *params = image->common.filter_params;
> +int x_off = (params[0] - pixman_fixed_1) >> 1;
> +int y_off = (params[1] - pixman_fixed_1) >> 1;
> +int32_t cwidth = pixman_fixed_to_int (params[0]);
> +int32_t cheight = pixman_fixed_to_int (params[1]);
> +int32_t i, j, x1, x2, y1, y2;
> +pixman_repeat_t repeat_mode = image->common.repeat;
> +int width = image->width;
> +int height = image->height;
> +int srtot, sgtot, sbtot, satot;
> +argb_t *ret = out;
> +
> +params += 2;
> +
> +x1 = pixman_fixed_to_int (x - pixman_fixed_e - x_off);
> +y1 = pixman_fixed_to_int (y - pixman_fixed_e - y_off);
> +x2 = x1 + cwidth;
> +y2 = y1 + cheight;
> +
> +srtot = sgtot = sbtot = satot = 0;
> +
> +for (i = y1; i < y2; ++i)
> +{
> + for (j = x1; j < x2; ++j)
> + {
> + int rx = j;
> + int ry = i;
> +
> + pixman_fixed_t f = *params;
> +
> + if (f)
> + {
> + argb_t pixel;
> +
> + if (repeat_mode != PIXMAN_REPEAT_NONE)
> + {
> + repeat (repeat_mode, , width);
> + repeat (repeat_mode, , height);
> +
> + get_pixel (image, rx, ry, FALSE, );
> + }
> + else
> + {
> + get_pixel (image, rx, ry, TRUE, );
> + }
> +
> + satot += pixel.a * f;
> + srtot += pixel.r * f;
> + sgtot += pixel.g * f;
> + sbtot += pixel.b * f;

I am concerned with those lines (and the corresponding lines in
separable_convolution).  Unless I am mistaken, `sXtot` and `f` are
integer variables while `pixel.X` are floating point variables in 0-1,
so it looks like more truncation than wanted may be occuring here.

> + }
> +
> + params++;
> + }
> +}
> +
> +ret->a = CLIP (satot / 65536.f, 0.f, 1.f);
> +ret->r = CLIP (srtot / 65536.f, 0.f, 1.f);
> +ret->g = CLIP (sgtot / 65536.f, 0.f, 1.f);
> +ret->b = CLIP (sbtot / 65536.f, 0.f, 1.f);
>  }
>  
> -static uint32_t
> -bits_image_fetch_pixel_separable_convolution (bits_image_t *image,
> -  pixman_fixed_t x,
> -  pixman_fixed_t y,
> -  get_pixel_tget_pixel)
> +static void
> +bits_image_fetch_pixel_separable_convolution_32 (bits_image_t  *image,
> +  pixman_fixed_t x,
> +  pixman_fixed_t y,
> +  get_pixel_tget_pixel,
> +  void  *out)
>  {
>  pixman_fixed_t *params = image->common.filter_params;
>  pixman_repeat_t repeat_mode = image->common.repeat;
> @@ -234,6 +359,7 @@ bits_image_fetch_pixel_separable_convolution 
> (bits_image_t *image,
>  int32_t x1, x2, y1, y2;
>  int32_t px, py;
>  int i, j;
> +uint32_t *ret = out;
>  
>  /* Round x and y to the middle of the closest phase before continuing. 
> This
>   * ensures that the convolution matrix is aligned right, since it was
> @@ -278,11 +404,11 @@ bits_image_fetch_pixel_separable_convolution 
> (bits_image_t *image,
>  repeat (repeat_mode, , width);
>  repeat (repeat_mode, , height);
>  
> -pixel = get_pixel (image, rx, ry, FALSE);
> +get_pixel (image, rx, ry, FALSE, );
>  }
>  else
>  {
> -pixel = get_pixel (image, rx, ry, TRUE);
> +  

[Pixman] [PATCH 1/4] Implement floating point gradient computation

[Basile Clement]

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(-)

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,
+ intBpp,
+ 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 (
-   , (pixman_fixed_48_16_t)pixman_double_to_fixed (t));
+   write_pixel (,
+(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 (
-   , (pixman_fixed_48_16_t)pixman_double_to_fixed (t));
+   write_pixel (,
+(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,
 {

[Pixman] [PATCH 4/4] demos: Add a dithering demo

[Basile Clement]

From: Basile Clement 

This adds a dither.c which provides a demo of the dithering feature.
This is based on the scale.c demo for scaling and provides a selection
of intermediate formats and dithering operators (currently, only
PIXMAN_DITHER_BAYER_8) to use.  Images are first blitted onto a surface
of the intermediate format with the requested dither setup, then blitted
back onto a a8r8g8b8 surface for display.
---
 .gitignore|   1 +
 demos/Makefile.am |   6 +-
 demos/dither.c| 274 ++
 demos/dither.ui   | 147 +
 4 files changed, 426 insertions(+), 2 deletions(-)
 create mode 100644 demos/dither.c
 create mode 100644 demos/dither.ui

diff --git a/.gitignore b/.gitignore
index a245b69..046b161 100644
--- a/.gitignore
+++ b/.gitignore
@@ -32,6 +32,7 @@ demos/clip-in
 demos/linear-gradient
 demos/quad2quad
 demos/scale
+demos/dither
 pixman/pixman-srgb.c
 pixman/pixman-version.h
 test/*-test
diff --git a/demos/Makefile.am b/demos/Makefile.am
index e04743d..8e304ec 100644
--- a/demos/Makefile.am
+++ b/demos/Makefile.am
@@ -1,4 +1,4 @@
-EXTRA_DIST = parrot.c parrot.jpg scale.ui
+EXTRA_DIST = parrot.c parrot.jpg scale.ui dither.ui
 
 if HAVE_GTK
 
@@ -28,7 +28,8 @@ DEMOS =   \
checkerboard\
srgb-trap-test  \
srgb-test   \
-   scale
+   scale   \
+   dither
 
 gradient_test_SOURCES = gradient-test.c $(GTK_UTILS)
 alpha_test_SOURCES = alpha-test.c $(GTK_UTILS)
@@ -46,6 +47,7 @@ checkerboard_SOURCES = checkerboard.c $(GTK_UTILS)
 srgb_test_SOURCES = srgb-test.c $(GTK_UTILS)
 srgb_trap_test_SOURCES = srgb-trap-test.c $(GTK_UTILS)
 scale_SOURCES = scale.c $(GTK_UTILS)
+dither_SOURCES = dither.c $(GTK_UTILS)
 
 noinst_PROGRAMS = $(DEMOS)
 
diff --git a/demos/dither.c b/demos/dither.c
new file mode 100644
index 000..91227fb
--- /dev/null
+++ b/demos/dither.c
@@ -0,0 +1,274 @@
+/*
+ * Copyright 2012, Red Hat, Inc.
+ * Copyright 2012, Soren Sandmann
+ * Copyright 2018, Basile Clement
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include 
+#include 
+#include 
+#include "../test/utils.h"
+#include "gtk-utils.h"
+
+#define WIDTH 1024
+#define HEIGHT 640
+
+typedef struct
+{
+GtkBuilder * builder;
+pixman_image_t *original;
+pixman_format_code_t format;
+pixman_dither_t  dither;
+int  width;
+int  height;
+} app_t;
+
+static GtkWidget *
+get_widget (app_t *app, const char *name)
+{
+GtkWidget *widget = GTK_WIDGET (gtk_builder_get_object (app->builder, 
name));
+
+if (!widget)
+   g_error ("Widget %s not found\n", name);
+
+return widget;
+}
+
+typedef struct
+{
+char   name [20];
+intvalue;
+} named_int_t;
+
+static const named_int_t formats[] =
+{
+{ "a8r8g8b8",  PIXMAN_a8r8g8b8  },
+{ "x14r6g6b6", PIXMAN_x14r6g6b6 },
+{ "r5g6b5",PIXMAN_r5g6b5},
+{ "x4r4g4b4",  PIXMAN_x4r4g4b4  },
+{ "a2r2g2b2",  PIXMAN_a2r2g2b2  },
+};
+
+static const named_int_t dithers[] =
+{
+{ "None",   PIXMAN_REPEAT_NONE },
+{ "Bayer 8x8",  PIXMAN_DITHER_BAYER_8 },
+};
+
+static int
+get_value (app_t *app, const named_int_t table[], const char *box_name)
+{
+GtkComboBox *box = GTK_COMBO_BOX (get_widget (app, box_name));
+
+return table[gtk_combo_box_get_active (box)].value;
+}
+
+static void
+rescale (GtkWidget *may_be_null, app_t *app)
+{
+app->dither = get_value (app, dithers, "dithering_combo_box");
+app->format = get_value (app, formats, "target_format_combo_box");
+
+gtk_w