Re: [Pixman] Dithering patches, v2

2019-04-09 Thread Bill Spitzak
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?

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

I think the blue noise large pattern is a great idea. Could have used that
years ago in Nuke (it uses error diffusion but people always complained
that the pattern did not reproduce when tiny changes were made to the
image).


On Tue, Apr 9, 2019 at 3:01 PM Basile Clement 
wrote:

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

Re: [Pixman] Dithering patches, v2

2019-04-09 Thread 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

[Pixman] [PATCH 4/4] Ordered dithering with blue noise

2019-04-09 Thread basile-pixman
From: Basile Clement 

On some screens (typically low quality laptop screens), using Bayer
ordered dithering has been observed to cause color changes depending on
*where the gradient is rendered on the screen*, causing visible
flickering when moving an image on the screen.

To alleviate the issue, this patch adds support for ordered dithering
using a 64x64 matrix tuned from blue noise.  In addition to being devoid
of the positional dependency on screen, the blue noise matrix also
generates more pleasing and less discernable patterns.  As such, it is
now the method used for PIXMAN_DITHER_GOOD and PIXMAN_DITHER_BEST
dithering methods.

The 64x64 blue noise matrix has been generated using the provided
`pixman/dither/make-blue-noise.c` script, which uses the
void-and-cluster method.
---
 demos/dither.c   |   1 +
 pixman/dither/blue-noise-64x64.h |  77 
 pixman/dither/make-blue-noise.c  | 679 +++
 pixman/pixman-bits-image.c   |  16 +-
 pixman/pixman.h  |   1 +
 test/tolerance-test.c|   1 +
 test/utils.c |   1 +
 7 files changed, 775 insertions(+), 1 deletion(-)
 create mode 100644 pixman/dither/blue-noise-64x64.h
 create mode 100644 pixman/dither/make-blue-noise.c

diff --git a/demos/dither.c b/demos/dither.c
index d4271e6..d72c250 100644
--- a/demos/dither.c
+++ b/demos/dither.c
@@ -78,6 +78,7 @@ static const named_int_t dithers[] =
 {
 { "None",   PIXMAN_REPEAT_NONE },
 { "Bayer 8x8",  PIXMAN_DITHER_ORDERED_BAYER_8 },
+{ "Blue noise 64x64",   PIXMAN_DITHER_ORDERED_BLUE_NOISE_64 },
 };
 
 static int
