From a7e8dc6835266bfdd0c17f70acec5585bd7a74d3 Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund <erik.faye-lund@collabora.com> Date: Thu, 8 Feb 2024 11:34:43 +0100 Subject: [PATCH 1/4] mesa/main: fix _mesa_base_tex_format for BGRA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function needs the GLES fix for all APIs now. And it should also handle the sized internal format. Fixes: 4de62731f4d ("mesa/main: add support for EXT_texture_storage") Reviewed-by: Marek Olšák <marek.olsak@amd.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27521> --- src/mesa/main/glformats.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c index 002934aab67f8..116d12e48d8d1 100644 --- a/src/mesa/main/glformats.c +++ b/src/mesa/main/glformats.c @@ -2270,21 +2270,13 @@ _mesa_base_tex_format(const struct gl_context *ctx, GLint internalFormat) case GL_RGBA12: case GL_RGBA16: return GL_RGBA; + case GL_BGRA: + case GL_BGRA8_EXT: + return GL_RGBA; default: ; /* fallthrough */ } - /* GL_BGRA can be an internal format *only* in OpenGL ES (1.x or 2.0). - */ - if (_mesa_is_gles(ctx)) { - switch (internalFormat) { - case GL_BGRA: - return GL_RGBA; - default: - ; /* fallthrough */ - } - } - if (_mesa_has_ARB_ES2_compatibility(ctx) || _mesa_has_OES_framebuffer_object(ctx) || _mesa_is_gles2(ctx)) { -- GitLab From 2b2a6a238ed77d003cbf38c43a2629b8f2a37380 Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund <erik.faye-lund@collabora.com> Date: Fri, 9 Feb 2024 10:19:29 +0100 Subject: [PATCH 2/4] mesa/main: mark GL_BGRA as color-renderable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The EXT_texture_format_BGRA8888-spec is quite clear that this format is color-renderable, so let's mark it properly as such. It should also be texture-filterable, because in the version of OpenGL ES it was written against all texture-formats were filterable to begin with. While we're at it, use the non-EXT version of the enum; it's been in the headers since OpenGL 1.2... Reviewed-by: Marek Olšák <marek.olsak@amd.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27521> --- src/mesa/main/genmipmap.c | 5 ----- src/mesa/main/glformats.c | 6 ++++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/mesa/main/genmipmap.c b/src/mesa/main/genmipmap.c index 519be2578c743..3d3b896a16cc5 100644 --- a/src/mesa/main/genmipmap.c +++ b/src/mesa/main/genmipmap.c @@ -87,15 +87,10 @@ _mesa_is_valid_generate_texture_mipmap_internalformat(struct gl_context *ctx, * not specified with an unsized internal format from table 8.3 or a * sized internal format that is both color-renderable and * texture-filterable according to table 8.10." - * - * GL_EXT_texture_format_BGRA8888 adds a GL_BGRA_EXT unsized internal - * format, and includes it in a very similar looking table. So we - * include it here as well. */ return internalformat == GL_RGBA || internalformat == GL_RGB || internalformat == GL_LUMINANCE_ALPHA || internalformat == GL_LUMINANCE || internalformat == GL_ALPHA || - internalformat == GL_BGRA_EXT || (_mesa_is_es3_color_renderable(ctx, internalformat) && _mesa_is_es3_texture_filterable(ctx, internalformat)); } diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c index 116d12e48d8d1..5fbcb0f3afd83 100644 --- a/src/mesa/main/glformats.c +++ b/src/mesa/main/glformats.c @@ -4027,6 +4027,9 @@ _mesa_is_es3_color_renderable(const struct gl_context *ctx, case GL_RGBA16_SNORM: return _mesa_has_EXT_texture_norm16(ctx) && _mesa_has_EXT_render_snorm(ctx); + case GL_BGRA: + assert(_mesa_has_EXT_texture_format_BGRA8888(ctx)); + return true; default: return false; } @@ -4085,6 +4088,9 @@ _mesa_is_es3_texture_filterable(const struct gl_context *ctx, * for the R32F, RG32F, RGB32F, and RGBA32F formats." */ return _mesa_has_OES_texture_float_linear(ctx); + case GL_BGRA: + assert(_mesa_has_EXT_texture_format_BGRA8888(ctx)); + return true; default: return false; } -- GitLab From d0cc0e74dd9b2aa116f7672388257a0facf6f4bd Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund <erik.faye-lund@collabora.com> Date: Fri, 9 Feb 2024 10:22:15 +0100 Subject: [PATCH 3/4] mesa/main: mark GL_BGRA8_EXT as color-renderable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While this is not quite as clear as in the previous commit, I still believe this is the case, but in a bit of an indirect way: 1. EXT_texture_storage defines that GL_BGRA8_EXT is allowed to be used in certain sitations if *either* EXT_texture_format_BGRA8888 *or* APPLE_texture_format_BGRA8888 is supported. 2. Surprisingly, EXT_texture_format_BGRA8888 (which we do support) does not even mention GL_BGRA8_EXT, only GL_BGRA_EXT. 3. APPLE_texture_format_BGRA8888 on the other hand (which we *don't* support) *does* introduce GL_BGRA8_EXT, and is pretty clear about it being intended for rendering-purposes. But it's written against GLES 1.1 instead of GLES 2 or later, so it doesn't explicitly add it to the required tables. I think the above tells us that GL_BGRA8_EXT is *supposed* to be a color-renderable format, even if the way we currently support it is rather underspecified. It should also be texture-filterable, for the same reason as in the previous commit. In the longer run, we should probably add support for APPLE_texture_format_BGRA8888, which would make things a bit clearer. Reviewed-by: Marek Olšák <marek.olsak@amd.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27521> --- src/mesa/main/glformats.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c index 5fbcb0f3afd83..15f089cc9fcd0 100644 --- a/src/mesa/main/glformats.c +++ b/src/mesa/main/glformats.c @@ -4030,6 +4030,10 @@ _mesa_is_es3_color_renderable(const struct gl_context *ctx, case GL_BGRA: assert(_mesa_has_EXT_texture_format_BGRA8888(ctx)); return true; + case GL_BGRA8_EXT: + assert(_mesa_has_EXT_texture_format_BGRA8888(ctx) && + _mesa_has_EXT_texture_storage(ctx)); + return true; default: return false; } @@ -4091,6 +4095,10 @@ _mesa_is_es3_texture_filterable(const struct gl_context *ctx, case GL_BGRA: assert(_mesa_has_EXT_texture_format_BGRA8888(ctx)); return true; + case GL_BGRA8_EXT: + assert(_mesa_has_EXT_texture_format_BGRA8888(ctx) && + _mesa_has_EXT_texture_storage(ctx)); + return true; default: return false; } -- GitLab From b6b980a904838260540541dcbb34c72de8437b67 Mon Sep 17 00:00:00 2001 From: Erik Faye-Lund <erik.faye-lund@collabora.com> Date: Thu, 8 Feb 2024 11:56:42 +0100 Subject: [PATCH 4/4] mesa/main: work around chrome/firefox bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to flawed logic, Chromium and Firefox thinks EXT_texture_storage allows using GL_BGRA8_EXT for *all* texturing, including things like glTexSubImage2D, which it does not. However, this bug was introduced in Chromium back in 2016, and there's also a lot of Electron apps that bundle outdated versions of Chromium. This means it's going to be a *mess* to fix this properly while staying within the spec. I've opened a ticket with Khronos to consider changing the spec to make this legal, because it seems most other OpenGL implementations allow it. But in the mean time, let's complain a bit, but accept the behavior. This way people can at least run browsers with hardware acceleration again. If we decide to change the spec, we can remove this wording. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/10550 Reviewed-by: Marek Olšák <marek.olsak@amd.com> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27521> --- src/mesa/main/glformats.c | 21 +++++++++++++++++++-- src/mesa/main/glformats.h | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c index 15f089cc9fcd0..f5a871388faf2 100644 --- a/src/mesa/main/glformats.c +++ b/src/mesa/main/glformats.c @@ -2745,7 +2745,7 @@ gles_effective_internal_format_for_format_and_type(GLenum format, * are required for complete checking between format and type. */ static GLenum -_mesa_gles_check_internalformat(const struct gl_context *ctx, +_mesa_gles_check_internalformat(struct gl_context *ctx, GLenum internalFormat) { switch (internalFormat) { @@ -2900,6 +2900,22 @@ _mesa_gles_check_internalformat(const struct gl_context *ctx, if (!_mesa_is_gles3(ctx)) return GL_INVALID_VALUE; return GL_NO_ERROR; + + case GL_BGRA8_EXT: { + /* This is technically speaking out-of-spec. But too many + * applications seems to depend on it, so let's allow it + * together with a small complaint */ + static bool warned = false; + if (!warned) { + _mesa_warning(ctx, + "internalformat = GL_BGRA8_EXT invalid by spec, but too many " + "applications depend on it to error. Please fix the software " + "that causes this problem."); + warned = true; + } + return GL_NO_ERROR; + } + default: return GL_INVALID_VALUE; } @@ -2911,7 +2927,7 @@ _mesa_gles_check_internalformat(const struct gl_context *ctx, * \return error code, or GL_NO_ERROR. */ GLenum -_mesa_gles_error_check_format_and_type(const struct gl_context *ctx, +_mesa_gles_error_check_format_and_type(struct gl_context *ctx, GLenum format, GLenum type, GLenum internalFormat) { @@ -2980,6 +2996,7 @@ _mesa_gles_error_check_format_and_type(const struct gl_context *ctx, if (type != GL_UNSIGNED_BYTE || (internalFormat != GL_BGRA && internalFormat != GL_RGBA8 && + internalFormat != GL_BGRA8_EXT && internalFormat != GL_SRGB8_ALPHA8)) return GL_INVALID_OPERATION; break; diff --git a/src/mesa/main/glformats.h b/src/mesa/main/glformats.h index a402a835962e9..391898d64e768 100644 --- a/src/mesa/main/glformats.h +++ b/src/mesa/main/glformats.h @@ -139,7 +139,7 @@ _mesa_es_error_check_format_and_type(const struct gl_context *ctx, unsigned dimensions); extern GLenum -_mesa_gles_error_check_format_and_type(const struct gl_context *ctx, +_mesa_gles_error_check_format_and_type(struct gl_context *ctx, GLenum format, GLenum type, GLenum internalFormat); extern GLint -- GitLab