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

[Ian Romanick]

On 10/07/2015 03:58 PM, Jason Ekstrand wrote:
> 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?"
                                                   ^^^ does

With that fixed,

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> +          * 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);
> +      }
>  
>        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:
>