[v2] pixman: Add support for argb/xrgb float formats

Submitted by Maarten Lankhorst on June 11, 2018, 11:34 a.m.

Details

Message ID 2eae4b74-b807-d005-3618-da8864b5d778@linux.intel.com
State New
Headers show
Series "pixman: Add support for argb/xrgb float formats" ( rev: 2 ) in Pixman

Not browsing as part of any series.

Commit Message

Maarten Lankhorst June 11, 2018, 11:34 a.m.
Pixman is already using the floating point formats internally, expose
this capability in case someone wants to support higher bit per
component formats.

This is useful for igt which depends on cairo to do the rendering.
It can use it to convert floats internally to planar Y'CbCr formats,
or to F16.

Changes since v1:
- Use RGBA 128 bits and RGB 96 bits memory layouts, to better match the opengl format.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 pixman/pixman-access.c | 128 ++++++++++++++++++++++++++++++++++++++++-
 pixman/pixman.h        |  36 ++++++++----
 test/alphamap.c        |  73 ++++++++++++++++-------
 test/utils.c           |  76 ++++++++++++++++++++++++
 test/utils.h           |   2 +
 5 files changed, 282 insertions(+), 33 deletions(-)

Patch hide | download patch | download mbox

diff --git a/pixman/pixman-access.c b/pixman/pixman-access.c
index 4f0642d77785..10fa049becf4 100644
--- a/pixman/pixman-access.c
+++ b/pixman/pixman-access.c
@@ -642,6 +642,48 @@  fetch_scanline_a2r10g10b10_float (bits_image_t *  image,
 }
 
 /* Expects a float buffer */
+#ifndef PIXMAN_FB_ACCESSORS
+static void
+fetch_scanline_rgb_float_float (bits_image_t   *image,
+				int             x,
+				int             y,
+				int             width,
+				uint32_t *      b,
+				const uint32_t *mask)
+{
+    const float *bits = (float *)image->bits + y * image->rowstride;
+    const float *pixel = bits + x * 3;
+    argb_t *buffer = (argb_t *)b;
+
+    for (; width--; buffer++) {
+	buffer->r = *pixel++;
+	buffer->g = *pixel++;
+	buffer->b = *pixel++;
+	buffer->a = 1.f;
+    }
+}
+
+static void
+fetch_scanline_rgba_float_float (bits_image_t   *image,
+				int             x,
+				int             y,
+				int             width,
+				uint32_t *      b,
+				const uint32_t *mask)
+{
+    const float *bits = (float *)image->bits + y * image->rowstride;
+    const float *pixel = bits + x * 4;
+    argb_t *buffer = (argb_t *)b;
+
+    for (; width--; buffer++) {
+	buffer->r = *pixel++;
+	buffer->g = *pixel++;
+	buffer->b = *pixel++;
+	buffer->a = *pixel++;
+    }
+}
+#endif
+
 static void
 fetch_scanline_x2r10g10b10_float (bits_image_t   *image,
 				  int             x,
@@ -805,6 +847,40 @@  fetch_scanline_yv12 (bits_image_t   *image,
 
 /**************************** Pixel wise fetching *****************************/
 
+#ifndef PIXMAN_FB_ACCESSORS
+static argb_t
+fetch_pixel_rgb_float_float (bits_image_t *image,
+			     int	    offset,
+			     int	    line)
+{
+    float *bits = (float *)image->bits + line * image->rowstride;
+    argb_t argb;
+
+    argb.r = bits[offset * 3];
+    argb.g = bits[offset * 3 + 1];
+    argb.b = bits[offset * 3 + 2];
+    argb.a = 1.f;
+
+    return argb;
+}
+
+static argb_t
+fetch_pixel_rgba_float_float (bits_image_t *image,
+			      int	    offset,
+			      int	    line)
+{
+    float *bits = (float *)image->bits + line * image->rowstride;
+    argb_t argb;
+
+    argb.r = bits[offset * 4];
+    argb.g = bits[offset * 4 + 1];
+    argb.b = bits[offset * 4 + 2];
+    argb.a = bits[offset * 4 + 3];
+
+    return argb;
+}
+#endif
+
 static argb_t
 fetch_pixel_x2r10g10b10_float (bits_image_t *image,
 			       int	   offset,
@@ -962,6 +1038,45 @@  fetch_pixel_yv12 (bits_image_t *image,
 
 /*********************************** Store ************************************/
 
+#ifndef PIXMAN_FB_ACCESSORS
+static void
+store_scanline_rgba_float_float (bits_image_t *  image,
+				 int             x,
+				 int             y,
+				 int             width,
+				 const uint32_t *v)
+{
+    float *bits = (float *)image->bits + image->rowstride * y + 4 * x;
+    const argb_t *values = (argb_t *)v;
+
+    for (; width; width--, values++)
+    {
+	*bits++ = values->r;
+	*bits++ = values->g;
+	*bits++ = values->b;
+	*bits++ = values->a;
+    }
+}
+
+static void
+store_scanline_rgb_float_float (bits_image_t *  image,
+				int             x,
+				int             y,
+				int             width,
+				const uint32_t *v)
+{
+    float *bits = (float *)image->bits + image->rowstride * y + 3 * x;
+    const argb_t *values = (argb_t *)v;
+
+    for (; width; width--, values++)
+    {
+	*bits++ = values->r;
+	*bits++ = values->g;
+	*bits++ = values->b;
+    }
+}
+#endif
+
 static void
 store_scanline_a2r10g10b10_float (bits_image_t *  image,
 				  int             x,
@@ -1351,7 +1466,18 @@  static const format_info_t accessors[] =
     FORMAT_INFO (g1),
     
 /* Wide formats */
-    
+#ifndef PIXMAN_FB_ACCESSORS
+    { PIXMAN_rgba_float,
+      NULL, fetch_scanline_rgba_float_float,
+      fetch_pixel_generic_lossy_32, fetch_pixel_rgba_float_float,
+      NULL, store_scanline_rgba_float_float },
+
+    { PIXMAN_rgb_float,
+      NULL, fetch_scanline_rgb_float_float,
+      fetch_pixel_generic_lossy_32, fetch_pixel_rgb_float_float,
+      NULL, store_scanline_rgb_float_float },
+#endif
+
     { PIXMAN_a2r10g10b10,
       NULL, fetch_scanline_a2r10g10b10_float,
       fetch_pixel_generic_lossy_32, fetch_pixel_a2r10g10b10_float,
diff --git a/pixman/pixman.h b/pixman/pixman.h
index 509ba5e534a8..c376278c83e7 100644
--- a/pixman/pixman.h
+++ b/pixman/pixman.h
@@ -647,19 +647,28 @@  struct pixman_indexed
  * sample implementation allows only packed RGB and GBR
  * representations for data to simplify software rendering,
  */
-#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	(((bpp) << 24) |  \
+#define PIXMAN_FORMAT_PACK_BPP(bpp) ((bpp) <= 64 || (bpp) == 96 ? (bpp) : \
+				     (bpp) == 128 ? 65 : 0)
+#define PIXMAN_FORMAT_UNPACK_BPP(bpp) ((bpp) <= 64 || (bpp) == 96 ? (bpp) : \
+				       (bpp) == 65 ? 128 : 0)
+#define PIXMAN_FORMAT_PACKC(val) ((val) <= 10 ? (val) : \
+				 (val) == 32 ? 11 : 0)
+#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \
+				    (val) == 11 ? 32 : 0)
+
+#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	((PIXMAN_FORMAT_PACK_BPP(bpp) << 24) |  \
 					 ((type) << 16) | \
-					 ((a) << 12) |	  \
-					 ((r) << 8) |	  \
-					 ((g) << 4) |	  \
-					 ((b)))
+					 (PIXMAN_FORMAT_PACKC(a) << 12) |	  \
+					 (PIXMAN_FORMAT_PACKC(r) << 8) |	  \
+					 (PIXMAN_FORMAT_PACKC(g) << 4) |	  \
+					 (PIXMAN_FORMAT_PACKC(b)))
 
-#define PIXMAN_FORMAT_BPP(f)	(((f) >> 24)       )
+#define PIXMAN_FORMAT_BPP(f)	PIXMAN_FORMAT_UNPACK_BPP((f) >> 24)
 #define PIXMAN_FORMAT_TYPE(f)	(((f) >> 16) & 0xff)
-#define PIXMAN_FORMAT_A(f)	(((f) >> 12) & 0x0f)
-#define PIXMAN_FORMAT_R(f)	(((f) >>  8) & 0x0f)
-#define PIXMAN_FORMAT_G(f)	(((f) >>  4) & 0x0f)
-#define PIXMAN_FORMAT_B(f)	(((f)      ) & 0x0f)
+#define PIXMAN_FORMAT_A(f)	PIXMAN_FORMAT_UNPACKC(((f) >> 12) & 0x0f)
+#define PIXMAN_FORMAT_R(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  8) & 0x0f)
+#define PIXMAN_FORMAT_G(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  4) & 0x0f)
+#define PIXMAN_FORMAT_B(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  0) & 0x0f)
 #define PIXMAN_FORMAT_RGB(f)	(((f)      ) & 0xfff)
 #define PIXMAN_FORMAT_VIS(f)	(((f)      ) & 0xffff)
 #define PIXMAN_FORMAT_DEPTH(f)	(PIXMAN_FORMAT_A(f) +	\
@@ -685,8 +694,13 @@  struct pixman_indexed
 	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_BGRA ||	\
 	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_RGBA)
 