diff --git a/pixman/dither/blue-noise-64x64.h b/pixman/dither/blue-noise-64x64.h
new file mode 100644
index 000..e196d2d
--- /dev/null
+++ b/pixman/dither/blue-noise-64x64.h
@@ -0,0 +1,77 @@
+/* WARNING: This file is generated by make-blue-noise.c
+ * Please edit that file instead of this one.
+ */
+
+#ifndef BLUE_NOISE_64X64_H
+#define BLUE_NOISE_64X64_H
+
+#include 
+
+static const uint32_t dither_blue_noise_64x64[4096] = {
+3039, 1368, 3169, 103, 2211, 1248, 2981, 668, 2633, 37, 3963, 2903, 384, 
2564, 3115, 1973, 3348, 830, 2505, 1293, 3054, 1060, 1505, 3268, 400, 1341, 
593, 3802, 3384, 429, 4082, 1411, 2503, 3863, 126, 1292, 1887, 2855, 205, 2094, 
2977, 1899, 3924, 356, 3088, 2500, 3942, 1409, 2293, 1734, 3732, 1291, 3227, 
277, 2054, 786, 2871, 411, 2425, 1678, 3986, 455, 2879, 2288,
+388, 1972, 3851, 778, 2768, 3697, 944, 2123, 1501, 3533, 937, 1713, 1381, 
3888, 156, 1242, 516, 2888, 1607, 3676, 632, 2397, 3804, 2673, 1898, 3534, 
2593, 1777, 1170, 2299, 3013, 1838, 523, 3053, 1647, 3601, 3197, 959, 1520, 
3633, 893, 2437, 3367, 2187, 1258, 137, 1965, 401, 3546, 643, 3087, 2498, 733, 
2786, 3371, 4053, 1266, 1977, 3663, 183, 2570, 2107, 1183, 3708,
+907, 2473, 1151, 3363, 1527, 1902, 232, 3903, 3060, 496, 2486, 3206, 2165, 
861, 2387, 3653, 2101, 3972, 132, 2162, 3437, 1827, 215, 895, 3114, 271, 969, 
2932, 197, 1598, 878, 3696, 1140, 2120, 904, 2431, 302, 3846, 2675, 481, 3187, 
66, 1440, 650, 3833, 2826, 3435, 901, 2936, 2111, 250, 1875, 3609, 1174, 1747, 
162, 2346, 3420, 913, 3172, 1383, 752, 3298, 1735,
+3540, 2938, 249, 2324, 526, 3099, 2561, 1324, 2347, 1861, 1200, 3702, 257, 
3442, 1514, 2999, 992, 1766, 2735, 1163, 478, 2943, 1279, 3635, 2177, 1464, 
3672, 2386, 3871, 3340, 2690, 64, 3489, 2811, 3999, 633, 1948, 1243, 2269, 
1807, 1143, 2750, 3729, 1790, 2363, 1053, 1537, 2636, 4065, 1076, 1476, 3869, 
450, 2200, 2676, 658, 2979, 1548, 544, 1913, 2838, 3911, 116, 2698,
+517, 1295, 3997, 1739, 3665, 1083, 3509, 599, 3400, 118, 2956, 720, 2689, 
1907, 567, 2523, 284, 3397, 711, 3219, 2450, 3985, 1665, 2549, 562, 3011, 1855, 
729, 1355, 528, 1908, 2456, 1384, 337, 1540, 2654, 3138, 3513, 703, 4080, 3314, 
2047, 855, 3037, 209, 3317, 577, 1828, 17, 2336, 3193, 2748, 962, 3441, 1450, 
3246, 1075, 3878, 2615, 3497, 1033, 2310, 1442, 2183,
+1654, 3254, 2061, 738, 2832, 148, 2030, 1670, 909, 3850, 2109, 1533, 4046, 
1085, 3098, 3897, 1378, 2248, 3829, 1495, 1966, 23, 797, 3427, 1124, 4057, 95, 
2787, 2190, 3074, 3950, 742, 3194, 1999, 3386, 1113, 16, 1657, 2804, 201, 1543, 
383, 2559, 1325, 3604, 2068, 2493, 3771, 1284, 3460, 710, 1716, 2447, 80, 3811, 
2032, 347, 2227, 15, 1689, 397, 3084, 662, 3798,
+973, 43, 2608, 3143, 1459, 2423, 4066, 2770, 3191, 1283, 2630, 314, 3235, 
2289, 72, 1822, 2840, 924, 350, 2653, 1057, 3715, 2235, 2775, 346, 2083, 1553, 
3292, 1081, 274, 1686, 1188, 2327, 3743, 578, 2234, 3916, 2519, 1011, 3056, 
2207, 3438, 3890, 537, 1617, 837, 3094, 373, 2795, 1980, 276, 3951, 1353, 3015, 
844, 1724, 3651, 2923, 1316, 4092, 2504, 3627, 1936, 2854,
+2461, 3929, 1193, 421, 3746, 820, 1180, 286, 2261, 532, 3625, 1812, 802, 
1327, 3527, 670, 3730, 2025, 3124, 3565, 529, 2960, 1769, 1390, 3196, 2494, 
3756, 796, 3618, 2602, 3463, 2847, 166, 953, 1745, 2900, 438, 2070, 1418, 3741, 
639, 1205, 1891, 2882, 2282, 

[Pixman] [PATCH 1/4] Implement basic dithering for the wide pipeline

2019-04-09 Thread basile-pixman
From: Basile Clement 

This patch implements dithering in pixman.  A "dither" property is added
to BITS images, which is used to:

 - Force rendering to the image to go through the floating point
   pipeline.  Note that this is different from FAST_PATH_NARROW_FORMAT
   as it should not enable the floating point pipeline when reading from
   the image.

 - Enable dithering in dest_write_back_wide.  The dithering uses the
   destination format to determine noise amplitude.

This does not change pixman's behavior when dithering is disabled (the
default).

Additional types and functions are added to the public API:

 - The `pixman_dither_t` enum exposes the available dithering methods.
   Currently a single dithering method based on 8x8 Bayer matrices is
   implemented (PIXMAN_DITHER_ORDERED_BAYER_8).  The PIXMAN_DITHER_FAST,
   PIXMAN_DITHER_GOOD and PIXMAN_DITHER_BEST aliases are provided and
   should be used to benefit from future specializations.

 - The `pixman_image_set_dither` function allows to set the dithering
   method to use when rendering to a bits image.

 - The `pixman_image_set_dither_offset` function allows to set a
   vertical and horizontal offsets for the dither matrix.  This can be
   used after scrolling to ensure a consistent spatial positioning of
   the dither matrix.

