Re: [PATCH] Add support for RGBA128F and RGB96F formats.
Maarten Lankhorst <[email protected]> Tue, 12 Jun 2018 12:11:19 +0200
| Newsgroups | gmane.comp.lib.cairo |
|---|---|
| Message-ID | <[email protected]> |
Hey, Op 11-06-18 om 20:54 schreef Uli Schlachter: > Not much time currently, so just some quick questions: > > - Alignment requirements? (e.g. cairo_stride_for_width()) I would say 4 byte alignment for 96bpp is fine, and 128bpp 16 byte alignment for aligned access with AVX extensions. > - What guarantees that sizeof(float) is what you expect it to be? Seems to be strong enough. > - Is RGB96F really 96bit aka 12 byte per pixel? That's surprisingly... > odd. E.g. RGB24 is still 32 bit per pixel. Yeah, the original patch series had 128bpp, but either value works for me, as long as we settle on a format. > - Test suite coverage? Any tests for this? I was using IGT to test with planar formats for conversion. I'll add some tests too when we decide on the final format. > - What are the values of the floats? Numbers from 0 to 1? That's quite a > number of unused bits... Yes, but that's how floating point values currently work in graphics cards. You won't get more precision than that. But it's still a lot better than 16-bits shorts. :) But I think only a single bit is unused, the highest bit of the exponent. ~Maarten > On 11.06.2018 15:19, Maarten Lankhorst wrote: >> IGT wants to add support for planes with a bit depth >10, which >> requires a higher precision format than we have currently. >> >> I'm using RGBA as format, because of its existence in OpenGL. >> With the new formats we can directly convert our bytes to half float, >> or do our own conversion to convert to one of the planar YCbCr formats. >> >> Cc: Ville Syrjälä <[email protected]> >> Signed-off-by: Maarten Lankhorst <[email protected]> >> --- >> This patch requires the following patch in pixman to work: >> https://lists.freedesktop.org/archives/pixman/2018-June/004734.html >> >> perf/micro/fill-clip.c | 2 ++ >> perf/micro/pixel.c | 2 ++ >> src/cairo-debug.c | 6 ++++ >> src/cairo-image-compositor.c | 4 +++ >> src/cairo-image-source.c | 27 ++++++++++++++++ >> src/cairo-image-surface.c | 16 ++++++++++ >> src/cairo-png.c | 2 ++ >> src/cairo-script-surface.c | 20 ++++++++++++ >> src/cairo-xlib-display.c | 12 +++++++ >> src/cairo.h | 6 +++- >> src/cairoint.h | 2 +- >> test/any2ppm.c | 2 ++ >> test/map-to-image.c | 2 ++ >> test/png.c | 2 ++ >> util/cairo-script/cairo-script-operators.c | 12 +++++++ >> util/cairo-trace/trace.c | 37 ++++++++++------------ >> 16 files changed, 131 insertions(+), 23 deletions(-) >> >> diff --git a/perf/micro/fill-clip.c b/perf/micro/fill-clip.c >> index 2d014aca832c..f9802705f4d0 100644 >> --- a/perf/micro/fill-clip.c >> +++ b/perf/micro/fill-clip.c >> @@ -92,6 +92,8 @@ direct (cairo_t *cr, int width, int height, int loops) >> case CAIRO_FORMAT_RGB24: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_ARGB32: bpp = 32; break; >> + case CAIRO_FORMAT_RGB96F: bpp = 96; break; >> + case CAIRO_FORMAT_RGBA128F: bpp = 128; break; >> } >> >> cairo_perf_timer_start (); >> diff --git a/perf/micro/pixel.c b/perf/micro/pixel.c >> index b600b5170d86..85a42e4427bc 100644 >> --- a/perf/micro/pixel.c >> +++ b/perf/micro/pixel.c >> @@ -51,6 +51,8 @@ pixel_direct (cairo_t *cr, int width, int height, int loops) >> case CAIRO_FORMAT_RGB24: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_ARGB32: bpp = 32; break; >> + case CAIRO_FORMAT_RGB96F: bpp = 96; break; >> + case CAIRO_FORMAT_RGBA128F: bpp = 128; break; >> } >> >> cairo_perf_timer_start (); >> diff --git a/src/cairo-debug.c b/src/cairo-debug.c >> index 6005060d4a0c..760f092e3a8b 100644 >> --- a/src/cairo-debug.c >> +++ b/src/cairo-debug.c >> @@ -131,6 +131,12 @@ _cairo_debug_check_image_surface_is_defined (const cairo_surface_t *surface) >> case CAIRO_FORMAT_ARGB32: >> width = image->width*4; >> break; >> + case CAIRO_FORMAT_RGB96F: >> + width = image->width*12; >> + break; >> + case CAIRO_FORMAT_RGBA128F: >> + width = image->width*16; >> + break; >> case CAIRO_FORMAT_INVALID: >> default: >> /* XXX compute width from pixman bpp */ >> diff --git a/src/cairo-image-compositor.c b/src/cairo-image-compositor.c >> index bbf4cf2281da..434f67e59b74 100644 >> --- a/src/cairo-image-compositor.c >> +++ b/src/cairo-image-compositor.c >> @@ -2845,6 +2845,8 @@ inplace_renderer_init (cairo_image_span_renderer_t *r, >> case CAIRO_FORMAT_A1: >> case CAIRO_FORMAT_RGB16_565: >> case CAIRO_FORMAT_RGB30: >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_INVALID: >> default: break; >> } >> @@ -2860,6 +2862,8 @@ inplace_renderer_init (cairo_image_span_renderer_t *r, >> case CAIRO_FORMAT_A1: >> case CAIRO_FORMAT_RGB16_565: >> case CAIRO_FORMAT_RGB30: >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_INVALID: >> default: break; >> } >> diff --git a/src/cairo-image-source.c b/src/cairo-image-source.c >> index 0b50afcad327..c56845ab2d91 100644 >> --- a/src/cairo-image-source.c >> +++ b/src/cairo-image-source.c >> @@ -455,6 +455,7 @@ static pixman_image_t * >> _pixel_to_solid (cairo_image_surface_t *image, int x, int y) >> { >> uint32_t pixel; >> + float *rgba; >> pixman_color_t color; >> >> TRACE ((stderr, "%s\n", __FUNCTION__)); >> @@ -523,6 +524,32 @@ _pixel_to_solid (cairo_image_surface_t *image, int x, int y) >> color.green = (pixel >> 8 & 0xff) | (pixel & 0xff00); >> color.blue = (pixel & 0xff) | (pixel << 8 & 0xff00); >> return pixman_image_create_solid_fill (&color); >> + >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> + if (image->format == CAIRO_FORMAT_RGBA128F) >> + { >> + rgba = (float *)&image->data[y * image->stride + 16 * x]; >> + color.alpha = 65535.f * rgba[3]; >> + >> + if (color.alpha == 0) >> + return _pixman_transparent_image (); >> + } >> + else >> + { >> + rgba = (float *)&image->data[y * image->stride + 12 * x]; >> + color.alpha = 0xffff; >> + } >> + >> + if (color.alpha == 0xffff && rgba[0] == 0.f && rgba[1] == 0.f && rgba[2] == 0.f) >> + return _pixman_black_image (); >> + if (color.alpha == 0xffff && rgba[0] == 1.f && rgba[1] == 1.f && rgba[2] == 1.f) >> + return _pixman_white_image (); >> + >> + color.red = rgba[0] * 65535.f; >> + color.green = rgba[1] * 65535.f; >> + color.blue = rgba[2] * 65535.f; >> + return pixman_image_create_solid_fill (&color); >> } >> } >> >> diff --git a/src/cairo-image-surface.c b/src/cairo-image-surface.c >> index 2ee1cad42cf7..0e17f3a1619c 100644 >> --- a/src/cairo-image-surface.c >> +++ b/src/cairo-image-surface.c >> @@ -93,6 +93,10 @@ cairo_format_t >> _cairo_format_from_pixman_format (pixman_format_code_t pixman_format) >> { >> switch (pixman_format) { >> + case PIXMAN_rgba_float: >> + return CAIRO_FORMAT_RGBA128F; >> + case PIXMAN_rgb_float: >> + return CAIRO_FORMAT_RGB96F; >> case PIXMAN_a8r8g8b8: >> return CAIRO_FORMAT_ARGB32; >> case PIXMAN_x2r10g10b10: >> @@ -322,6 +326,12 @@ _cairo_format_to_pixman_format_code (cairo_format_t format) >> case CAIRO_FORMAT_RGB16_565:) >> ret = PIXMAN_r5g6b5; >> break; >> + case CAIRO_FORMAT_RGB96F: >> + ret = PIXMAN_rgb_float; >> + break; >> + case CAIRO_FORMAT_RGBA128F: >> + ret = PIXMAN_rgba_float; >> + break; >> case CAIRO_FORMAT_ARGB32: >> case CAIRO_FORMAT_INVALID: >> default: >> @@ -693,8 +703,10 @@ _cairo_format_from_content (cairo_content_t content) >> _cairo_content_from_format (cairo_format_t format) >> { >> switch (format) { >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_ARGB32: >> return CAIRO_CONTENT_COLOR_ALPHA; >> + case CAIRO_FORMAT_RGB96F: >> case CAIRO_FORMAT_RGB30: >> return CAIRO_CONTENT_COLOR; >> case CAIRO_FORMAT_RGB24: >> @@ -716,6 +728,10 @@ _cairo_content_from_format (cairo_format_t format) >> _cairo_format_bits_per_pixel (cairo_format_t format) >> { >> switch (format) { >> + case CAIRO_FORMAT_RGBA128F: >> + return 128; >> + case CAIRO_FORMAT_RGB96F: >> + return 96; >> case CAIRO_FORMAT_ARGB32: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_RGB24: >> diff --git a/src/cairo-png.c b/src/cairo-png.c >> index ab0b9d0c51ea..b9fc9160a8ab 100644 >> --- a/src/cairo-png.c >> +++ b/src/cairo-png.c >> @@ -265,6 +265,8 @@ write_png (cairo_surface_t *surface, >> break; >> case CAIRO_FORMAT_INVALID: >> case CAIRO_FORMAT_RGB16_565: >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> default: >> status = _cairo_error (CAIRO_STATUS_INVALID_FORMAT); >> goto BAIL4; >> diff --git a/src/cairo-script-surface.c b/src/cairo-script-surface.c >> index e715cae50fb3..39be5d1bee36 100644 >> --- a/src/cairo-script-surface.c >> +++ b/src/cairo-script-surface.c >> @@ -870,6 +870,8 @@ static const char * >> _format_to_string (cairo_format_t format) >> { >> switch (format) { >> + case CAIRO_FORMAT_RGBA128F: return "RGBA128F"; >> + case CAIRO_FORMAT_RGB96F: return "RGB96F"; >> case CAIRO_FORMAT_ARGB32: return "ARGB32"; >> case CAIRO_FORMAT_RGB30: return "RGB30"; >> case CAIRO_FORMAT_RGB24: return "RGB24"; >> @@ -1314,6 +1316,18 @@ _write_image_surface (cairo_output_stream_t *output, >> data += stride; >> } >> break; >> + case CAIRO_FORMAT_RGB96F: >> + for (row = image->height; row--; ) { >> + _cairo_output_stream_write (output, data, 12*width); >> + data += stride; >> + } >> + break; >> + case CAIRO_FORMAT_RGBA128F: >> + for (row = image->height; row--; ) { >> + _cairo_output_stream_write (output, data, 16*width); >> + data += stride; >> + } >> + break; >> case CAIRO_FORMAT_INVALID: >> default: >> ASSERT_NOT_REACHED; >> @@ -1420,6 +1434,12 @@ _emit_image_surface (cairo_script_surface_t *surface, >> case CAIRO_FORMAT_ARGB32: >> len = clone->width * 4; >> break; >> + case CAIRO_FORMAT_RGB96F: >> + len = clone->width * 12; >> + break; >> + case CAIRO_FORMAT_RGBA128F: >> + len = clone->width * 16; >> + break; >> case CAIRO_FORMAT_INVALID: >> default: >> ASSERT_NOT_REACHED; >> diff --git a/src/cairo-xlib-display.c b/src/cairo-xlib-display.c >> index add42299b4cd..108897e92a2e 100644 >> --- a/src/cairo-xlib-display.c >> +++ b/src/cairo-xlib-display.c >> @@ -383,6 +383,10 @@ _cairo_xlib_display_get_xrender_format_for_pixman(cairo_xlib_display_t *display, >> XRenderPictFormat tmpl; >> int mask; >> >> + /* No equivalent in X11 yet. */ >> + if (format == PIXMAN_rgba_float || format == PIXMAN_rgb_float) >> + return NULL; >> + >> #define MASK(x) ((1<<(x))-1) >> >> tmpl.depth = PIXMAN_FORMAT_DEPTH(format); >> @@ -510,6 +514,14 @@ _cairo_xlib_display_get_xrender_format (cairo_xlib_display_t *display, >> xrender_format = _cairo_xlib_display_get_xrender_format_for_pixman(display, >> PIXMAN_x2r10g10b10); >> break; >> + case CAIRO_FORMAT_RGBA128F: >> + xrender_format = _cairo_xlib_display_get_xrender_format_for_pixman(display, >> + PIXMAN_rgba_float); >> + break; >> + case CAIRO_FORMAT_RGB96F: >> + xrender_format = _cairo_xlib_display_get_xrender_format_for_pixman(display, >> + PIXMAN_rgb_float); >> + break; >> case CAIRO_FORMAT_INVALID: >> default: >> ASSERT_NOT_REACHED; >> diff --git a/src/cairo.h b/src/cairo.h >> index b2386af5d920..29be5ef7e57e 100644 >> --- a/src/cairo.h >> +++ b/src/cairo.h >> @@ -405,6 +405,8 @@ typedef enum _cairo_content { >> * with red in the upper 5 bits, then green in the middle >> * 6 bits, and blue in the lower 5 bits. (Since 1.2) >> * @CAIRO_FORMAT_RGB30: like RGB24 but with 10bpc. (Since 1.12) >> + * @CAIRO_FORMAT_RGB96F: 3 floats, R, G, B. (Since 1.16) >> + * @CAIRO_FORMAT_RGBA128F: 4 floats, R, G, B, A. (Since 1.16) >> * >> * #cairo_format_t is used to identify the memory format of >> * image data. >> @@ -420,7 +422,9 @@ typedef enum _cairo_format { >> CAIRO_FORMAT_A8 = 2, >> CAIRO_FORMAT_A1 = 3, >> CAIRO_FORMAT_RGB16_565 = 4, >> - CAIRO_FORMAT_RGB30 = 5 >> + CAIRO_FORMAT_RGB30 = 5, >> + CAIRO_FORMAT_RGB96F = 6, >> + CAIRO_FORMAT_RGBA128F = 7 >> } cairo_format_t; >> >> >> diff --git a/src/cairoint.h b/src/cairoint.h >> index cfae18cf93bd..331ab5357ac4 100644 >> --- a/src/cairoint.h >> +++ b/src/cairoint.h >> @@ -1536,7 +1536,7 @@ _cairo_surface_release_device_reference (cairo_surface_t *surface); >> * in cairo-xlib-surface.c--again see -Wswitch-enum). >> */ >> #define CAIRO_FORMAT_VALID(format) ((format) >= CAIRO_FORMAT_ARGB32 && \ >> - (format) <= CAIRO_FORMAT_RGB30) >> + (format) <= CAIRO_FORMAT_RGBA128F) >> >> /* pixman-required stride alignment in bytes. */ >> #define CAIRO_STRIDE_ALIGNMENT (sizeof (uint32_t)) >> diff --git a/test/any2ppm.c b/test/any2ppm.c >> index a92412d084f1..7eb582c04d22 100644 >> --- a/test/any2ppm.c >> +++ b/test/any2ppm.c >> @@ -201,6 +201,8 @@ write_ppm (cairo_surface_t *surface, int fd) >> case CAIRO_FORMAT_A1: >> case CAIRO_FORMAT_RGB16_565: >> case CAIRO_FORMAT_RGB30: >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_INVALID: >> default: >> return "unhandled image format"; >> diff --git a/test/map-to-image.c b/test/map-to-image.c >> index 0262245a88e1..2b1799f71107 100644 >> --- a/test/map-to-image.c >> +++ b/test/map-to-image.c >> @@ -45,6 +45,8 @@ set_pixel_black(uint8_t *data, int stride, >> case CAIRO_FORMAT_RGB16_565: >> *(uint16_t *)(data + y * stride + 2*x) = black_pixel; >> break; >> + case CAIRO_FORMAT_RGBA128F: >> + case CAIRO_FORMAT_RGB96F: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_A8: >> case CAIRO_FORMAT_A1: >> diff --git a/test/png.c b/test/png.c >> index cd11fd0d1825..a199d4867d3f 100644 >> --- a/test/png.c >> +++ b/test/png.c >> @@ -60,6 +60,8 @@ format_to_string (cairo_format_t format) >> case CAIRO_FORMAT_RGB24: return "rgb24"; >> case CAIRO_FORMAT_RGB30: return "rgb30"; >> case CAIRO_FORMAT_ARGB32: return "argb32"; >> + case CAIRO_FORMAT_RGB96F: return "rgb96f"; >> + case CAIRO_FORMAT_RGBA128F: return "rgba128f"; >> case CAIRO_FORMAT_INVALID: >> default: return "???"; >> } >> diff --git a/util/cairo-script/cairo-script-operators.c b/util/cairo-script/cairo-script-operators.c >> index e493311e74a0..7cdb5afb66a2 100644 >> --- a/util/cairo-script/cairo-script-operators.c >> +++ b/util/cairo-script/cairo-script-operators.c >> @@ -2967,6 +2967,12 @@ _image_read_raw (csi_t *ctx, >> case CAIRO_FORMAT_ARGB32: >> instride = rowlen = 4 * width; >> break; >> + case CAIRO_FORMAT_RGB96F: >> + instride = rowlen = 12 * width; >> + break; >> + case CAIRO_FORMAT_RGBA128F: >> + instride = rowlen = 16 * width; >> + break; >> } >> len = rowlen * height; >> >> @@ -3066,6 +3072,8 @@ err_decompress: >> #endif >> } >> break; >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_INVALID: >> case CAIRO_FORMAT_ARGB32: >> @@ -3155,6 +3163,8 @@ err_decompress: >> #endif >> } >> break; >> + case CAIRO_FORMAT_RGBA128F: >> + case CAIRO_FORMAT_RGB96F: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_INVALID: >> case CAIRO_FORMAT_ARGB32: >> @@ -3191,6 +3201,8 @@ err_decompress: >> case CAIRO_FORMAT_A8: >> break; >> >> + case CAIRO_FORMAT_RGBA128F: >> + case CAIRO_FORMAT_RGB96F: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_RGB24: >> case CAIRO_FORMAT_INVALID: >> diff --git a/util/cairo-trace/trace.c b/util/cairo-trace/trace.c >> index 3c056134eacc..f72a6c4cdace 100644 >> --- a/util/cairo-trace/trace.c >> +++ b/util/cairo-trace/trace.c >> @@ -1508,6 +1508,8 @@ _format_to_string (cairo_format_t format) >> #define f(name) case CAIRO_FORMAT_ ## name: return #name >> switch (format) { >> f(INVALID); >> + f(RGBA128F); >> + f(RGB96F); >> f(ARGB32); >> f(RGB30); >> f(RGB24); >> @@ -1525,8 +1527,10 @@ _format_to_content_string (cairo_format_t format) >> switch (format) { >> case CAIRO_FORMAT_INVALID: >> return "INVALID"; >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_ARGB32: >> return "COLOR_ALPHA"; >> + case CAIRO_FORMAT_RGB96F: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_RGB24: >> case CAIRO_FORMAT_RGB16_565: >> @@ -1671,6 +1675,8 @@ _emit_image (cairo_surface_t *image, >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_INVALID: >> case CAIRO_FORMAT_ARGB32: len = 4*width; break; >> + case CAIRO_FORMAT_RGB96F: len = 12*width; break; >> + case CAIRO_FORMAT_RGBA128F: len = 16*width; break; >> } >> >> _trace_printf (" /source "); >> @@ -1678,24 +1684,6 @@ _emit_image (cairo_surface_t *image, >> >> #ifdef WORDS_BIGENDIAN >> switch (format) { >> - case CAIRO_FORMAT_A1: >> - for (row = height; row--; ) { >> - _write_data (&stream, data, (width+7)/8); >> - data += stride; >> - } >> - break; >> - case CAIRO_FORMAT_A8: >> - for (row = height; row--; ) { >> - _write_data (&stream, data, width); >> - data += stride; >> - } >> - break; >> - case CAIRO_FORMAT_RGB16_565: >> - for (row = height; row--; ) { >> - _write_data (&stream, data, 2*width); >> - data += stride; >> - } >> - break; >> case CAIRO_FORMAT_RGB24: >> for (row = height; row--; ) { >> int col; >> @@ -1707,10 +1695,15 @@ _emit_image (cairo_surface_t *image, >> data += stride; >> } >> break; >> + case CAIRO_FORMAT_A1: >> + case CAIRO_FORMAT_A8: >> + case CAIRO_FORMAT_RGB16_565: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_ARGB32: >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> for (row = height; row--; ) { >> - _write_data (&stream, data, 4*width); >> + _write_data (&stream, data, len); >> data += stride; >> } >> break; >> @@ -1767,6 +1760,8 @@ _emit_image (cairo_surface_t *image, >> data += stride; >> } >> break; >> + case CAIRO_FORMAT_RGB96F: >> + case CAIRO_FORMAT_RGBA128F: >> case CAIRO_FORMAT_RGB30: >> case CAIRO_FORMAT_ARGB32: >> for (row = height; row--; ) { >> @@ -1775,11 +1770,11 @@ _emit_image (cairo_surface_t *image, >> int col; >> for (col = 0; col < width; col++) >> dst[col] = bswap_32 (src[col]); >> - _write_data (&stream, rowdata, 4*width); >> + _write_data (&stream, rowdata, len); >> data += stride; >> } >> break; >> - case CAIRO_FORMAT_INVALID: >> + case CAIRO_FORMAT_INVALID: >> default: >> break; >> } >> > -- cairo mailing list [email protected] https://lists.cairographics.org/mailman/listinfo/cairo