[Mesa-dev] [PATCH] mesa: Improve handling of GL_BGRA format in es3 format_and_type checks
Jason Ekstrand
jason at jlekstrand.net
Thu Oct 15 21:33:50 PDT 2015
On Wed, Oct 14, 2015 at 6:56 PM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> We recently added support for GL_BGRA internal format when validating
> combination of format+type+internal_format in Tex(Sub)ImageXD calls
> (to fix https://bugs.freedesktop.org/show_bug.cgi?id=92265).
>
> However, the current implementation handles it as a special case when
> obtaining the effective internal format, treating GL_BGRA as if its
> base format is GL_RGBA execpt for the case of validation.
>
> This causes Mesa to accept a combination like:
> internalFormat = GL_BGRA_EXT, format = GL_RGBA, type = GL_UNSIGNED_BYTE as
> valid arguments to TexImage2D, when it is actually an invalid combination
> per EXT_texture_format_BGRA8888
> <https://www.khronos.org/registry/gles/extensions/EXT/EXT_texture_format_BGRA8888.txt>
>
> This patch makes _mesa_base_tex_format() return GL_BGRA_EXT as base format of
> GL_BGRA_EXT internal format, which is consistent with the extension
> spec. As a result, the code for handling GL_BGRA during validation gets
> simplified.
> ---
> src/mesa/main/glformats.c | 21 +++++++--------------
> 1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index faa6382..e0192fe 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2148,6 +2148,9 @@ _mesa_es_error_check_format_and_type(GLenum format, GLenum type,
> *
> * \return the corresponding \u base internal format (GL_ALPHA, GL_LUMINANCE,
> * GL_LUMANCE_ALPHA, GL_INTENSITY, GL_RGB, or GL_RGBA), or -1 if invalid enum.
> + * When profile is GLES, it will also return GL_BGRA as base format of
> + * GL_BGRA internal format, as specified by extension
> + * EXT_texture_format_BGRA8888.
> *
> * This is the format which is used during texture application (i.e. the
> * texture format and env mode determine the arithmetic used.
> @@ -2215,7 +2218,7 @@ _mesa_base_tex_format(const struct gl_context *ctx, GLint internalFormat)
> if (_mesa_is_gles(ctx)) {
> switch (internalFormat) {
> case GL_BGRA:
> - return GL_RGBA;
> + return GL_BGRA_EXT;
I don't think we can just up-and-change this. It does get used some
places that don't expect GL_BGRA_EXT. For instance, in
copytexture_error_check (teximage.c:2265) we call
_mesa_base_tex_format to get the base tex format for the renderbuffer
or texture and then pass that into _mesa_base_format_component_count()
which doesn't know about GL_BGRA_EXT and so returns -1. Also, in
gallium in st_format.c:1982 they have a line to treat BGRA as RGBA
which I'm guessing is to hack around mesa doing the same. I think we
probably can make this change, but it's going to take some more care.
Given that we completely broke Weston and KWin with this, I don't
think "It passes piglit" is a particularly convincing argument for
this one. Some code-searching would be good and we probably need a
test that actually tests that the format works (not just API errors).
--Jason
> default:
> ; /* fallthrough */
> }
> @@ -2799,18 +2802,8 @@ _mesa_es3_error_check_format_and_type(const struct gl_context *ctx,
> return GL_INVALID_OPERATION;
>
> GLenum baseInternalFormat;
> - if (internalFormat == GL_BGRA_EXT) {
> - /* Unfortunately, _mesa_base_tex_format returns a base format of
> - * GL_RGBA for GL_BGRA_EXT. This makes perfect sense if you're
> - * asking the question, "what channels does this format have?"
> - * However, if we're trying to determine if two internal formats
> - * match in the ES3 sense, we actually want GL_BGRA.
> - */
> - baseInternalFormat = GL_BGRA_EXT;
> - } else {
> - baseInternalFormat =
> - _mesa_base_tex_format(ctx, effectiveInternalFormat);
> - }
> + baseInternalFormat =
> + _mesa_base_tex_format(ctx, effectiveInternalFormat);
>
> if (internalFormat != baseInternalFormat)
> return GL_INVALID_OPERATION;
> @@ -2820,7 +2813,7 @@ _mesa_es3_error_check_format_and_type(const struct gl_context *ctx,
>
> switch (format) {
> case GL_BGRA_EXT:
> - if (type != GL_UNSIGNED_BYTE || internalFormat != GL_BGRA)
> + if (type != GL_UNSIGNED_BYTE || internalFormat != format)
> return GL_INVALID_OPERATION;
> break;
>
> --
> 2.5.3
>
More information about the mesa-dev
mailing list