From 8e65e510d48be2d9e223b5b074716bb6946f26ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Mon, 10 Oct 2022 22:13:05 +0200 Subject: cogl/framebuffer/gl: Move most read restriction to drivers OpenGL requires more hand holding in the driver regarding what pixel memory layouts can be written when calling glReadPixels(), compared to GLES2. Lets move the details of this logic to the corresponding backends, so in the future, the GLES2 backend can be adapted to handle more formats, without placing that logic in the generic layer. Part-of: --- cogl/cogl/cogl-driver.h | 6 +++++ cogl/cogl/cogl-private.h | 2 +- cogl/cogl/driver/gl/cogl-framebuffer-gl.c | 40 ++++++++++++++++------------- cogl/cogl/driver/gl/gl/cogl-driver-gl.c | 12 ++++++++- cogl/cogl/driver/gl/gles/cogl-driver-gles.c | 13 ++++++++++ cogl/cogl/driver/nop/cogl-driver-nop.c | 1 + 6 files changed, 54 insertions(+), 20 deletions(-) diff --git a/cogl/cogl/cogl-driver.h b/cogl/cogl/cogl-driver.h index a87e57285..4b639df87 100644 --- a/cogl/cogl/cogl-driver.h +++ b/cogl/cogl/cogl-driver.h @@ -70,6 +70,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); + gboolean (* update_features) (CoglContext *context, GError **error); diff --git a/cogl/cogl/cogl-private.h b/cogl/cogl/cogl-private.h index f842047f0..16d01a651 100644 --- a/cogl/cogl/cogl-private.h +++ b/cogl/cogl/cogl-private.h @@ -50,7 +50,7 @@ typedef enum COGL_PRIVATE_FEATURE_TEXTURE_FORMAT_HALF_FLOAT, COGL_PRIVATE_FEATURE_UNPACK_SUBIMAGE, COGL_PRIVATE_FEATURE_SAMPLER_OBJECTS, - COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT, + COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE, COGL_PRIVATE_FEATURE_FORMAT_CONVERSION, COGL_PRIVATE_FEATURE_QUERY_FRAMEBUFFER_BITS, COGL_PRIVATE_FEATURE_QUERY_TEXTURE_PARAMETERS, diff --git a/cogl/cogl/driver/gl/cogl-framebuffer-gl.c b/cogl/cogl/driver/gl/cogl-framebuffer-gl.c index dfd9b9913..f809d9382 100644 --- a/cogl/cogl/driver/gl/cogl-framebuffer-gl.c +++ b/cogl/cogl/driver/gl/cogl-framebuffer-gl.c @@ -429,6 +429,9 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver *driver, 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 pack_invert_set; int status = FALSE; @@ -466,21 +469,21 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver *driver, else pack_invert_set = FALSE; - /* Under GLES only GL_RGBA with GL_UNSIGNED_BYTE as well as an - implementation specific format under - GL_IMPLEMENTATION_COLOR_READ_FORMAT_OES and - GL_IMPLEMENTATION_COLOR_READ_TYPE_OES is supported. We could try - to be more clever and check if the requested type matches that - but we would need some reliable functions to convert from GL - types to Cogl types. For now, lets just always read in - GL_RGBA/GL_UNSIGNED_BYTE and convert if necessary. We also need - to use this intermediate buffer if the rowstride has padding - because GLES does not support setting GL_ROW_LENGTH */ - if ((!_cogl_has_private_feature - (ctx, COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT) && - (gl_format != GL_RGBA || gl_type != GL_UNSIGNED_BYTE || - cogl_bitmap_get_rowstride (bitmap) != 4 * width)) || - (required_format & ~COGL_PREMULT_BIT) != (format & ~COGL_PREMULT_BIT)) + 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); + + 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)) { CoglBitmap *tmp_bmp; CoglPixelFormat read_format; @@ -488,9 +491,10 @@ cogl_gl_framebuffer_read_pixels_into_bitmap (CoglFramebufferDriver *driver, uint8_t *tmp_data; gboolean succeeded; - if (_cogl_has_private_feature - (ctx, COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT)) - read_format = required_format; + if (is_read_pixels_format_supported) + { + read_format = required_format; + } else { read_format = COGL_PIXEL_FORMAT_RGBA_8888; diff --git a/cogl/cogl/driver/gl/gl/cogl-driver-gl.c b/cogl/cogl/driver/gl/gl/cogl-driver-gl.c index 7c9449f5b..a05fe61b6 100644 --- a/cogl/cogl/driver/gl/gl/cogl-driver-gl.c +++ b/cogl/cogl/driver/gl/gl/cogl-driver-gl.c @@ -354,6 +354,15 @@ _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) +{ + return TRUE; +} + static gboolean _cogl_get_gl_version (CoglContext *ctx, int *major_out, @@ -517,7 +526,7 @@ _cogl_driver_update_features (CoglContext *ctx, } COGL_FLAGS_SET (private_features, - COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_FORMAT, TRUE); + COGL_PRIVATE_FEATURE_READ_PIXELS_ANY_STRIDE, TRUE); COGL_FLAGS_SET (private_features, COGL_PRIVATE_FEATURE_ANY_GL, TRUE); COGL_FLAGS_SET (private_features, COGL_PRIVATE_FEATURE_FORMAT_CONVERSION, TRUE); @@ -582,6 +591,7 @@ _cogl_driver_gl = _cogl_gl_get_graphics_reset_status, _cogl_driver_pixel_format_from_gl_internal, _cogl_driver_pixel_format_to_gl, + _cogl_driver_read_pixels_format_supported, _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 59cf0c07e..41387a3eb 100644 --- a/cogl/cogl/driver/gl/gles/cogl-driver-gles.c +++ b/cogl/cogl/driver/gl/gles/cogl-driver-gles.c @@ -271,6 +271,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) +{ + if (glformat == GL_RGBA && gltype == GL_UNSIGNED_BYTE) + return TRUE; + + return FALSE; +} + static gboolean _cogl_get_gl_version (CoglContext *ctx, int *major_out, @@ -477,6 +489,7 @@ _cogl_driver_gles = _cogl_gl_get_graphics_reset_status, _cogl_driver_pixel_format_from_gl_internal, _cogl_driver_pixel_format_to_gl, + _cogl_driver_read_pixels_format_supported, _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 2cde8576b..f18e78873 100644 --- a/cogl/cogl/driver/nop/cogl-driver-nop.c +++ b/cogl/cogl/driver/nop/cogl-driver-nop.c @@ -95,6 +95,7 @@ _cogl_driver_nop = NULL, /* get_graphics_reset_status */ NULL, /* pixel_format_from_gl_internal */ NULL, /* pixel_format_to_gl */ + NULL, /* read_pixels_format_supported */ _cogl_driver_update_features, _cogl_driver_nop_create_framebuffer_driver, _cogl_driver_nop_flush_framebuffer_state, -- cgit v1.2.1