pixman: Add support for argb/xrgb float formats, v5.

Submitted by Maarten Lankhorst on Oct. 31, 2018, 11:20 a.m.

Details

Message ID 937b03d6-fdc4-12af-90bf-bac92924da8a@linux.intel.com
State New
Series "Add float support to pixman"
Headers show

Commit Message

Maarten Lankhorst Oct. 31, 2018, 11:20 a.m.
Op 29-10-18 om 18:06 schreef Bill Spitzak:
> I think an indicator for float can double as indicating the size is multiplied by 8. All float formats I am familiar with take a mulitple of 8 bits, including an 8-bit format that is sometimes used in CG.
What about this, then?

----------------->8------------
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.
Changes since v2:
- Add asserts in accessor and for strides to force alignment.
- Move test changes to their own commit.
Changes since v3:
- Define 32bpc as PIXMAN_FORMAT_PACKED_C32
- Rename pixman accessors from rgb*_float_float to rgb*f_float
Changes since v4:
- Create a new PIXMAN_FORMAT_BYTE for fitting up to 64 bits per component.
  (based on Siarhei Siamashka's suggestion)
- Use new format type PIXMAN_TYPE_RGBA_FLOAT

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v4
---
 pixman/pixman-access.c     | 128 ++++++++++++++++++++++++++++++++++++-
 pixman/pixman-bits-image.c |   3 +
 pixman/pixman-image.c      |   4 ++
 pixman/pixman.c            |   5 ++
 pixman/pixman.h            |  35 +++++++---
 5 files changed, 166 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/pixman/pixman-access.c b/pixman/pixman-access.c
index 4f0642d77785..8dfd35f45063 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_rgbf_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_rgbaf_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_rgbf_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_rgbaf_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_rgbaf_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_rgbf_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_rgbaf_float,
+      fetch_pixel_generic_lossy_32, fetch_pixel_rgbaf_float,
+      NULL, store_scanline_rgbaf_float },
+
+    { PIXMAN_rgb_float,
+      NULL, fetch_scanline_rgbf_float,
+      fetch_pixel_generic_lossy_32, fetch_pixel_rgbf_float,
+      NULL, store_scanline_rgbf_float },
+#endif
+
     { PIXMAN_a2r10g10b10,
       NULL, fetch_scanline_a2r10g10b10_float,
       fetch_pixel_generic_lossy_32, fetch_pixel_a2r10g10b10_float,
diff --git a/pixman/pixman-bits-image.c b/pixman/pixman-bits-image.c
index dcdcc69946de..9fb91ff5831d 100644
--- a/pixman/pixman-bits-image.c
+++ b/pixman/pixman-bits-image.c
@@ -948,6 +948,9 @@  _pixman_bits_image_init (pixman_image_t *     image,
 {
     uint32_t *free_me = NULL;
 
+    if (PIXMAN_FORMAT_BPP (format) == 128)
+	return_val_if_fail(!(rowstride % 4), FALSE);
+
     if (!bits && width && height)
     {
 	int rowstride_bytes;
diff --git a/pixman/pixman-image.c b/pixman/pixman-image.c
index 681864eb7d17..7a851e221992 100644
--- a/pixman/pixman-image.c
+++ b/pixman/pixman-image.c
@@ -842,6 +842,10 @@  pixman_image_set_accessors (pixman_image_t *           image,
 
     if (image->type == BITS)
     {
+	/* Accessors only work for <= 32 bpp. */
+	if (PIXMAN_FORMAT_BPP(image->bits.format) > 32)
+	    return_if_fail (!read_func && !write_func);
+
 	image->bits.read_func = read_func;
 	image->bits.write_func = write_func;
 
diff --git a/pixman/pixman.c b/pixman/pixman.c
index f932eac3c1b6..c09b5280831f 100644
--- a/pixman/pixman.c
+++ b/pixman/pixman.c
@@ -777,6 +777,11 @@  color_to_pixel (const pixman_color_t *color,
 {
     uint32_t c = color_to_uint32 (color);
 
+    if (PIXMAN_FORMAT_TYPE (format) == PIXMAN_TYPE_RGBA_FLOAT)
+    {
+	return FALSE;
+    }
+
     if (!(format == PIXMAN_a8r8g8b8     ||
           format == PIXMAN_x8r8g8b8     ||
           format == PIXMAN_a8b8g8r8     ||
diff --git a/pixman/pixman.h b/pixman/pixman.h
index 509ba5e534a8..04d7504729e2 100644
--- a/pixman/pixman.h
+++ b/pixman/pixman.h
@@ -654,12 +654,24 @@  struct pixman_indexed
 					 ((g) << 4) |	  \
 					 ((b)))
 
-#define PIXMAN_FORMAT_BPP(f)	(((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_BYTE(bpp,type,a,r,g,b) \
+	(((bpp >> 3) << 24) | \
+	(3 << 22) | ((type) << 16) | \
+	((a >> 3) << 12) | \
+	((r >> 3) << 8) | \
+	((g >> 3) << 4) | \
+	((b >> 3)))
+
+#define PIXMAN_FORMAT_RESHIFT(val, ofs, num) \
+	((val >> (ofs)) & ((1 << (num)) - 1) << ((val >> 22) & 3))
+
+#define PIXMAN_FORMAT_BPP(f)	PIXMAN_FORMAT_RESHIFT(f, 24, 8)
+#define PIXMAN_FORMAT_SHIFT(f)	((uint32_t)((f >> 22) & 3))
+#define PIXMAN_FORMAT_TYPE(f)	(((f) >> 16) & 0x3f)
+#define PIXMAN_FORMAT_A(f)	PIXMAN_FORMAT_RESHIFT(f, 12, 4)
+#define PIXMAN_FORMAT_R(f)	PIXMAN_FORMAT_RESHIFT(f, 8, 4)
+#define PIXMAN_FORMAT_G(f)	PIXMAN_FORMAT_RESHIFT(f, 4, 4)
+#define PIXMAN_FORMAT_B(f)	PIXMAN_FORMAT_RESHIFT(f, 0, 4)
 #define PIXMAN_FORMAT_RGB(f)	(((f)      ) & 0xfff)
 #define PIXMAN_FORMAT_VIS(f)	(((f)      ) & 0xffff)
 #define PIXMAN_FORMAT_DEPTH(f)	(PIXMAN_FORMAT_A(f) +	\
@@ -678,15 +690,22 @@  struct pixman_indexed
 #define PIXMAN_TYPE_BGRA	8
 #define PIXMAN_TYPE_RGBA	9
 #define PIXMAN_TYPE_ARGB_SRGB	10
+#define PIXMAN_TYPE_RGBA_FLOAT	11
 
 #define PIXMAN_FORMAT_COLOR(f)				\
 	(PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_ARGB ||	\
 	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_ABGR ||	\
 	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_BGRA ||	\
-	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_RGBA)
+	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_RGBA ||	\
+	 PIXMAN_FORMAT_TYPE(f) == PIXMAN_TYPE_RGBA_FLOAT)
 
-/* 32bpp formats */
 typedef enum {
+/* 128bpp formats */
+    PIXMAN_rgba_float =	PIXMAN_FORMAT_BYTE(128,PIXMAN_TYPE_RGBA_FLOAT,32,32,32,32),
+/* 96bpp formats */
+    PIXMAN_rgb_float =	PIXMAN_FORMAT_BYTE(96,PIXMAN_TYPE_RGBA_FLOAT,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),

Comments

Bill Spitzak Oct. 31, 2018, 7:04 p.m.
I don't like the fact that the design which allows the rgba to be different
sizes cannot make them vary between float and decimal. The main reason is
so that an integer for Alpha can be supported. I think maybe there should
be a bit that changes the interpretation of the bit widths to a 16-valued
type enumeration. For n-bit integers use n-1 as now, but remove the unusual
ones and replace them with floating point and signed types. Use a lookup
table to get type + width. I really don't think there are more than 16 of
them:

8, 16, 32, 64 bit unsigned
8, 16, 32, 64 bit float
this set leaves 8 unused values, which maybe could be allocated in the
future for signed types or 128-bit types. You could also provide a few more
unsigned widths (1,2,4,10,12?)
Maarten Lankhorst Nov. 1, 2018, 11:54 a.m.
Op 31-10-18 om 20:04 schreef Bill Spitzak:
> I don't like the fact that the design which allows the rgba to be different sizes cannot make them vary between float and decimal. The main reason is so that an integer for Alpha can be supported. I think maybe there should be a bit that changes the interpretation of the bit widths to a 16-valued type enumeration. For n-bit integers use n-1 as now, but remove the unusual ones and replace them with floating point and signed types. Use a lookup table to get type + width. I really don't think there are more than 16 of them:
>
> 8, 16, 32, 64 bit unsigned
> 8, 16, 32, 64 bit float
> this set leaves 8 unused values, which maybe could be allocated in the future for signed types or 128-bit types. You could also provide a few more unsigned widths (1,2,4,10,12?)
>
I'm not aware of any format that has a different type for float alpha and colors.. In any case
that's probably what you should put in the type info.

~Maarten