Changes since previous version:
 - Renamed PIXMAN_DITHER_BAYER_8 to PIXMAN_DITHER_ORDERED_BAYER_8
 - Disable dithering for channels with 32bpp or more (since they can
   represent exactly the wide values already).  This makes the patches
   compatible with the newly added floating point format.
---
 pixman/pixman-bits-image.c  | 122 
 pixman/pixman-general.c |   3 +-
 pixman/pixman-image.c   |  35 +
 pixman/pixman-linear-gradient.c |  11 +--
 pixman/pixman-private.h |   4 ++
 pixman/pixman.h |  14 
 6 files changed, 183 insertions(+), 6 deletions(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 7bc2ba8..fe3919b 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -1048,6 +1048,112 @@ dest_write_back_narrow (pixman_iter_t *iter)
 iter->y++;
 }
 
+static const float
+dither_factor_bayer_8 (int x, int y)
+{
+uint32_t m;
+
+y ^= x;
+
+/* Compute reverse(interleave(xor(x mod n, y mod n), x mod n))
+ * Here n = 8 and `mod n` is the bottom 3 bits.
+ */
+m = ((y & 0b001) << 5) | ((x & 0b001) << 4) |
+   ((y & 0b010) << 2) | ((x & 0b010) << 1) |
+   ((y & 0b100) >> 1) | ((x & 0b100) >> 2);
+
+/* m is in range [0, 63].  We scale it to [0, 63.0f/64.0f], then
+ * shift it to to [1.0f/128.0f, 127.0f/128.0f] so that 0 < d < 1.
+ * This ensures exact values are not changed by dithering.
+ */
+return (float)(m) * (1 / 64.0f) + (1.0f / 128.0f);
+}
+
+typedef float (* dither_factor_t)(int x, int y);
+
+static force_inline float
+dither_apply_channel (float f, float d, float s)
+{
+/* float_to_unorm splits the [0, 1] segment in (1 << n_bits)
+ * subsections of equal length; however unorm_to_float does not
+ * map to the center of those sections.  In fact, pixel value u is
+ * mapped to:
+ *
+ *   u  u  u   1
+ * -- = -- + -- * --
+ *  2^n_bits - 1 2^n_bits 2^n_bits - 1 2^n_bits
+ *
+ * Hence if f = u / (2^n_bits - 1) is exactly representable on a
+ * n_bits palette, all the numbers between
+ *
+ * u
+ * --  =  f - f * 2^n_bits = f + (0 - f) * 2^n_bits
+ *  2^n_bits
+ *
+ *  and
+ *
+ *u + 1
+ * -- = f - (f - 1) * 2^n_bits = f + (1 - f) * 2^n_bits
+ *  2^n_bits
+ *
+ * are also mapped back to u.
+ *
+ * Hence the following calculation ensures that we add as much
+ * noise as possible without perturbing values which are exactly
+ * representable in the target colorspace.  Note that this corresponds to
+ * mixing the original color with noise with a ratio of `1 / 2^n_bits`.
+ */
+return f + (d - f) * s;
+}
+
+static force_inline float
+dither_compute_scale (int n_bits)
+{
+// No dithering for wide formats
+if (n_bits == 0 || n_bits >= 32)
+   return 0.f;
+
+return 1.f / (float)(1 << n_bits);
+}
+
+static const uint32_t *
+dither_apply_ordered (pixman_iter_t *iter, dither_factor_t factor)
+{
+bits_image_t*image  = >image->bits;
+int  x  = iter->x + image->dither_offset_x;
+int  y  = iter->y + image->dither_offset_y;
+int  width  = iter->width;
+argb_t  *buffer = (argb_t *)iter->buffer;
+
+pixman_format_code_t format = image->format;
+int  a_size = PIXMAN_FORMAT_A (format);
+int  r_size = PIXMAN_FORMAT_R (format);
+int  g_size = PIXMAN_FORMAT_G 

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

2019-04-09 Thread basile-pixman
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_ORDERED_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 |   5 +-
 demos/dither.c| 278 ++
 demos/dither.ui   | 147 
 demos/meson.build |   1 +
 5 files changed, 431 insertions(+), 1 deletion(-)
 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 44a5553..c58e359 100644
--- a/demos/Makefile.am
+++ b/demos/Makefile.am
@@ -2,6 +2,7 @@ EXTRA_DIST =\
  parrot.c  \
  parrot.jpg\
  scale.ui  \
+ dither.ui \
  meson.build   \
  $(NULL)
 
@@ -33,7 +34,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)
@@ -51,6 +53,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..d4271e6
--- /dev/null
+++ b/demos/dither.c
@@ -0,0 +1,278 @@
+/*
+ * 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  },
+{ "rgb",   PIXMAN_rgb_float },
+{ "sRGB",  PIXMAN_a8r8g8b8_sRGB },
+{ "r5g6b5",PIXMAN_r5g6b5},
+{ "a4r4g4b4",  PIXMAN_a4r4g4b4  },
+{ "a2r2g2b2",  PIXMAN_a2r2g2b2  },
+{ "r3g3b2",PIXMAN_r3g3b2},
+{ "r1g2b1",PIXMAN_r1g2b1},
+{ "a1r1g1b1",  PIXMAN_a1r1g1b1  },
+};
+
+static const named_int_t dithers[] =
+{
+{ "None",   PIXMAN_REPEAT_NONE },
+{ "Bayer 8x8",  PIXMAN_DITHER_ORDERED_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 

[Pixman] [PATCH 2/4] test: Check the dithering path in tolerance-test

2019-04-09 Thread basile-pixman
From: Basile Clement 

This adds support for testing dithered destinations in tolerance-test.
When dithering is enabled, the pixel checker allows for an additional
quantization error.
---
 test/tolerance-test.c |  22 +++-
 test/utils.c  | 121 ++
 test/utils.h  |  12 +
 3 files changed, 142 insertions(+), 13 deletions(-)

diff --git a/test/tolerance-test.c b/test/tolerance-test.c
index 320bb7f..011cdb4 100644
--- a/test/tolerance-test.c
+++ b/test/tolerance-test.c
@@ -76,6 +76,11 @@ static const pixman_op_t operators[] =
 PIXMAN_OP_EXCLUSION,
 };
 
+static const pixman_dither_t dithers[] =
+{
+PIXMAN_DITHER_ORDERED_BAYER_8,
+};
+
 #define RANDOM_ELT(array)   \
 (array[prng_rand_n (ARRAY_LENGTH (array))])
 
@@ -176,7 +181,8 @@ verify (int test_no,
 pixman_image_t *orig_dest,
 int x, int y,
 int width, int height,
-   pixman_bool_t component_alpha)
+   pixman_bool_t component_alpha,
+   pixman_dither_t dither)
 {
 pixel_checker_t dest_checker, src_checker, mask_checker;
 int i, j;
@@ -185,6 +191,9 @@ verify (int test_no,
 pixel_checker_init (_checker, dest->bits.format);
 pixel_checker_init (_checker, mask->bits.format);
 
+if (dest->bits.dither != PIXMAN_DITHER_NONE)
+   pixel_checker_allow_dither (_checker);
+
 assert (dest->bits.format == orig_dest->bits.format);
 
 for (j = y; j < y + height; ++j)
@@ -220,6 +229,7 @@ verify (int test_no,
 
 printf ("   operator: %s (%s alpha)\n", operator_name 
(op),
component_alpha? "component" : "unified");
+   printf ("   dither:   %s\n", dither_name (dither));
 printf ("   dest_x, dest_y:   %d %d\n", x, y);
 printf ("   width, height:%d %d\n", width, height);
 printf ("   source:   format: %-14s  size: %2d x 
%2d\n",
@@ -275,6 +285,7 @@ do_check (int i)
 pixman_image_t *dest_copy;
 pixman_bool_t result = TRUE;
 pixman_bool_t component_alpha;
+pixman_dither_t dither = PIXMAN_DITHER_NONE;
 
 prng_srand (i);
 op = RANDOM_ELT (operators);
@@ -296,6 +307,12 @@ do_check (int i)
 if (y + height > dest->bits.height)
 height = dest->bits.height - y;
 
+if (prng_rand_n (2))
+{
+   dither = RANDOM_ELT (dithers);
+   pixman_image_set_dither (dest, dither);
+}
+
 component_alpha = prng_rand_n (2);
 
 pixman_image_set_component_alpha (mask, component_alpha);
@@ -305,7 +322,8 @@ do_check (int i)
   x, y, width, height);
 
 if (!verify (i, op, source, mask, dest, dest_copy,
-x, y, width, height, component_alpha))
+x, y, width, height, component_alpha,
+dither))
 {
result = FALSE;
 }
diff --git a/test/utils.c b/test/utils.c
index 4eeb068..9ce7a6c 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -1174,6 +1174,31 @@ static const operator_entry_t op_list[] =
 #undef ALIAS
 };
 
+typedef struct {
+pixman_dither_t dither;
+const char *name;
+pixman_bool_t   is_alias;
+} dither_entry_t;
+
+static const dither_entry_t dither_list[] =
+{
+#define ENTRY(dither)  \
+{ PIXMAN_DITHER_##dither, "PIXMAN_DITHER_" #dither, FALSE }
+#define ALIAS(dither, nam) 
\
+{ PIXMAN_DITHER_##dither, nam, TRUE }
+
+/* dither_name () will return the first hit in this table,
+ * so keep the list properly ordered between entries and aliases.
+ * Aliases are not listed by list_dithers ().
+ */
+
+ENTRY (ORDERED_BAYER_8),
+ENTRY (NONE),
+
+#undef ENTRY
+#undef ALIAS
+};
+
 struct format_entry
 {
 pixman_format_code_t format;
@@ -1382,6 +1407,28 @@ list_operators (void)
 printf ("\n\n");
 }
 
