[1/2] pixman: Add support for argb/xrgb float formats, v4.

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

Details

Message ID 1cfe5634-8a90-9e3b-eb45-e55a10204290@linux.intel.com
State New
Headers show
Series "Add float support to pixman" ( rev: 2 ) in Pixman

Not browsing as part of any series.

Commit Message

Maarten Lankhorst Oct. 25, 2018, 11:25 a.m.
Op 03-10-18 om 11:11 schreef Chris Wilson:
> Quoting Maarten Lankhorst (2018-08-01 13:41:33)
>> diff --git a/pixman/pixman.h b/pixman/pixman.h
>> index 509ba5e534a8..c9bf4faa80d4 100644
>> --- a/pixman/pixman.h
>> +++ b/pixman/pixman.h
>> @@ -647,19 +647,24 @@ 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_PACKC(val) ((val) <= 10 ? (val) : \
>> +                                (val) == 32 ? 11 : 0)
>> +#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \
>> +                                   (val) == 11 ? 32 : 0)
> We have 4bits, why not reserve 0xf for float32? Certainly you
> want space for
>
> #define PIXMAN_FORMAT_PACKED_FLOAT16 0xb
> #define PIXMAN_FORMAT_PACKED_FLOAT32 0xc
> #define PIXMAN_FORMAT_PACKED_FLOAT64 0xd
>
> I just wonder if we may have 12bpc one day as well, so leaving 0xc clear
> has some some appeal.
> -Chris

I don't think we support double precision in pixman yet, but here's v4. :)

Apply with --scissors
------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

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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.h b/pixman/pixman.h
index 509ba5e534a8..978dc8c2d4a8 100644
--- a/pixman/pixman.h
+++ b/pixman/pixman.h
@@ -647,19 +647,27 @@  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_PACKED_C32 0xf
+
+#define PIXMAN_FORMAT_PACKC(val) ((val) == 32 ? \
+				  PIXMAN_FORMAT_PACKED_C32 : \
+				  (val))
+#define PIXMAN_FORMAT_UNPACKC(val) ((val) == PIXMAN_FORMAT_PACKED_C32 ? \
+				    32 : (val))
+
+#define PIXMAN_FORMAT(bpp,type,a,r,g,b)	((((uint32_t)(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)	(((uint32_t)(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 +693,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),

Comments

Quoting Maarten Lankhorst (2018-10-25 12:25:15)
> Op 03-10-18 om 11:11 schreef Chris Wilson:
> > Quoting Maarten Lankhorst (2018-08-01 13:41:33)
> >> diff --git a/pixman/pixman.h b/pixman/pixman.h
> >> index 509ba5e534a8..c9bf4faa80d4 100644
> >> --- a/pixman/pixman.h
> >> +++ b/pixman/pixman.h
> >> @@ -647,19 +647,24 @@ 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_PACKC(val) ((val) <= 10 ? (val) : \
> >> +                                (val) == 32 ? 11 : 0)
> >> +#define PIXMAN_FORMAT_UNPACKC(val) ((val) <= 10 ? (val) : \
> >> +                                   (val) == 11 ? 32 : 0)
> > We have 4bits, why not reserve 0xf for float32? Certainly you
> > want space for
> >
> > #define PIXMAN_FORMAT_PACKED_FLOAT16 0xb
> > #define PIXMAN_FORMAT_PACKED_FLOAT32 0xc
> > #define PIXMAN_FORMAT_PACKED_FLOAT64 0xd
> >
> > I just wonder if we may have 12bpc one day as well, so leaving 0xc clear
> > has some some appeal.
> > -Chris
> 
> I don't think we support double precision in pixman yet, but here's v4. :)
> 
> Apply with --scissors
> ------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
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

I'm confident that we are not shooting ourselves in the foot here wrt to
potential future api directions, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris