From d90c2ab2a05cdb6cbd9f47acbe5d07720a9a84bd Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Mon, 4 Dec 2023 17:36:36 +0100 Subject: [PATCH] cogl: Determine glReadPixels format based on framebuffer format Which gltype and glformat are allowed for a given gl framebuffer depends on the internal gl framebuffer format. Either the format we want to read into matches the gltype, glformat and doesn't need CPU packing from another format or we have to use a tmp buffer with a format that matches the GL requirements. Part-of: --- cogl/cogl/cogl-driver.h | 11 +- cogl/cogl/driver/gl/cogl-framebuffer-gl.c | 52 +++------ cogl/cogl/driver/gl/gl/cogl-driver-gl.c | 19 ++-- cogl/cogl/driver/gl/gles/cogl-driver-gles.c | 115 +++++++++++++++++--- cogl/cogl/driver/nop/cogl-driver-nop.c | 2 +- 5 files changed, 137 insertions(+), 62 deletions(-) diff --git a/cogl/cogl/cogl-driver.h b/cogl/cogl/cogl-driver.h index c0870062b..3980443d2 100644 --- a/cogl/cogl/cogl-driver.h +++ b/cogl/cogl/cogl-driver.h @@ -62,11 +62,12 @@ struct _CoglDriverVtable GLenum *out_glformat, GLenum *out_gltype); - gboolean - (* read_pixels_format_supported) (CoglContext *ctx, - GLenum gl_intformat, - GLenum gl_format, - GLenum gl_type); + CoglPixelFormat + (* get_read_pixels_format) (CoglContext *context, + CoglPixelFormat from, + CoglPixelFormat to, + GLenum *gl_format_out, + GLenum *gl_type_out); gboolean (* update_features) (CoglContext *context, diff --git a/cogl/cogl/driver/gl/cogl-framebuffer-gl.c b/cogl/cogl/driver/gl/cogl-framebuffer-gl.c index 6ea71831d..f175ad0f9 100644 --- a/cogl/cogl/driver/gl/cogl-framebuffer-gl.c +++ b/cogl/cogl/driver/gl/cogl-framebuffer-gl.c @@ -424,14 +424,13 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver *driver, CoglPixelFormat format = cogl_bitmap_get_format (bitmap); CoglPixelFormat internal_format = cogl_framebuffer_get_internal_format (framebuffer); - CoglPixelFormat required_format; - GLenum gl_intformat; + CoglPixelFormat read_format; GLenum gl_format; GLenum gl_type; GLenum gl_pack_enum = GL_FALSE; int bytes_per_pixel; - gboolean is_read_pixels_format_supported; gboolean format_mismatch; + gboolean stride_mismatch; gboolean pack_invert_set; int status = FALSE; @@ -448,12 +447,6 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver *driver, if (!cogl_framebuffer_is_y_flipped (framebuffer)) y = framebuffer_height - y - height; - required_format = ctx->driver_vtable->pixel_format_to_gl (ctx, - format, - &gl_intformat, - &gl_format, - &gl_type); - if (_cogl_has_private_feature (ctx, COGL_PRIVATE_FEATURE_MESA_PACK_INVERT) && (source & COGL_READ_PIXELS_NO_FLIP) == 0 && !cogl_framebuffer_is_y_flipped (framebuffer)) @@ -469,39 +462,28 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver *driver, else pack_invert_set = FALSE; - bytes_per_pixel = cogl_pixel_format_get_bytes_per_pixel (format, 0); - is_read_pixels_format_supported = - ctx->driver_vtable->read_pixels_format_supported (ctx, - gl_intformat, - gl_format, - gl_type); - format_mismatch = - (required_format & ~COGL_PREMULT_BIT) != - (format & ~COGL_PREMULT_BIT); + read_format = ctx->driver_vtable->get_read_pixels_format (ctx, + internal_format, + format, + &gl_format, + &gl_type); - if (!is_read_pixels_format_supported || - format_mismatch || - (!_cogl_has_private_feature (ctx, - COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE) && - cogl_bitmap_get_rowstride (bitmap) != bytes_per_pixel * width)) + format_mismatch = + (read_format & ~COGL_PREMULT_BIT) != (format & ~COGL_PREMULT_BIT); + + bytes_per_pixel = cogl_pixel_format_get_bytes_per_pixel (format, 0); + stride_mismatch = + !_cogl_has_private_feature (ctx, + COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE) && + (cogl_bitmap_get_rowstride (bitmap) != bytes_per_pixel * width); + + if (format_mismatch || stride_mismatch) { CoglBitmap *tmp_bmp; - CoglPixelFormat read_format; int bpp, rowstride; uint8_t *tmp_data; gboolean succeeded; - if (is_read_pixels_format_supported) - { - read_format = required_format; - } - else - { - read_format = COGL_PIXEL_FORMAT_RGBA_8888; - gl_format = GL_RGBA; - gl_type = GL_UNSIGNED_BYTE; - } - if (COGL_PIXEL_FORMAT_CAN_HAVE_PREMULT (read_format)) { read_format = ((read_format & ~COGL_PREMULT_BIT) | diff --git a/cogl/cogl/driver/gl/gl/cogl-driver-gl.c b/cogl/cogl/driver/gl/gl/cogl-driver-gl.c index 2707b139c..c807f03e2 100644 --- a/cogl/cogl/driver/gl/gl/cogl-driver-gl.c +++ b/cogl/cogl/driver/gl/gl/cogl-driver-gl.c @@ -329,13 +329,18 @@ _cogl_driver_pixel_format_to_gl (CoglContext *context, return required_format; } -static gboolean -_cogl_driver_read_pixels_format_supported (CoglContext *context, - GLenum glintformat, - GLenum glformat, - GLenum gltype) +static CoglPixelFormat +_cogl_driver_get_read_pixels_format (CoglContext *context, + CoglPixelFormat from, + CoglPixelFormat to, + GLenum *gl_format_out, + GLenum *gl_type_out) { - return TRUE; + return _cogl_driver_pixel_format_to_gl (context, + to, + NULL, + gl_format_out, + gl_type_out); } static gboolean @@ -533,7 +538,7 @@ _cogl_driver_gl = _cogl_driver_gl_is_hardware_accelerated, _cogl_gl_get_graphics_reset_status, _cogl_driver_pixel_format_to_gl, - _cogl_driver_read_pixels_format_supported, + _cogl_driver_get_read_pixels_format, _cogl_driver_update_features, _cogl_driver_gl_create_framebuffer_driver, _cogl_driver_gl_flush_framebuffer_state, diff --git a/cogl/cogl/driver/gl/gles/cogl-driver-gles.c b/cogl/cogl/driver/gl/gles/cogl-driver-gles.c index 98e74c497..11b271bbb 100644 --- a/cogl/cogl/driver/gl/gles/cogl-driver-gles.c +++ b/cogl/cogl/driver/gl/gles/cogl-driver-gles.c @@ -356,22 +356,109 @@ _cogl_driver_pixel_format_to_gl (CoglContext *context, return required_format; } -static gboolean -_cogl_driver_read_pixels_format_supported (CoglContext *context, - GLenum glintformat, - GLenum glformat, - GLenum gltype) +static CoglPixelFormat +_cogl_driver_get_read_pixels_format (CoglContext *context, + CoglPixelFormat from, + CoglPixelFormat to, + GLenum *gl_format_out, + GLenum *gl_type_out) { - if (glformat == GL_RGBA && gltype == GL_UNSIGNED_BYTE) - return TRUE; + CoglPixelFormat required_format = 0; + GLenum required_gl_format = 0; + GLenum required_gl_type = 0; + CoglPixelFormat to_required_format; + GLenum to_gl_format; + GLenum to_gl_type; - if (glintformat == GL_RGB10_A2_EXT && - glformat == GL_RGBA && - gltype == GL_UNSIGNED_INT_2_10_10_10_REV && - cogl_has_feature (context, COGL_FEATURE_ID_TEXTURE_RGBA1010102)) - return TRUE; + switch (from) + { + /* fixed point normalized */ + case COGL_PIXEL_FORMAT_A_8: + case COGL_PIXEL_FORMAT_R_8: + case COGL_PIXEL_FORMAT_RG_88: + case COGL_PIXEL_FORMAT_RGB_888: + case COGL_PIXEL_FORMAT_BGR_888: + case COGL_PIXEL_FORMAT_BGRA_8888: + case COGL_PIXEL_FORMAT_BGRA_8888_PRE: + case COGL_PIXEL_FORMAT_BGRX_8888: + case COGL_PIXEL_FORMAT_RGBX_8888: + case COGL_PIXEL_FORMAT_XRGB_8888: + case COGL_PIXEL_FORMAT_XBGR_8888: + case COGL_PIXEL_FORMAT_ARGB_8888: + case COGL_PIXEL_FORMAT_ARGB_8888_PRE: + case COGL_PIXEL_FORMAT_ABGR_8888: + case COGL_PIXEL_FORMAT_ABGR_8888_PRE: + case COGL_PIXEL_FORMAT_RGBA_8888: + case COGL_PIXEL_FORMAT_RGBA_8888_PRE: + case COGL_PIXEL_FORMAT_RGB_565: + case COGL_PIXEL_FORMAT_RGBA_4444: + case COGL_PIXEL_FORMAT_RGBA_4444_PRE: + case COGL_PIXEL_FORMAT_RGBA_5551: + case COGL_PIXEL_FORMAT_RGBA_5551_PRE: + required_gl_format = GL_RGBA; + required_gl_type = GL_UNSIGNED_BYTE; + required_format = COGL_PIXEL_FORMAT_RGBA_8888; + break; - return FALSE; + /* fixed point normalized, 10bpc special case */ + case COGL_PIXEL_FORMAT_ABGR_2101010: + case COGL_PIXEL_FORMAT_ABGR_2101010_PRE: + case COGL_PIXEL_FORMAT_RGBA_1010102: + case COGL_PIXEL_FORMAT_RGBA_1010102_PRE: + case COGL_PIXEL_FORMAT_BGRA_1010102: + case COGL_PIXEL_FORMAT_BGRA_1010102_PRE: + case COGL_PIXEL_FORMAT_XBGR_2101010: + case COGL_PIXEL_FORMAT_XRGB_2101010: + case COGL_PIXEL_FORMAT_ARGB_2101010: + case COGL_PIXEL_FORMAT_ARGB_2101010_PRE: + required_gl_format = GL_RGBA; + required_gl_type = GL_UNSIGNED_INT_2_10_10_10_REV; + required_format = COGL_PIXEL_FORMAT_ABGR_2101010; + break; + + /* floating point */ + case COGL_PIXEL_FORMAT_RGBX_FP_16161616: + case COGL_PIXEL_FORMAT_RGBA_FP_16161616: + case COGL_PIXEL_FORMAT_RGBA_FP_16161616_PRE: + case COGL_PIXEL_FORMAT_BGRX_FP_16161616: + case COGL_PIXEL_FORMAT_BGRA_FP_16161616: + case COGL_PIXEL_FORMAT_XRGB_FP_16161616: + case COGL_PIXEL_FORMAT_ARGB_FP_16161616: + case COGL_PIXEL_FORMAT_XBGR_FP_16161616: + case COGL_PIXEL_FORMAT_ABGR_FP_16161616: + case COGL_PIXEL_FORMAT_BGRA_FP_16161616_PRE: + case COGL_PIXEL_FORMAT_ARGB_FP_16161616_PRE: + case COGL_PIXEL_FORMAT_ABGR_FP_16161616_PRE: + g_assert_not_reached (); + break; + + case COGL_PIXEL_FORMAT_DEPTH_16: + case COGL_PIXEL_FORMAT_DEPTH_24_STENCIL_8: + case COGL_PIXEL_FORMAT_R_16: + case COGL_PIXEL_FORMAT_RG_1616: + case COGL_PIXEL_FORMAT_ANY: + case COGL_PIXEL_FORMAT_YUV: + g_assert_not_reached (); + break; + } + + g_assert (required_format != 0); + + to_required_format = _cogl_driver_pixel_format_to_gl (context, + to, + NULL, + &to_gl_format, + &to_gl_type); + + *gl_format_out = required_gl_format; + *gl_type_out = required_gl_type; + + if (to_required_format != to || + to_gl_format != required_gl_format || + to_gl_type != required_gl_type) + return required_format; + + return to_required_format; } static gboolean @@ -594,7 +681,7 @@ _cogl_driver_gles = _cogl_driver_gl_is_hardware_accelerated, _cogl_gl_get_graphics_reset_status, _cogl_driver_pixel_format_to_gl, - _cogl_driver_read_pixels_format_supported, + _cogl_driver_get_read_pixels_format, _cogl_driver_update_features, _cogl_driver_gl_create_framebuffer_driver, _cogl_driver_gl_flush_framebuffer_state, diff --git a/cogl/cogl/driver/nop/cogl-driver-nop.c b/cogl/cogl/driver/nop/cogl-driver-nop.c index ae35a371f..c2eb0d631 100644 --- a/cogl/cogl/driver/nop/cogl-driver-nop.c +++ b/cogl/cogl/driver/nop/cogl-driver-nop.c @@ -94,7 +94,7 @@ _cogl_driver_nop = _cogl_driver_nop_is_hardware_accelerated, NULL, /* get_graphics_reset_status */ NULL, /* pixel_format_to_gl */ - NULL, /* read_pixels_format_supported */ + NULL, /* _cogl_driver_get_read_pixels_format */ _cogl_driver_update_features, _cogl_driver_nop_create_framebuffer_driver, _cogl_driver_nop_flush_framebuffer_state,