+void
+list_dithers (void)
+{
+int n_chars;
+int i;
+
+printf ("Dithers:\n");
+
+n_chars = 0;
+for (i = 0; i < ARRAY_LENGTH (dither_list); ++i)
+{
+const dither_entry_t *ent = _list[i];
+
+if (ent->is_alias)
+continue;
+
+emit (ent->name, _chars);
+}
+
+printf ("\n\n");
+}
+
 pixman_op_t
 operator_from_string (const char *s)
 {
@@ -1406,6 +1453,22 @@ operator_from_string (const char *s)
 return PIXMAN_OP_NONE;
 }
 
+pixman_dither_t
+dither_from_string (const char *s)
+{
+int i;
+
+for (i = 0; i < ARRAY_LENGTH (dither_list); ++i)
+{
+const dither_entry_t *ent = _list[i];
+
+if (strcasecmp (ent->name, s) == 0)
+return ent->dither;
+}
+
+return PIXMAN_DITHER_NONE;
+}
+
 const char *
 operator_name (pixman_op_t op)
 {
@@ -1438,6 +1501,22 @@ format_name (pixman_format_code_t format)
 return "";
 };
 
+const char *
+dither_name (pixman_dither_t dither)
+{
+int 

[Pixman] Dithering patches, v2

2019-04-09 Thread basile-pixman
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] [PATCH] Fix bilinear filter computation in wide pipeline

