[Mesa-dev] [PATCH] mesa: Correctly handle GL_BGRA_EXT in ES3 format_and_type checks

[Michael Schellenberger Costa]

Hi,
Am 08.10.2015 um 00:58 schrieb Jason Ekstrand:
> The EXT_texture_format_BGRA8888 extension (which mesa supports 
> unconditionally) adds a new format and internal format called 
> GL_BGRA_EXT. Previously, this was not really handled at all in 
> _mesa_ex3_error_check_format_and_type.  When the checks were 
> tightened in commit f15a7f3c, we accidentally tightened things too 
> far and GL_BGRA_EXT would always cause an error to be thrown.
> 
> There were two primary issues here.  First, is that 
> _mesa_es3_effective_internal_format_for_format_and_type didn't 
> handle the GL_BGRA_EXT format.  Second is that it blindly uses 
> _mesa_base_tex_format which returns GL_RGBA for GL_BGRA_EXT.  This 
> commit fixes both of these issues as well as adds explicit checks 
> that GL_BGRA_EXT is only ever used with GL_BGRA_EXT and 
> GL_UNSIGNED_BYTE.
> 
> Signed-off-by: Jason Ekstrand <jason.ekstrand at intel.com> Bugzilla: 
> https://bugs.freedesktop.org/show_bug.cgi?id=92265 Cc: Eduardo
> Lima Mitev <elima at igalia.com> --- src/mesa/main/glformats.c | 21 
> +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 
> deletions(-)
> 
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
>  index 7dab33c..843ec02 100644 --- a/src/mesa/main/glformats.c +++ 
> b/src/mesa/main/glformats.c @@ -2678,6 +2678,7 @@ 
> _mesa_es3_effective_internal_format_for_format_and_type(GLenum 
> format, * internal formats, they do not correspond to GL
> constants, so the base * format is returned instead. */ +
> case GL_BGRA_EXT: case GL_LUMINANCE_ALPHA: case GL_LUMINANCE: case 
> GL_ALPHA: @@ -2797,8 +2798,19 @@ 
> _mesa_es3_error_check_format_and_type(const struct gl_context *ctx,
> if (effectiveInternalFormat == GL_NONE) return 
> GL_INVALID_OPERATION;
> 
> -      GLenum baseInternalFormat = - _mesa_base_tex_format(ctx,
> effectiveInternalFormat); +      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 doe this format have?" + * However, if
> we're trying to determine if two internal formats + * match in the
> ES3 sense, we actually want GL_BGRA.
Shouldnt this be GL_BGRA_EXT at the end of the comment?

Michael
> +          */ +         baseInternalFormat = GL_BGRA_EXT; +      } 
> else { +         baseInternalFormat = + _mesa_base_tex_format(ctx,
> effectiveInternalFormat); +      }
> 
> if (internalFormat != baseInternalFormat) return 
> GL_INVALID_OPERATION; @@ -2807,6 +2819,11 @@ 
> _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) +         return 
> GL_INVALID_OPERATION; +      break; + case GL_RGBA: switch (type) {
> case GL_UNSIGNED_BYTE:
>