-/* 32bpp formats */
 typedef enum {
+/* 128bpp formats */
+    PIXMAN_rgba_float =	PIXMAN_FORMAT(128,PIXMAN_TYPE_ARGB,32,32,32,32),
+/* 96bpp formats */
+    PIXMAN_rgb_float =	PIXMAN_FORMAT(96,PIXMAN_TYPE_ARGB,0,32,32,32),
+
+/* 32bpp formats */
     PIXMAN_a8r8g8b8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ARGB,8,8,8,8),
     PIXMAN_x8r8g8b8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ARGB,0,8,8,8),
     PIXMAN_a8b8g8r8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ABGR,8,8,8,8),
diff --git a/test/alphamap.c b/test/alphamap.c
index 4d09076fbcf3..150d33eed5b5 100644
--- a/test/alphamap.c
+++ b/test/alphamap.c
@@ -10,7 +10,8 @@  static const pixman_format_code_t formats[] =
     PIXMAN_a8r8g8b8,
     PIXMAN_a2r10g10b10,
     PIXMAN_a4r4g4b4,
-    PIXMAN_a8
+    PIXMAN_a8,
+    PIXMAN_rgba_float,
 };
 
 static const pixman_format_code_t alpha_formats[] =
@@ -18,7 +19,8 @@  static const pixman_format_code_t alpha_formats[] =
     PIXMAN_null,
     PIXMAN_a8,
     PIXMAN_a2r10g10b10,
-    PIXMAN_a4r4g4b4
+    PIXMAN_a4r4g4b4,
+    PIXMAN_rgba_float,
 };
 
 static const int origins[] =
@@ -41,7 +43,10 @@  make_image (pixman_format_code_t format)
     uint8_t bpp = PIXMAN_FORMAT_BPP (format) / 8;
     pixman_image_t *image;
 
-    bits = (uint32_t *)make_random_bytes (WIDTH * HEIGHT * bpp);
+    if (format != PIXMAN_rgba_float)
+	bits = (uint32_t *)make_random_bytes (WIDTH * HEIGHT * bpp);
+    else
+	bits = (uint32_t *)make_random_floats (WIDTH * HEIGHT * bpp);
 
     image = pixman_image_create_bits (format, WIDTH, HEIGHT, bits, WIDTH * bpp);
 
@@ -51,11 +56,11 @@  make_image (pixman_format_code_t format)
     return image;
 }
 
-static uint8_t
+static float
 get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
 {
     uint8_t *bits;
-    uint8_t r;
+    uint32_t r;
 
     if (image->common.alpha_map)
     {
@@ -69,7 +74,7 @@  get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
 	}
 	else
 	{
-	    return 0;
+	    return 0.f;
 	}
     }
 
@@ -78,28 +83,32 @@  get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
     if (image->bits.format == PIXMAN_a8)
     {
 	r = bits[y * WIDTH + x];
+	return r / 255.f;
     }
     else if (image->bits.format == PIXMAN_a2r10g10b10)
     {
 	r = ((uint32_t *)bits)[y * WIDTH + x] >> 30;
-	r |= r << 2;
-	r |= r << 4;
+	return r / 3.f;
     }
     else if (image->bits.format == PIXMAN_a8r8g8b8)
     {
 	r = ((uint32_t *)bits)[y * WIDTH + x] >> 24;
+	return r / 255.f;
     }
     else if (image->bits.format == PIXMAN_a4r4g4b4)
     {
 	r = ((uint16_t *)bits)[y * WIDTH + x] >> 12;
-	r |= r << 4;
+	return r / 15.f;
+    }
+    else if (image->bits.format == PIXMAN_rgba_float)
+    {
+	return ((float *)bits)[y * WIDTH * 4 + x * 4 + 3];
     }
     else
     {
 	assert (0);
+	return 0.f;
     }
-
-    return r;
 }
 
 static uint16_t
@@ -133,6 +142,11 @@  get_red (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
 	r |= r << 4;
 	r |= r << 8;
     }