2019-04-09 Thread basile-pixman
From: Basile Clement 

The recently introduced wide pipeline for filters has a typo which
causes it to improperly compute bilinear interpolation positions,
causing various glitches when enabled.

This patch uses the proper computation for bilinear interpolation in the
wide pipeline.  It also makes related `if` statements conformant to the
CODING_STYLE:

* If a substatement spans multiple lines, then there must be braces
  around it.

* If one substatement of an if statement has braces, then the other
  must too.
---
 pixman/pixman-bits-image.c | 9 +
 pixman/pixman-inlines.h| 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index 564789e..7bc2ba8 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -432,29 +432,38 @@ bits_image_fetch_pixel_filtered (bits_image_t  *image,
 
 case PIXMAN_FILTER_CONVOLUTION:
if (wide)
+   {
bits_image_fetch_pixel_convolution (image, x, y,
get_pixel, out,
accum_float,
reduce_float);
+   }
else
+   {
bits_image_fetch_pixel_convolution (image, x, y,
get_pixel, out,
accum_32, reduce_32);
+   }
break;
 
 case PIXMAN_FILTER_SEPARABLE_CONVOLUTION:
if (wide)
+   {
bits_image_fetch_pixel_separable_convolution (image, x, y,
  get_pixel, out,
  accum_float,
  reduce_float);
+   }
else
+   {
bits_image_fetch_pixel_separable_convolution (image, x, y,
  get_pixel, out,
  accum_32, reduce_32);
+   }
 break;
 
 default:
+   assert (0);
 break;
 }
 }
diff --git a/pixman/pixman-inlines.h b/pixman/pixman-inlines.h
index 332e208..f785910 100644
--- a/pixman/pixman-inlines.h
+++ b/pixman/pixman-inlines.h
@@ -231,7 +231,7 @@ bilinear_interpolation_float (argb_t tl, argb_t tr,
 argb_t r;
 
 distxy = distx * disty;
-distxiy = distx - (1.f - distxy);
+distxiy = distx * (1.f - disty);
 distixy = (1.f - distx) * disty;
 distixiy = (1.f - distx) * (1.f - disty);
 
-- 
2.21.0

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