+    else if (image->bits.format == PIXMAN_rgba_float)
+    {
+	double tmp = ((float *)bits)[y * WIDTH * 4 + x * 4];
+	return tmp * 65535.;
+    }
     else
     {
 	assert (0);
@@ -141,6 +155,23 @@  get_red (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
     return r;
 }
 
+static float get_alpha_err(pixman_format_code_t sf, pixman_format_code_t saf,
+			   pixman_format_code_t df, pixman_format_code_t daf)
+{
+	pixman_format_code_t s = saf != PIXMAN_null ? saf : sf;
+	pixman_format_code_t d = daf != PIXMAN_null ? daf : df;
+
+	/* There are cases where we go through the 8 bit compositing
+	 * path even with 10bpc and higher formats.
+	 */
+	if (PIXMAN_FORMAT_A(s) == PIXMAN_FORMAT_A(d))
+		return 1.f / 255.f;
+	else if (PIXMAN_FORMAT_A(s) > PIXMAN_FORMAT_A(d))
+		return 1.f / ((1 << PIXMAN_FORMAT_A(d)) - 1);
+	else
+		return 1.f / ((1 << PIXMAN_FORMAT_A(s)) - 1);
+}
+
 static int
 run_test (int s, int d, int sa, int da, int soff, int doff)
 {
@@ -151,15 +182,11 @@  run_test (int s, int d, int sa, int da, int soff, int doff)
     pixman_image_t *src, *dst, *orig_dst, *alpha, *orig_alpha;
     pixman_transform_t t1;
     int j, k;
-    int n_alpha_bits, n_red_bits;
+    int n_red_bits;
 
     soff = origins[soff];
     doff = origins[doff];
 
-    n_alpha_bits = PIXMAN_FORMAT_A (df);
-    if (daf != PIXMAN_null)
-	n_alpha_bits = PIXMAN_FORMAT_A (daf);
-
     n_red_bits = PIXMAN_FORMAT_R (df);
 
     /* Source */
@@ -211,21 +238,25 @@  run_test (int s, int d, int sa, int da, int soff, int doff)
     {
 	for (k = MAX (doff, 0); k < MIN (WIDTH, WIDTH + doff); ++k)
 	{
-	    uint8_t sa, da, oda, refa;
+	    float sa, da, oda, refa;
 	    uint16_t sr, dr, odr, refr;
+	    float err;
+
+	    err = get_alpha_err(sf, saf, df, daf);
 
 	    sa = get_alpha (src, k, j, soff, soff);
 	    da = get_alpha (dst, k, j, doff, doff);
 	    oda = get_alpha (orig_dst, k, j, doff, doff);
 
-	    if (sa + oda > 255)
-		refa = 255;
+	    if (sa + oda > 1.f)
+		refa = 1.f;
 	    else
 		refa = sa + oda;
 
-	    if (da >> (8 - n_alpha_bits) != refa >> (8 - n_alpha_bits))
+	    if (da - err > refa ||
+	        da + err < refa)
 	    {
-		printf ("\nWrong alpha value at (%d, %d). Should be 0x%x; got 0x%x. Source was 0x%x, original dest was 0x%x\n",
+		printf ("\nWrong alpha value at (%d, %d). Should be %g; got %g. Source was %g, original dest was %g\n",
 			k, j, refa, da, sa, oda);
 
 		printf ("src: %s, alpha: %s, origin %d %d\ndst: %s, alpha: %s, origin: %d %d\n\n",
diff --git a/test/utils.c b/test/utils.c
index f8e42a5d3f3d..4eeb068497ec 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -595,6 +595,21 @@  make_random_bytes (int n_bytes)
     return bytes;
 }
 
+float *
+make_random_floats (int n_bytes)
+{
+    uint8_t *bytes = fence_malloc (n_bytes);
+    float *vals = (float *)bytes;
+
+    if (!bytes)
+	return 0;
+
+    for (n_bytes /= 4; n_bytes; vals++, n_bytes--)
+	*vals = (float)rand() / (float)RAND_MAX;
+
+    return (float *)bytes;
+}
+
 void
 a8r8g8b8_to_rgba_np (uint32_t *dst, uint32_t *src, int n_pixels)
 {
@@ -1180,6 +1195,11 @@  static const format_entry_t format_list[] =
      * Aliases are not listed by list_formats ().
      */
 
+/* 128bpp formats */
+    ENTRY (rgba_float),
+/* 96bpp formats */
+    ENTRY (rgb_float),
+
 /* 32bpp formats */
     ENTRY (a8r8g8b8),
     ALIAS (a8r8g8b8,		"8888"),
@@ -1914,6 +1934,10 @@  pixel_checker_init (pixel_checker_t *checker, pixman_format_code_t format)
 
     checker->format = format;
 
+    if (format == PIXMAN_rgba_float ||
+	format == PIXMAN_rgb_float)
+	return;
+
     switch (PIXMAN_FORMAT_TYPE (format))
     {
     case PIXMAN_TYPE_A:
@@ -1970,10 +1994,19 @@  pixel_checker_init (pixel_checker_t *checker, pixman_format_code_t format)
     checker->bw = PIXMAN_FORMAT_B (format);
 }
 
+static void
+pixel_checker_require_uint32_format (const pixel_checker_t *checker)
+{
+    assert (checker->format != PIXMAN_rgba_float &&
+	    checker->format != PIXMAN_rgb_float);
+}
+
 void
 pixel_checker_split_pixel (const pixel_checker_t *checker, uint32_t pixel,
 			   int *a, int *r, int *g, int *b)
 {
+    pixel_checker_require_uint32_format(checker);
+
     *a = (pixel & checker->am) >> checker->as;
     *r = (pixel & checker->rm) >> checker->rs;
     *g = (pixel & checker->gm) >> checker->gs;
@@ -1987,6 +2020,8 @@  pixel_checker_get_masks (const pixel_checker_t *checker,
                          uint32_t              *gm,
                          uint32_t              *bm)
 {
+    pixel_checker_require_uint32_format(checker);
+
     if (am)
         *am = checker->am;
     if (rm)
@@ -2003,6 +2038,8 @@  pixel_checker_convert_pixel_to_color (const pixel_checker_t *checker,
 {
     int a, r, g, b;
 
+    pixel_checker_require_uint32_format(checker);
+
     pixel_checker_split_pixel (checker, pixel, &a, &r, &g, &b);
 
     if (checker->am == 0)
@@ -2078,6 +2115,8 @@  void
 pixel_checker_get_max (const pixel_checker_t *checker, color_t *color,
 		       int *am, int *rm, int *gm, int *bm)
 {
+    pixel_checker_require_uint32_format(checker);
+
     get_limits (checker, DEVIATION, color, am, rm, gm, bm);
 }
 
@@ -2085,6 +2124,8 @@  void
 pixel_checker_get_min (const pixel_checker_t *checker, color_t *color,
 		       int *am, int *rm, int *gm, int *bm)
 {
+    pixel_checker_require_uint32_format(checker);
+
     get_limits (checker, - DEVIATION, color, am, rm, gm, bm);
 }
 
@@ -2096,6 +2137,8 @@  pixel_checker_check (const pixel_checker_t *checker, uint32_t pixel,
     int32_t ai, ri, gi, bi;
     pixman_bool_t result;
 
+    pixel_checker_require_uint32_format(checker);
+
     pixel_checker_get_min (checker, color, &a_lo, &r_lo, &g_lo, &b_lo);
     pixel_checker_get_max (checker, color, &a_hi, &r_hi, &g_hi, &b_hi);
     pixel_checker_split_pixel (checker, pixel, &ai, &ri, &gi, &bi);
@@ -2108,3 +2151,36 @@  pixel_checker_check (const pixel_checker_t *checker, uint32_t pixel,
 
     return result;
 }
+
+static void
+color_limits (const pixel_checker_t *checker,
+	      double limit, const color_t *color, color_t *out)
+{
+    if (PIXMAN_FORMAT_A(checker->format))
+	out->a = color->a + limit;
+    else
+	out->a = 1.;
+
+    out->r = color->r + limit;
+    out->g = color->g + limit;
+    out->b = color->b + limit;
+}
+
+pixman_bool_t
+pixel_checker_check_color (const pixel_checker_t *checker,
+			   const color_t *actual, const color_t *reference)
+{
+    color_t min, max;
+    pixman_bool_t result;
+
+    color_limits(checker, -DEVIATION, reference, &min);
+    color_limits(checker, DEVIATION, reference, &max);
+
+    result =
+	actual->a >= min.a && actual->a <= max.a &&
+	actual->r >= min.r && actual->r <= max.r &&
+	actual->g >= min.g && actual->g <= max.g &&
+	actual->b >= min.b && actual->b <= max.b;
+
+    return result;
+}
diff --git a/test/utils.h b/test/utils.h
index e299d1d066ed..e5ac945ab3f8 100644
--- a/test/utils.h
+++ b/test/utils.h
@@ -119,6 +119,8 @@  fence_get_page_size ();
 /* Generate n_bytes random bytes in fence_malloced memory */
 uint8_t *
 make_random_bytes (int n_bytes);
+float *
+make_random_floats (int n_bytes);
 
 /* Return current time in seconds */
 double

Comments

On Mon, 11 Jun 2018 13:34:50 +0200
Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:

> Pixman is already using the floating point formats internally, expose
> this capability in case someone wants to support higher bit per
> component formats.

Pixman is currently using floating point formats for temporary scanline
buffers in the cases when better accuracy is needed (primarily for
PDF operators).

But this does not automatically mean that everything is perfectly
ready to use floating point formats for pixman images. Some work
may be required to implement everything correctly.

> This is useful for igt which depends on cairo to do the rendering.
> It can use it to convert floats internally to planar Y'CbCr formats,
> or to F16.
>
> Changes since v1:
> - Use RGBA 128 bits and RGB 96 bits memory layouts, to better match the opengl format.

Does it make sense to actually support both layouts?

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  pixman/pixman-access.c | 128 ++++++++++++++++++++++++++++++++++++++++-
>  pixman/pixman.h        |  36 ++++++++----
>  test/alphamap.c        |  73 ++++++++++++++++-------
>  test/utils.c           |  76 ++++++++++++++++++++++++
>  test/utils.h           |   2 +
>  5 files changed, 282 insertions(+), 33 deletions(-)

Thanks for trying to add these new floating point image formats to at
least one test from the pixman test suite. But we still need a much
better test coverage.

I think that making sure that at least stress-test does not fail
would be a good start. Currently stress-test segfaults if I add
PIXMAN_rgba_float or PIXMAN_rgb_float to the list of the tested
formats in its source code.

Updating existing tests and/or adding new tests for floating point
formats should be preferably done in separate commits.

> diff --git a/pixman/pixman-access.c b/pixman/pixman-access.c
> index 4f0642d77785..10fa049becf4 100644
> --- a/pixman/pixman-access.c
> +++ b/pixman/pixman-access.c
> @@ -642,6 +642,48 @@ fetch_scanline_a2r10g10b10_float (bits_image_t *  image,
>  }
>  
>  /* Expects a float buffer */
> +#ifndef PIXMAN_FB_ACCESSORS
> +static void
> +fetch_scanline_rgb_float_float (bits_image_t   *image,
> +				int             x,
> +				int             y,
> +				int             width,
> +				uint32_t *      b,
> +				const uint32_t *mask)
> +{
> +    const float *bits = (float *)image->bits + y * image->rowstride;
> +    const float *pixel = bits + x * 3;
> +    argb_t *buffer = (argb_t *)b;
> +
> +    for (; width--; buffer++) {
> +	buffer->r = *pixel++;
> +	buffer->g = *pixel++;
> +	buffer->b = *pixel++;
> +	buffer->a = 1.f;
> +    }
> +}
> +
> +static void
> +fetch_scanline_rgba_float_float (bits_image_t   *image,
> +				int             x,
> +				int             y,
> +				int             width,
> +				uint32_t *      b,
> +				const uint32_t *mask)
> +{
> +    const float *bits = (float *)image->bits + y * image->rowstride;
> +    const float *pixel = bits + x * 4;
> +    argb_t *buffer = (argb_t *)b;
> +
> +    for (; width--; buffer++) {
> +	buffer->r = *pixel++;
> +	buffer->g = *pixel++;
> +	buffer->b = *pixel++;
> +	buffer->a = *pixel++;
> +    }
> +}
> +#endif
> +
>  static void
>  fetch_scanline_x2r10g10b10_float (bits_image_t   *image,
>  				  int             x,
> @@ -805,6 +847,40 @@ fetch_scanline_yv12 (bits_image_t   *image,
>  
>  /**************************** Pixel wise fetching *****************************/
>  
> +#ifndef PIXMAN_FB_ACCESSORS
> +static argb_t
> +fetch_pixel_rgb_float_float (bits_image_t *image,
> +			     int	    offset,
> +			     int	    line)
> +{
> +    float *bits = (float *)image->bits + line * image->rowstride;
> +    argb_t argb;
> +
> +    argb.r = bits[offset * 3];
> +    argb.g = bits[offset * 3 + 1];
> +    argb.b = bits[offset * 3 + 2];
> +    argb.a = 1.f;
> +
> +    return argb;
> +}
> +
> +static argb_t
> +fetch_pixel_rgba_float_float (bits_image_t *image,
> +			      int	    offset,
> +			      int	    line)
> +{
> +    float *bits = (float *)image->bits + line * image->rowstride;
> +    argb_t argb;
> +
> +    argb.r = bits[offset * 4];
> +    argb.g = bits[offset * 4 + 1];
> +    argb.b = bits[offset * 4 + 2];
> +    argb.a = bits[offset * 4 + 3];
> +
> +    return argb;
> +}
> +#endif
> +
>  static argb_t
>  fetch_pixel_x2r10g10b10_float (bits_image_t *image,
>  			       int	   offset,
> @@ -962,6 +1038,45 @@ fetch_pixel_yv12 (bits_image_t *image,
>  
>  /*********************************** Store ************************************/
>  
> +#ifndef PIXMAN_FB_ACCESSORS
> +static void
> +store_scanline_rgba_float_float (bits_image_t *  image,
> +				 int             x,
> +				 int             y,
> +				 int             width,
> +				 const uint32_t *v)
> +{
> +    float *bits = (float *)image->bits + image->rowstride * y + 4 * x;
> +    const argb_t *values = (argb_t *)v;
> +
> +    for (; width; width--, values++)
> +    {
> +	*bits++ = values->r;
> +	*bits++ = values->g;
> +	*bits++ = values->b;
> +	*bits++ = values->a;
> +    }
> +}
> +
> +static void
> +store_scanline_rgb_float_float (bits_image_t *  image,
> +				int             x,
> +				int             y,
> +				int             width,
> +				const uint32_t *v)
> +{
> +    float *bits = (float *)image->bits + image->rowstride * y + 3 * x;
> +    const argb_t *values = (argb_t *)v;
> +
> +    for (; width; width--, values++)
> +    {
> +	*bits++ = values->r;
> +	*bits++ = values->g;
> +	*bits++ = values->b;
> +    }
> +}
> +#endif
> +
>  static void
>  store_scanline_a2r10g10b10_float (bits_image_t *  image,
>  				  int             x,
> @@ -1351,7 +1466,18 @@ static const format_info_t accessors[] =
>      FORMAT_INFO (g1),
>      
>  /* Wide formats */
> -    
> +#ifndef PIXMAN_FB_ACCESSORS
> +    { PIXMAN_rgba_float,
> +      NULL, fetch_scanline_rgba_float_float,
> +      fetch_pixel_generic_lossy_32, fetch_pixel_rgba_float_float,
> +      NULL, store_scanline_rgba_float_float },
> +
> +    { PIXMAN_rgb_float,
> +      NULL, fetch_scanline_rgb_float_float,
> +      fetch_pixel_generic_lossy_32, fetch_pixel_rgb_float_float,
> +      NULL, store_scanline_rgb_float_float },
> +#endif
> +
>      { PIXMAN_a2r10g10b10,
>        NULL, fetch_scanline_a2r10g10b10_float,
>        fetch_pixel_generic_lossy_32, fetch_pixel_a2r10g10b10_float,
> diff --git a/pixman/pixman.h b/pixman/pixman.h
> index 509ba5e534a8..c376278c83e7 100644
> --- a/pixman/pixman.h
> +++ b/pixman/pixman.h
> @@ -647,19 +647,28 @@ struct pixman_indexed
>   * sample implementation allows only packed RGB and GBR
>   * representations for data to simplify software rendering,
>   */
> -#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	(((bpp) << 24) |  \
> +#define PIXMAN_FORMAT_PACK_BPP(bpp) ((bpp) <= 64 || (bpp) == 96 ? (bpp) : \
> +				     (bpp) == 128 ? 65 : 0)
> +#define PIXMAN_FORMAT_UNPACK_BPP(bpp) ((bpp) <= 64 || (bpp) == 96 ? (bpp) : \
> +				       (bpp) == 65 ? 128 : 0)

We have 8 bits for encoding the bit depth and it should be enough
for storing 96 and 128 directly. Maybe with a cast to unsigned
data type.

> +#define PIXMAN_FORMAT_PACKC(val) ((val) <= 10 ? (val) : \
> +				 (val) == 32 ? 11 : 0)
> +#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \
> +				    (val) == 11 ? 32 : 0)

I was initially not very happy about having these extra branches,
because they potentially may affect the performance of this code:
    https://cgit.freedesktop.org/pixman/tree/pixman/pixman-access.c?h=pixman-0.34.0#n197

And it is used on the general path to fetch/store various uncommon
image formats. This code is usually a performance bottleneck if such
formats are used:

  $ perf record ./lowlevel_blt_bench -m 1000 add_2222_2222_2222
  $ perf report

    79.52%  lowlevel-blt-be  lowlevel-blt-bench  [.] fetch_scanline_a2r2g2b2
     8.98%  lowlevel-blt-be  lowlevel-blt-bench  [.] store_scanline_a2r2g2b2
     4.93%  lowlevel-blt-be  lowlevel-blt-bench  [.] sse2_combine_add_u
     1.32%  lowlevel-blt-be  lowlevel-blt-bench  [.] _pixman_implementation_iter_init

But the compiler at least is able to inline everything and optimize out
the branches. So probably it's okay.

I'm just trying to be careful. Because after we add these new format
identifiers and macros to pixman.h, there will be no way to change them
later without breaking compatibility.

> +#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	((PIXMAN_FORMAT_PACK_BPP(bpp) << 24) |  \
>  					 ((type) << 16) | \
> -					 ((a) << 12) |	  \
> -					 ((r) << 8) |	  \
> -					 ((g) << 4) |	  \
> -					 ((b)))
> +					 (PIXMAN_FORMAT_PACKC(a) << 12) |	  \
> +					 (PIXMAN_FORMAT_PACKC(r) << 8) |	  \
> +					 (PIXMAN_FORMAT_PACKC(g) << 4) |	  \
> +					 (PIXMAN_FORMAT_PACKC(b)))
>  
> -#define PIXMAN_FORMAT_BPP(f)	(((f) >> 24)       )
> +#define PIXMAN_FORMAT_BPP(f)	PIXMAN_FORMAT_UNPACK_BPP((f) >> 24)
>  #define PIXMAN_FORMAT_TYPE(f)	(((f) >> 16) & 0xff)
> -#define PIXMAN_FORMAT_A(f)	(((f) >> 12) & 0x0f)
> -#define PIXMAN_FORMAT_R(f)	(((f) >>  8) & 0x0f)
> -#define PIXMAN_FORMAT_G(f)	(((f) >>  4) & 0x0f)
> -#define PIXMAN_FORMAT_B(f)	(((f)      ) & 0x0f)
> +#define PIXMAN_FORMAT_A(f)	PIXMAN_FORMAT_UNPACKC(((f) >> 12) & 0x0f)
> +#define PIXMAN_FORMAT_R(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  8) & 0x0f)
> +#define PIXMAN_FORMAT_G(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  4) & 0x0f)
> +#define PIXMAN_FORMAT_B(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  0) & 0x0f)
>  #define PIXMAN_FORMAT_RGB(f)	(((f)      ) & 0xfff)
>  #define PIXMAN_FORMAT_VIS(f)	(((f)      ) & 0xffff)
>  #define PIXMAN_FORMAT_DEPTH(f)	(PIXMAN_FORMAT_A(f) +	\
> @@ -685,8 +694,13 @@ struct pixman_indexed
>  	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_BGRA ||	\
>  	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_RGBA)
>  
> -/* 32bpp formats */
>  typedef enum {
> +/* 128bpp formats */
> +    PIXMAN_rgba_float =	PIXMAN_FORMAT(128,PIXMAN_TYPE_ARGB,32,32,32,32),
> +/* 96bpp formats */
> +    PIXMAN_rgb_float =	PIXMAN_FORMAT(96,PIXMAN_TYPE_ARGB,0,32,32,32),

The PIXMAN_TYPE_ARGB type does not seem like a good choice for these
formats. Your  fetch_scanline_rgb_float_float function is doing the
following:

    for (; width--; buffer++) {
	buffer->r = *pixel++;
	buffer->g = *pixel++;
	buffer->b = *pixel++;
	buffer->a = 1.f;
    }

Such data layout can be interpreted as either RGBA or ABGR (depending
on whether the system is big or little endian), but it can't be ARGB.

Right now pixman only supports pixel formats with sizes up to 32
bits. And pixels are always loaded or stored as 32-bit values. The
PIXMAN_a8r8g8b8 format means that the alpha channel is located in
the highest 8-bit part of a 32-bit integer. On little endian systems
alpha channel has offset 3 in memory (the BGRA byte layout) and on
big endian systems it has offset 0 in memory (the ARGB byte layout).

Introducing 128-bit pixels makes everything a little bit more
complicated. How are we going to handle big and little endian
systems? Should we interpret it as an array of 4 float values,
where the alpha channel has the same array index on both big and
little endian systems?

> +
> +/* 32bpp formats */
>      PIXMAN_a8r8g8b8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ARGB,8,8,8,8),
>      PIXMAN_x8r8g8b8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ARGB,0,8,8,8),
>      PIXMAN_a8b8g8r8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ABGR,8,8,8,8),
> diff --git a/test/alphamap.c b/test/alphamap.c
> index 4d09076fbcf3..150d33eed5b5 100644
> --- a/test/alphamap.c
> +++ b/test/alphamap.c
> @@ -10,7 +10,8 @@ static const pixman_format_code_t formats[] =
>      PIXMAN_a8r8g8b8,
>      PIXMAN_a2r10g10b10,
>      PIXMAN_a4r4g4b4,
> -    PIXMAN_a8
> +    PIXMAN_a8,
> +    PIXMAN_rgba_float,
>  };
>  
>  static const pixman_format_code_t alpha_formats[] =
> @@ -18,7 +19,8 @@ static const pixman_format_code_t alpha_formats[] =
>      PIXMAN_null,
>      PIXMAN_a8,
>      PIXMAN_a2r10g10b10,
> -    PIXMAN_a4r4g4b4
> +    PIXMAN_a4r4g4b4,
> +    PIXMAN_rgba_float,
>  };
>  
>  static const int origins[] =
> @@ -41,7 +43,10 @@ make_image (pixman_format_code_t format)
>      uint8_t bpp = PIXMAN_FORMAT_BPP (format) / 8;
>      pixman_image_t *image;
>  
> -    bits = (uint32_t *)make_random_bytes (WIDTH * HEIGHT * bpp);
> +    if (format != PIXMAN_rgba_float)
> +	bits = (uint32_t *)make_random_bytes (WIDTH * HEIGHT * bpp);
> +    else
> +	bits = (uint32_t *)make_random_floats (WIDTH * HEIGHT * bpp);
>  
>      image = pixman_image_create_bits (format, WIDTH, HEIGHT, bits, WIDTH * bpp);
>  
> @@ -51,11 +56,11 @@ make_image (pixman_format_code_t format)
>      return image;
>  }
>  
> -static uint8_t
> +static float
>  get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>  {
>      uint8_t *bits;
> -    uint8_t r;
> +    uint32_t r;
>  
>      if (image->common.alpha_map)
>      {
> @@ -69,7 +74,7 @@ get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>  	}
>  	else
>  	{
> -	    return 0;
> +	    return 0.f;
>  	}
>      }
>  
> @@ -78,28 +83,32 @@ get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>      if (image->bits.format == PIXMAN_a8)
>      {
>  	r = bits[y * WIDTH + x];
> +	return r / 255.f;
>      }
>      else if (image->bits.format == PIXMAN_a2r10g10b10)
>      {
>  	r = ((uint32_t *)bits)[y * WIDTH + x] >> 30;
> -	r |= r << 2;
> -	r |= r << 4;
> +	return r / 3.f;
>      }
>      else if (image->bits.format == PIXMAN_a8r8g8b8)
>      {
>  	r = ((uint32_t *)bits)[y * WIDTH + x] >> 24;
> +	return r / 255.f;
>      }
>      else if (image->bits.format == PIXMAN_a4r4g4b4)
>      {
>  	r = ((uint16_t *)bits)[y * WIDTH + x] >> 12;
> -	r |= r << 4;
> +	return r / 15.f;
> +    }
> +    else if (image->bits.format == PIXMAN_rgba_float)
> +    {
> +	return ((float *)bits)[y * WIDTH * 4 + x * 4 + 3];
>      }
>      else
>      {
>  	assert (0);
> +	return 0.f;
>      }
> -
> -    return r;
>  }
>  
>  static uint16_t
> @@ -133,6 +142,11 @@ get_red (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>  	r |= r << 4;
>  	r |= r << 8;
>      }
> +    else if (image->bits.format == PIXMAN_rgba_float)
> +    {
> +	double tmp = ((float *)bits)[y * WIDTH * 4 + x * 4];
> +	return tmp * 65535.;
> +    }
>      else
>      {
>  	assert (0);
> @@ -141,6 +155,23 @@ get_red (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>      return r;
>  }
>  
> +static float get_alpha_err(pixman_format_code_t sf, pixman_format_code_t saf,
> +			   pixman_format_code_t df, pixman_format_code_t daf)
> +{
> +	pixman_format_code_t s = saf != PIXMAN_null ? saf : sf;
> +	pixman_format_code_t d = daf != PIXMAN_null ? daf : df;
> +
> +	/* There are cases where we go through the 8 bit compositing
> +	 * path even with 10bpc and higher formats.
> +	 */
> +	if (PIXMAN_FORMAT_A(s) == PIXMAN_FORMAT_A(d))
> +		return 1.f / 255.f;
> +	else if (PIXMAN_FORMAT_A(s) > PIXMAN_FORMAT_A(d))
> +		return 1.f / ((1 << PIXMAN_FORMAT_A(d)) - 1);
> +	else
> +		return 1.f / ((1 << PIXMAN_FORMAT_A(s)) - 1);
> +}
> +
>  static int
>  run_test (int s, int d, int sa, int da, int soff, int doff)
>  {
> @@ -151,15 +182,11 @@ run_test (int s, int d, int sa, int da, int soff, int doff)
>      pixman_image_t *src, *dst, *orig_dst, *alpha, *orig_alpha;
>      pixman_transform_t t1;
>      int j, k;
> -    int n_alpha_bits, n_red_bits;
> +    int n_red_bits;
>  
>      soff = origins[soff];
>      doff = origins[doff];
>  
> -    n_alpha_bits = PIXMAN_FORMAT_A (df);
> -    if (daf != PIXMAN_null)
> -	n_alpha_bits = PIXMAN_FORMAT_A (daf);
> -
>      n_red_bits = PIXMAN_FORMAT_R (df);
>  
>      /* Source */
> @@ -211,21 +238,25 @@ run_test (int s, int d, int sa, int da, int soff, int doff)
>      {
>  	for (k = MAX (doff, 0); k < MIN (WIDTH, WIDTH + doff); ++k)
>  	{
> -	    uint8_t sa, da, oda, refa;
> +	    float sa, da, oda, refa;
>  	    uint16_t sr, dr, odr, refr;
> +	    float err;
> +
> +	    err = get_alpha_err(sf, saf, df, daf);
>  
>  	    sa = get_alpha (src, k, j, soff, soff);
>  	    da = get_alpha (dst, k, j, doff, doff);
>  	    oda = get_alpha (orig_dst, k, j, doff, doff);
>  
> -	    if (sa + oda > 255)
> -		refa = 255;
> +	    if (sa + oda > 1.f)
> +		refa = 1.f;
>  	    else
>  		refa = sa + oda;
>  
> -	    if (da >> (8 - n_alpha_bits) != refa >> (8 - n_alpha_bits))
> +	    if (da - err > refa ||
> +	        da + err < refa)

This changes the behaviour of the alphamap test for the existing
image formats by making the test less strict.

>  	    {
> -		printf ("\nWrong alpha value at (%d, %d). Should be 0x%x; got 0x%x. Source was 0x%x, original dest was 0x%x\n",
> +		printf ("\nWrong alpha value at (%d, %d). Should be %g; got %g. Source was %g, original dest was %g\n",
>  			k, j, refa, da, sa, oda);
>  
>  		printf ("src: %s, alpha: %s, origin %d %d\ndst: %s, alpha: %s, origin: %d %d\n\n",
> diff --git a/test/utils.c b/test/utils.c
> index f8e42a5d3f3d..4eeb068497ec 100644
> --- a/test/utils.c
> +++ b/test/utils.c
> @@ -595,6 +595,21 @@ make_random_bytes (int n_bytes)
>      return bytes;
>  }
>  
> +float *
> +make_random_floats (int n_bytes)
> +{
> +    uint8_t *bytes = fence_malloc (n_bytes);
> +    float *vals = (float *)bytes;
> +
> +    if (!bytes)
> +	return 0;
> +
> +    for (n_bytes /= 4; n_bytes; vals++, n_bytes--)
> +	*vals = (float)rand() / (float)RAND_MAX;
> +
> +    return (float *)bytes;
> +}
> +
>  void
>  a8r8g8b8_to_rgba_np (uint32_t *dst, uint32_t *src, int n_pixels)
>  {
> @@ -1180,6 +1195,11 @@ static const format_entry_t format_list[] =
>       * Aliases are not listed by list_formats ().
>       */
>  
> +/* 128bpp formats */
> +    ENTRY (rgba_float),
> +/* 96bpp formats */
> +    ENTRY (rgb_float),
> +
>  /* 32bpp formats */
>      ENTRY (a8r8g8b8),
>      ALIAS (a8r8g8b8,		"8888"),
> @@ -1914,6 +1934,10 @@ pixel_checker_init (pixel_checker_t *checker, pixman_format_code_t format)
>  
>      checker->format = format;
>  
> +    if (format == PIXMAN_rgba_float ||
> +	format == PIXMAN_rgb_float)
> +	return;
> +
>      switch (PIXMAN_FORMAT_TYPE (format))
>      {
>      case PIXMAN_TYPE_A:
> @@ -1970,10 +1994,19 @@ pixel_checker_init (pixel_checker_t *checker, pixman_format_code_t format)
>      checker->bw = PIXMAN_FORMAT_B (format);
>  }
>  
> +static void
> +pixel_checker_require_uint32_format (const pixel_checker_t *checker)
> +{
> +    assert (checker->format != PIXMAN_rgba_float &&
> +	    checker->format != PIXMAN_rgb_float);
> +}
> +
>  void
>  pixel_checker_split_pixel (const pixel_checker_t *checker, uint32_t pixel,
>  			   int *a, int *r, int *g, int *b)
>  {
> +    pixel_checker_require_uint32_format(checker);
> +
>      *a = (pixel & checker->am) >> checker->as;
>      *r = (pixel & checker->rm) >> checker->rs;
>      *g = (pixel & checker->gm) >> checker->gs;
> @@ -1987,6 +2020,8 @@ pixel_checker_get_masks (const pixel_checker_t *checker,
>                           uint32_t              *gm,
>                           uint32_t              *bm)
>  {
> +    pixel_checker_require_uint32_format(checker);
> +
>      if (am)
>          *am = checker->am;
>      if (rm)
> @@ -2003,6 +2038,8 @@ pixel_checker_convert_pixel_to_color (const pixel_checker_t *checker,
>  {
>      int a, r, g, b;
>  
> +    pixel_checker_require_uint32_format(checker);
> +
>      pixel_checker_split_pixel (checker, pixel, &a, &r, &g, &b);
>  
>      if (checker->am == 0)
> @@ -2078,6 +2115,8 @@ void
>  pixel_checker_get_max (const pixel_checker_t *checker, color_t *color,
>  		       int *am, int *rm, int *gm, int *bm)
>  {
> +    pixel_checker_require_uint32_format(checker);
> +
>      get_limits (checker, DEVIATION, color, am, rm, gm, bm);
>  }
>  
> @@ -2085,6 +2124,8 @@ void
>  pixel_checker_get_min (const pixel_checker_t *checker, color_t *color,
>  		       int *am, int *rm, int *gm, int *bm)
>  {
> +    pixel_checker_require_uint32_format(checker);
> +
>      get_limits (checker, - DEVIATION, color, am, rm, gm, bm);
>  }
>  
> @@ -2096,6 +2137,8 @@ pixel_checker_check (const pixel_checker_t *checker, uint32_t pixel,
>      int32_t ai, ri, gi, bi;
>      pixman_bool_t result;
>  
> +    pixel_checker_require_uint32_format(checker);
> +
>      pixel_checker_get_min (checker, color, &a_lo, &r_lo, &g_lo, &b_lo);
>      pixel_checker_get_max (checker, color, &a_hi, &r_hi, &g_hi, &b_hi);
>      pixel_checker_split_pixel (checker, pixel, &ai, &ri, &gi, &bi);
> @@ -2108,3 +2151,36 @@ pixel_checker_check (const pixel_checker_t *checker, uint32_t pixel,
>  
>      return result;
>  }
> +
> +static void
> +color_limits (const pixel_checker_t *checker,
> +	      double limit, const color_t *color, color_t *out)
> +{
> +    if (PIXMAN_FORMAT_A(checker->format))
> +	out->a = color->a + limit;
> +    else
> +	out->a = 1.;
> +
> +    out->r = color->r + limit;
> +    out->g = color->g + limit;
> +    out->b = color->b + limit;
> +}
> +
> +pixman_bool_t
> +pixel_checker_check_color (const pixel_checker_t *checker,
> +			   const color_t *actual, const color_t *reference)
> +{
> +    color_t min, max;
> +    pixman_bool_t result;
> +
> +    color_limits(checker, -DEVIATION, reference, &min);
> +    color_limits(checker, DEVIATION, reference, &max);
> +
> +    result =
> +	actual->a >= min.a && actual->a <= max.a &&
> +	actual->r >= min.r && actual->r <= max.r &&
> +	actual->g >= min.g && actual->g <= max.g &&
> +	actual->b >= min.b && actual->b <= max.b;
> +
> +    return result;
> +}
> diff --git a/test/utils.h b/test/utils.h
> index e299d1d066ed..e5ac945ab3f8 100644
> --- a/test/utils.h
> +++ b/test/utils.h
> @@ -119,6 +119,8 @@ fence_get_page_size ();
>  /* Generate n_bytes random bytes in fence_malloced memory */
>  uint8_t *
>  make_random_bytes (int n_bytes);
> +float *
> +make_random_floats (int n_bytes);
>  
>  /* Return current time in seconds */
>  double
Op 12-06-18 om 16:24 schreef Siarhei Siamashka:
> On Mon, 11 Jun 2018 13:34:50 +0200
> Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>
>> Pixman is already using the floating point formats internally, expose
>> this capability in case someone wants to support higher bit per
>> component formats.
> Pixman is currently using floating point formats for temporary scanline
> buffers in the cases when better accuracy is needed (primarily for
> PDF operators).
>
> But this does not automatically mean that everything is perfectly
> ready to use floating point formats for pixman images. Some work
> may be required to implement everything correctly.
Yeah noticed when running the tests that something is still clipping to the nearest 8 bit. :(


>> This is useful for igt which depends on cairo to do the rendering.
>> It can use it to convert floats internally to planar Y'CbCr formats,
>> or to F16.
>>
>> Changes since v1:
>> - Use RGBA 128 bits and RGB 96 bits memory layouts, to better match the opengl format.
> Does it make sense to actually support both layouts?
Don't know, I wouldn't mind storing it as in the same layout, with alpha = 1.
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  pixman/pixman-access.c | 128 ++++++++++++++++++++++++++++++++++++++++-
>>  pixman/pixman.h        |  36 ++++++++----
>>  test/alphamap.c        |  73 ++++++++++++++++-------
>>  test/utils.c           |  76 ++++++++++++++++++++++++
>>  test/utils.h           |   2 +
>>  5 files changed, 282 insertions(+), 33 deletions(-)
> Thanks for trying to add these new floating point image formats to at
> least one test from the pixman test suite. But we still need a much
> better test coverage.
>
> I think that making sure that at least stress-test does not fail
> would be a good start. Currently stress-test segfaults if I add
> PIXMAN_rgba_float or PIXMAN_rgb_float to the list of the tested
> formats in its source code.
>
> Updating existing tests and/or adding new tests for floating point
> formats should be preferably done in separate commits.

>> diff --git a/pixman/pixman-access.c b/pixman/pixman-access.c
>> index 4f0642d77785..10fa049becf4 100644
>> --- a/pixman/pixman-access.c
>> +++ b/pixman/pixman-access.c
>> @@ -642,6 +642,48 @@ fetch_scanline_a2r10g10b10_float (bits_image_t *  image,
>>  }
>>  
>>  /* Expects a float buffer */
>> +#ifndef PIXMAN_FB_ACCESSORS
>> +static void
>> +fetch_scanline_rgb_float_float (bits_image_t   *image,
>> +				int             x,
>> +				int             y,
>> +				int             width,
>> +				uint32_t *      b,
>> +				const uint32_t *mask)
>> +{
>> +    const float *bits = (float *)image->bits + y * image->rowstride;
>> +    const float *pixel = bits + x * 3;
>> +    argb_t *buffer = (argb_t *)b;
>> +
>> +    for (; width--; buffer++) {
>> +	buffer->r = *pixel++;
>> +	buffer->g = *pixel++;
>> +	buffer->b = *pixel++;
>> +	buffer->a = 1.f;
>> +    }
>> +}
>> +
>> +static void
>> +fetch_scanline_rgba_float_float (bits_image_t   *image,
>> +				int             x,
>> +				int             y,
>> +				int             width,
>> +				uint32_t *      b,
>> +				const uint32_t *mask)
>> +{
>> +    const float *bits = (float *)image->bits + y * image->rowstride;
>> +    const float *pixel = bits + x * 4;
>> +    argb_t *buffer = (argb_t *)b;
>> +
>> +    for (; width--; buffer++) {
>> +	buffer->r = *pixel++;
>> +	buffer->g = *pixel++;
>> +	buffer->b = *pixel++;
>> +	buffer->a = *pixel++;
>> +    }
>> +}
>> +#endif
>> +
>>  static void
>>  fetch_scanline_x2r10g10b10_float (bits_image_t   *image,
>>  				  int             x,
>> @@ -805,6 +847,40 @@ fetch_scanline_yv12 (bits_image_t   *image,
>>  
>>  /**************************** Pixel wise fetching *****************************/
>>  
>> +#ifndef PIXMAN_FB_ACCESSORS
>> +static argb_t
>> +fetch_pixel_rgb_float_float (bits_image_t *image,
>> +			     int	    offset,
>> +			     int	    line)
>> +{
>> +    float *bits = (float *)image->bits + line * image->rowstride;
>> +    argb_t argb;
>> +
>> +    argb.r = bits[offset * 3];
>> +    argb.g = bits[offset * 3 + 1];
>> +    argb.b = bits[offset * 3 + 2];
>> +    argb.a = 1.f;
>> +
>> +    return argb;
>> +}
>> +
>> +static argb_t
>> +fetch_pixel_rgba_float_float (bits_image_t *image,
>> +			      int	    offset,
>> +			      int	    line)
>> +{
>> +    float *bits = (float *)image->bits + line * image->rowstride;
>> +    argb_t argb;
>> +
>> +    argb.r = bits[offset * 4];
>> +    argb.g = bits[offset * 4 + 1];
>> +    argb.b = bits[offset * 4 + 2];
>> +    argb.a = bits[offset * 4 + 3];
>> +
>> +    return argb;
>> +}
>> +#endif
>> +
>>  static argb_t
>>  fetch_pixel_x2r10g10b10_float (bits_image_t *image,
>>  			       int	   offset,
>> @@ -962,6 +1038,45 @@ fetch_pixel_yv12 (bits_image_t *image,
>>  
>>  /*********************************** Store ************************************/
>>  
>> +#ifndef PIXMAN_FB_ACCESSORS
>> +static void
>> +store_scanline_rgba_float_float (bits_image_t *  image,
>> +				 int             x,
>> +				 int             y,
>> +				 int             width,
>> +				 const uint32_t *v)
>> +{
>> +    float *bits = (float *)image->bits + image->rowstride * y + 4 * x;
>> +    const argb_t *values = (argb_t *)v;
>> +
>> +    for (; width; width--, values++)
>> +    {
>> +	*bits++ = values->r;
>> +	*bits++ = values->g;
>> +	*bits++ = values->b;
>> +	*bits++ = values->a;
>> +    }
>> +}
>> +
>> +static void
>> +store_scanline_rgb_float_float (bits_image_t *  image,
>> +				int             x,
>> +				int             y,
>> +				int             width,
>> +				const uint32_t *v)
>> +{
>> +    float *bits = (float *)image->bits + image->rowstride * y + 3 * x;
>> +    const argb_t *values = (argb_t *)v;
>> +
>> +    for (; width; width--, values++)
>> +    {
>> +	*bits++ = values->r;
>> +	*bits++ = values->g;
>> +	*bits++ = values->b;
>> +    }
>> +}
>> +#endif
>> +
>>  static void
>>  store_scanline_a2r10g10b10_float (bits_image_t *  image,
>>  				  int             x,
>> @@ -1351,7 +1466,18 @@ static const format_info_t accessors[] =
>>      FORMAT_INFO (g1),
>>      
>>  /* Wide formats */
>> -    
>> +#ifndef PIXMAN_FB_ACCESSORS
>> +    { PIXMAN_rgba_float,
>> +      NULL, fetch_scanline_rgba_float_float,
>> +      fetch_pixel_generic_lossy_32, fetch_pixel_rgba_float_float,
>> +      NULL, store_scanline_rgba_float_float },
>> +
>> +    { PIXMAN_rgb_float,
>> +      NULL, fetch_scanline_rgb_float_float,
>> +      fetch_pixel_generic_lossy_32, fetch_pixel_rgb_float_float,
>> +      NULL, store_scanline_rgb_float_float },
>> +#endif
>> +
>>      { PIXMAN_a2r10g10b10,
>>        NULL, fetch_scanline_a2r10g10b10_float,
>>        fetch_pixel_generic_lossy_32, fetch_pixel_a2r10g10b10_float,
>> diff --git a/pixman/pixman.h b/pixman/pixman.h
>> index 509ba5e534a8..c376278c83e7 100644
>> --- a/pixman/pixman.h
>> +++ b/pixman/pixman.h
>> @@ -647,19 +647,28 @@ struct pixman_indexed
>>   * sample implementation allows only packed RGB and GBR
>>   * representations for data to simplify software rendering,
>>   */
>> -#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	(((bpp) << 24) |  \
>> +#define PIXMAN_FORMAT_PACK_BPP(bpp) ((bpp) <= 64 || (bpp) == 96 ? (bpp) : \
>> +				     (bpp) == 128 ? 65 : 0)
>> +#define PIXMAN_FORMAT_UNPACK_BPP(bpp) ((bpp) <= 64 || (bpp) == 96 ? (bpp) : \
>> +				       (bpp) == 65 ? 128 : 0)
> We have 8 bits for encoding the bit depth and it should be enough
> for storing 96 and 128 directly. Maybe with a cast to unsigned
> data type.
Yeah, sign extension was calling me.
>> +#define PIXMAN_FORMAT_PACKC(val) ((val) <= 10 ? (val) : \
>> +				 (val) == 32 ? 11 : 0)
>> +#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \
>> +				    (val) == 11 ? 32 : 0)
> I was initially not very happy about having these extra branches,
> because they potentially may affect the performance of this code:
>     https://cgit.freedesktop.org/pixman/tree/pixman/pixman-access.c?h=pixman-0.34.0#n197
>
> And it is used on the general path to fetch/store various uncommon
> image formats. This code is usually a performance bottleneck if such
> formats are used:
>
>   $ perf record ./lowlevel_blt_bench -m 1000 add_2222_2222_2222
>   $ perf report
>
>     79.52%  lowlevel-blt-be  lowlevel-blt-bench  [.] fetch_scanline_a2r2g2b2
>      8.98%  lowlevel-blt-be  lowlevel-blt-bench  [.] store_scanline_a2r2g2b2
>      4.93%  lowlevel-blt-be  lowlevel-blt-bench  [.] sse2_combine_add_u
>      1.32%  lowlevel-blt-be  lowlevel-blt-bench  [.] _pixman_implementation_iter_init
>
> But the compiler at least is able to inline everything and optimize out
> the branches. So probably it's okay.
Yeah that's the idea. :)
> I'm just trying to be careful. Because after we add these new format
> identifiers and macros to pixman.h, there will be no way to change them
> later without breaking compatibility.
Indeed, but there are too few bits to describe it in pixman. :(
>> +#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	((PIXMAN_FORMAT_PACK_BPP(bpp) << 24) |  \
>>  					 ((type) << 16) | \
>> -					 ((a) << 12) |	  \
>> -					 ((r) << 8) |	  \
>> -					 ((g) << 4) |	  \
>> -					 ((b)))
>> +					 (PIXMAN_FORMAT_PACKC(a) << 12) |	  \
>> +					 (PIXMAN_FORMAT_PACKC(r) << 8) |	  \
>> +					 (PIXMAN_FORMAT_PACKC(g) << 4) |	  \
>> +					 (PIXMAN_FORMAT_PACKC(b)))
>>  
>> -#define PIXMAN_FORMAT_BPP(f)	(((f) >> 24)       )
>> +#define PIXMAN_FORMAT_BPP(f)	PIXMAN_FORMAT_UNPACK_BPP((f) >> 24)
>>  #define PIXMAN_FORMAT_TYPE(f)	(((f) >> 16) & 0xff)
>> -#define PIXMAN_FORMAT_A(f)	(((f) >> 12) & 0x0f)
>> -#define PIXMAN_FORMAT_R(f)	(((f) >>  8) & 0x0f)
>> -#define PIXMAN_FORMAT_G(f)	(((f) >>  4) & 0x0f)
>> -#define PIXMAN_FORMAT_B(f)	(((f)      ) & 0x0f)
>> +#define PIXMAN_FORMAT_A(f)	PIXMAN_FORMAT_UNPACKC(((f) >> 12) & 0x0f)
>> +#define PIXMAN_FORMAT_R(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  8) & 0x0f)
>> +#define PIXMAN_FORMAT_G(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  4) & 0x0f)
>> +#define PIXMAN_FORMAT_B(f)	PIXMAN_FORMAT_UNPACKC(((f) >>  0) & 0x0f)
>>  #define PIXMAN_FORMAT_RGB(f)	(((f)      ) & 0xfff)
>>  #define PIXMAN_FORMAT_VIS(f)	(((f)      ) & 0xffff)
>>  #define PIXMAN_FORMAT_DEPTH(f)	(PIXMAN_FORMAT_A(f) +	\
>> @@ -685,8 +694,13 @@ struct pixman_indexed
>>  	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_BGRA ||	\
>>  	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_RGBA)
>>  
>> -/* 32bpp formats */
>>  typedef enum {
>> +/* 128bpp formats */
>> +    PIXMAN_rgba_float =	PIXMAN_FORMAT(128,PIXMAN_TYPE_ARGB,32,32,32,32),
>> +/* 96bpp formats */
>> +    PIXMAN_rgb_float =	PIXMAN_FORMAT(96,PIXMAN_TYPE_ARGB,0,32,32,32),
> The PIXMAN_TYPE_ARGB type does not seem like a good choice for these
> formats. Your  fetch_scanline_rgb_float_float function is doing the
> following:
>
>     for (; width--; buffer++) {
> 	buffer->r = *pixel++;
> 	buffer->g = *pixel++;
> 	buffer->b = *pixel++;
> 	buffer->a = 1.f;
>     }
>
> Such data layout can be interpreted as either RGBA or ABGR (depending
> on whether the system is big or little endian), but it can't be ARGB.
>
> Right now pixman only supports pixel formats with sizes up to 32
> bits. And pixels are always loaded or stored as 32-bit values. The
> PIXMAN_a8r8g8b8 format means that the alpha channel is located in
> the highest 8-bit part of a 32-bit integer. On little endian systems
> alpha channel has offset 3 in memory (the BGRA byte layout) and on
> big endian systems it has offset 0 in memory (the ARGB byte layout).
>
> Introducing 128-bit pixels makes everything a little bit more
> complicated. How are we going to handle big and little endian
> systems? Should we interpret it as an array of 4 float values,
> where the alpha channel has the same array index on both big and
> little endian systems?
I think that's the correct solution. This is more like a struct of 4 separate floats than a single packed value with 4 components.
>> +
>> +/* 32bpp formats */
>>      PIXMAN_a8r8g8b8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ARGB,8,8,8,8),
>>      PIXMAN_x8r8g8b8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ARGB,0,8,8,8),
>>      PIXMAN_a8b8g8r8 =	 PIXMAN_FORMAT(32,PIXMAN_TYPE_ABGR,8,8,8,8),
>> diff --git a/test/alphamap.c b/test/alphamap.c
>> index 4d09076fbcf3..150d33eed5b5 100644
>> --- a/test/alphamap.c
>> +++ b/test/alphamap.c
>> @@ -10,7 +10,8 @@ static const pixman_format_code_t formats[] =
>>      PIXMAN_a8r8g8b8,
>>      PIXMAN_a2r10g10b10,
>>      PIXMAN_a4r4g4b4,
>> -    PIXMAN_a8
>> +    PIXMAN_a8,
>> +    PIXMAN_rgba_float,
>>  };
>>  
>>  static const pixman_format_code_t alpha_formats[] =
>> @@ -18,7 +19,8 @@ static const pixman_format_code_t alpha_formats[] =
>>      PIXMAN_null,
>>      PIXMAN_a8,
>>      PIXMAN_a2r10g10b10,
>> -    PIXMAN_a4r4g4b4
>> +    PIXMAN_a4r4g4b4,
>> +    PIXMAN_rgba_float,
>>  };
>>  
>>  static const int origins[] =
>> @@ -41,7 +43,10 @@ make_image (pixman_format_code_t format)
>>      uint8_t bpp = PIXMAN_FORMAT_BPP (format) / 8;
>>      pixman_image_t *image;
>>  
>> -    bits = (uint32_t *)make_random_bytes (WIDTH * HEIGHT * bpp);
>> +    if (format != PIXMAN_rgba_float)
>> +	bits = (uint32_t *)make_random_bytes (WIDTH * HEIGHT * bpp);
>> +    else
>> +	bits = (uint32_t *)make_random_floats (WIDTH * HEIGHT * bpp);
>>  
>>      image = pixman_image_create_bits (format, WIDTH, HEIGHT, bits, WIDTH * bpp);
>>  
>> @@ -51,11 +56,11 @@ make_image (pixman_format_code_t format)
>>      return image;
>>  }
>>  
>> -static uint8_t
>> +static float
>>  get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>>  {
>>      uint8_t *bits;
>> -    uint8_t r;
>> +    uint32_t r;
>>  
>>      if (image->common.alpha_map)
>>      {
>> @@ -69,7 +74,7 @@ get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>>  	}
>>  	else
>>  	{
>> -	    return 0;
>> +	    return 0.f;
>>  	}
>>      }
>>  
>> @@ -78,28 +83,32 @@ get_alpha (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>>      if (image->bits.format == PIXMAN_a8)
>>      {
>>  	r = bits[y * WIDTH + x];
>> +	return r / 255.f;
>>      }
>>      else if (image->bits.format == PIXMAN_a2r10g10b10)
>>      {
>>  	r = ((uint32_t *)bits)[y * WIDTH + x] >> 30;
>> -	r |= r << 2;
>> -	r |= r << 4;
>> +	return r / 3.f;
>>      }
>>      else if (image->bits.format == PIXMAN_a8r8g8b8)
>>      {
>>  	r = ((uint32_t *)bits)[y * WIDTH + x] >> 24;
>> +	return r / 255.f;
>>      }
>>      else if (image->bits.format == PIXMAN_a4r4g4b4)
>>      {
>>  	r = ((uint16_t *)bits)[y * WIDTH + x] >> 12;
>> -	r |= r << 4;
>> +	return r / 15.f;
>> +    }
>> +    else if (image->bits.format == PIXMAN_rgba_float)
>> +    {
>> +	return ((float *)bits)[y * WIDTH * 4 + x * 4 + 3];
>>      }
>>      else
>>      {
>>  	assert (0);
>> +	return 0.f;
>>      }
>> -
>> -    return r;
>>  }
>>  
>>  static uint16_t
>> @@ -133,6 +142,11 @@ get_red (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>>  	r |= r << 4;
>>  	r |= r << 8;
>>      }
>> +    else if (image->bits.format == PIXMAN_rgba_float)
>> +    {
>> +	double tmp = ((float *)bits)[y * WIDTH * 4 + x * 4];
>> +	return tmp * 65535.;
>> +    }
>>      else
>>      {
>>  	assert (0);
>> @@ -141,6 +155,23 @@ get_red (pixman_image_t *image, int x, int y, int orig_x, int orig_y)
>>      return r;
>>  }
>>  
>> +static float get_alpha_err(pixman_format_code_t sf, pixman_format_code_t saf,
>> +			   pixman_format_code_t df, pixman_format_code_t daf)
>> +{
>> +	pixman_format_code_t s = saf != PIXMAN_null ? saf : sf;
>> +	pixman_format_code_t d = daf != PIXMAN_null ? daf : df;
>> +
>> +	/* There are cases where we go through the 8 bit compositing
>> +	 * path even with 10bpc and higher formats.
>> +	 */
>> +	if (PIXMAN_FORMAT_A(s) == PIXMAN_FORMAT_A(d))
>> +		return 1.f / 255.f;
>> +	else if (PIXMAN_FORMAT_A(s) > PIXMAN_FORMAT_A(d))
>> +		return 1.f / ((1 << PIXMAN_FORMAT_A(d)) - 1);
>> +	else
>> +		return 1.f / ((1 << PIXMAN_FORMAT_A(s)) - 1);
>> +}
>> +
>>  static int
>>  run_test (int s, int d, int sa, int da, int soff, int doff)
>>  {
>> @@ -151,15 +182,11 @@ run_test (int s, int d, int sa, int da, int soff, int doff)
>>      pixman_image_t *src, *dst, *orig_dst, *alpha, *orig_alpha;
>>      pixman_transform_t t1;
>>      int j, k;
>> -    int n_alpha_bits, n_red_bits;
>> +    int n_red_bits;
>>  
>>      soff = origins[soff];
>>      doff = origins[doff];
>>  
>> -    n_alpha_bits = PIXMAN_FORMAT_A (df);
>> -    if (daf != PIXMAN_null)
>> -	n_alpha_bits = PIXMAN_FORMAT_A (daf);
>> -
>>      n_red_bits = PIXMAN_FORMAT_R (df);
>>  
>>      /* Source */
>> @@ -211,21 +238,25 @@ run_test (int s, int d, int sa, int da, int soff, int doff)
>>      {
>>  	for (k = MAX (doff, 0); k < MIN (WIDTH, WIDTH + doff); ++k)
>>  	{
>> -	    uint8_t sa, da, oda, refa;
>> +	    float sa, da, oda, refa;
>>  	    uint16_t sr, dr, odr, refr;
>> +	    float err;
>> +
>> +	    err = get_alpha_err(sf, saf, df, daf);
>>  
>>  	    sa = get_alpha (src, k, j, soff, soff);
>>  	    da = get_alpha (dst, k, j, doff, doff);
>>  	    oda = get_alpha (orig_dst, k, j, doff, doff);
>>  
>> -	    if (sa + oda > 255)
>> -		refa = 255;
>> +	    if (sa + oda > 1.f)
>> +		refa = 1.f;
>>  	    else
>>  		refa = sa + oda;
>>  
>> -	    if (da >> (8 - n_alpha_bits) != refa >> (8 - n_alpha_bits))
>> +	    if (da - err > refa ||
>> +	        da + err < refa)
> This changes the behaviour of the alphamap test for the existing
> image formats by making the test less strict.
Yeah, perhaps we should special case float?
>>  	    {
>> -		printf ("\nWrong alpha value at (%d, %d). Should be 0x%x; got 0x%x. Source was 0x%x, original dest was 0x%x\n",
>> +		printf ("\nWrong alpha value at (%d, %d). Should be %g; got %g. Source was %g, original dest was %g\n",
>>  			k, j, refa, da, sa, oda);
>>  
>>  		printf ("src: %s, alpha: %s, origin %d %d\ndst: %s, alpha: %s, origin: %d %d\n\n",
>> diff --git a/test/utils.c b/test/utils.c
>> index f8e42a5d3f3d..4eeb068497ec 100644
>> --- a/test/utils.c
>> +++ b/test/utils.c
>> @@ -595,6 +595,21 @@ make_random_bytes (int n_bytes)
>>      return bytes;
>>  }
>>  
>> +float *
>> +make_random_floats (int n_bytes)
>> +{
>> +    uint8_t *bytes = fence_malloc (n_bytes);
>> +    float *vals = (float *)bytes;
>> +
>> +    if (!bytes)
>> +	return 0;
>> +
>> +    for (n_bytes /= 4; n_bytes; vals++, n_bytes--)
>> +	*vals = (float)rand() / (float)RAND_MAX;
>> +
>> +    return (float *)bytes;
>> +}
>> +
>>  void
>>  a8r8g8b8_to_rgba_np (uint32_t *dst, uint32_t *src, int n_pixels)
>>  {
>> @@ -1180,6 +1195,11 @@ static const format_entry_t format_list[] =
>>       * Aliases are not listed by list_formats ().
>>       */
>>  
>> +/* 128bpp formats */
>> +    ENTRY (rgba_float),
>> +/* 96bpp formats */
>> +    ENTRY (rgb_float),
>> +
>>  /* 32bpp formats */
>>      ENTRY (a8r8g8b8),
>>      ALIAS (a8r8g8b8,		"8888"),
>> @@ -1914,6 +1934,10 @@ pixel_checker_init (pixel_checker_t *checker, pixman_format_code_t format)
>>  
>>      checker->format = format;
>>  
>> +    if (format == PIXMAN_rgba_float ||
>> +	format == PIXMAN_rgb_float)
>> +	return;
>> +
>>      switch (PIXMAN_FORMAT_TYPE (format))
>>      {
>>      case PIXMAN_TYPE_A:
>> @@ -1970,10 +1994,19 @@ pixel_checker_init (pixel_checker_t *checker, pixman_format_code_t format)
>>      checker->bw = PIXMAN_FORMAT_B (format);
>>  }
>>  
>> +static void
>> +pixel_checker_require_uint32_format (const pixel_checker_t *checker)
>> +{
>> +    assert (checker->format != PIXMAN_rgba_float &&
>> +	    checker->format != PIXMAN_rgb_float);
>> +}
>> +
>>  void
>>  pixel_checker_split_pixel (const pixel_checker_t *checker, uint32_t pixel,
>>  			   int *a, int *r, int *g, int *b)
>>  {
>> +    pixel_checker_require_uint32_format(checker);
>> +
>>      *a = (pixel & checker->am) >> checker->as;
>>      *r = (pixel & checker->rm) >> checker->rs;
>>      *g = (pixel & checker->gm) >> checker->gs;
>> @@ -1987,6 +2020,8 @@ pixel_checker_get_masks (const pixel_checker_t *checker,
>>                           uint32_t              *gm,
>>                           uint32_t              *bm)
>>  {
>> +    pixel_checker_require_uint32_format(checker);
>> +
>>      if (am)
>>          *am = checker->am;
>>      if (rm)
>> @@ -2003,6 +2038,8 @@ pixel_checker_convert_pixel_to_color (const pixel_checker_t *checker,
>>  {
>>      int a, r, g, b;
>>  
>> +    pixel_checker_require_uint32_format(checker);
>> +
>>      pixel_checker_split_pixel (checker, pixel, &a, &r, &g, &b);
>>  
>>      if (checker->am == 0)
>> @@ -2078,6 +2115,8 @@ void
>>  pixel_checker_get_max (const pixel_checker_t *checker, color_t *color,
>>  		       int *am, int *rm, int *gm, int *bm)
>>  {
>> +    pixel_checker_require_uint32_format(checker);
>> +
>>      get_limits (checker, DEVIATION, color, am, rm, gm, bm);
>>  }
>>  
>> @@ -2085,6 +2124,8 @@ void
>>  pixel_checker_get_min (const pixel_checker_t *checker, color_t *color,
>>  		       int *am, int *rm, int *gm, int *bm)
>>  {
>> +    pixel_checker_require_uint32_format(checker);
>> +
>>      get_limits (checker, - DEVIATION, color, am, rm, gm, bm);
>>  }
>>  
>> @@ -2096,6 +2137,8 @@ pixel_checker_check (const pixel_checker_t *checker, uint32_t pixel,
>>      int32_t ai, ri, gi, bi;
>>      pixman_bool_t result;
>>  
>> +    pixel_checker_require_uint32_format(checker);
>> +
>>      pixel_checker_get_min (checker, color, &a_lo, &r_lo, &g_lo, &b_lo);
>>      pixel_checker_get_max (checker, color, &a_hi, &r_hi, &g_hi, &b_hi);
>>      pixel_checker_split_pixel (checker, pixel, &ai, &ri, &gi, &bi);
>> @@ -2108,3 +2151,36 @@ pixel_checker_check (const pixel_checker_t *checker, uint32_t pixel,
>>  
>>      return result;
>>  }
>> +
>> +static void
>> +color_limits (const pixel_checker_t *checker,
>> +	      double limit, const color_t *color, color_t *out)
>> +{
>> +    if (PIXMAN_FORMAT_A(checker->format))
>> +	out->a = color->a + limit;
>> +    else
>> +	out->a = 1.;
>> +
>> +    out->r = color->r + limit;
>> +    out->g = color->g + limit;
>> +    out->b = color->b + limit;
>> +}
>> +
>> +pixman_bool_t
>> +pixel_checker_check_color (const pixel_checker_t *checker,
>> +			   const color_t *actual, const color_t *reference)
>> +{
>> +    color_t min, max;
>> +    pixman_bool_t result;
>> +
>> +    color_limits(checker, -DEVIATION, reference, &min);
>> +    color_limits(checker, DEVIATION, reference, &max);
>> +
>> +    result =
>> +	actual->a >= min.a && actual->a <= max.a &&
>> +	actual->r >= min.r && actual->r <= max.r &&
>> +	actual->g >= min.g && actual->g <= max.g &&
>> +	actual->b >= min.b && actual->b <= max.b;
>> +
>> +    return result;
>> +}
>> diff --git a/test/utils.h b/test/utils.h
>> index e299d1d066ed..e5ac945ab3f8 100644
>> --- a/test/utils.h
>> +++ b/test/utils.h
>> @@ -119,6 +119,8 @@ fence_get_page_size ();
>>  /* Generate n_bytes random bytes in fence_malloced memory */
>>  uint8_t *
>>  make_random_bytes (int n_bytes);
>> +float *
>> +make_random_floats (int n_bytes);
>>  
>>  /* Return current time in seconds */
>>  double