[Mesa-dev] [PATCH] mesa/glformats: Bypass resolving effective internal format for GL_BGRA

[Eduardo Lima Mitev]

Anyone willing to take a look?

Thanks!

Eduardo

On 02/10/2016 02:57 PM, Eduardo Lima Mitev wrote:
> Currently, when validating format and type on ES3, we treat GL_BGRA as a
> special case when obtaining the effective internal format from the format
> and type. This is because _mesa_base_tex_format() returns GL_RGBA as base
> format for GL_BGRA (and quite a few code paths depend on this behavior).
> 
> However, this makes calls to glTexImage with:
> internalFormat=GL_RGBA format=GL_BGRA_EXT and type=GL_UNSIGNED_BYTE
> to pass when it should generate an invalid operation (since format and
> internalformat mismatch,
> see https://bugs.freedesktop.org/show_bug.cgi?id=92265#c17)
> 
> This patch bypasses the calculation of the effective internal format
> altogether, if either format or internalformat is GL_BGRA. It is just simpler
> to check that both format and internalformat match in the presence of
> GL_BGRA; than tricking the calculation of the effective internal format
> to avoid handling this case incorrectly.
> 
> So effectively, what it does is moving the handling of the special case to
> a higher level, leaving the effective internal format calculation clearer
> and more concise.
> 
> It also fixes piglit test:
> 
> spec at ext_texture_format_bgra8888@api-errors
> ---
>  src/mesa/main/glformats.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/src/mesa/main/glformats.c b/src/mesa/main/glformats.c
> index f528444..9983ab9 100644
> --- a/src/mesa/main/glformats.c
> +++ b/src/mesa/main/glformats.c
> @@ -2663,27 +2663,26 @@ _mesa_es3_error_check_format_and_type(const struct gl_context *ctx,
>      *     CopyTex* commands. In these cases, the GL is required to operate
>      *     as if the effective internal format was used as the internalformat
>      *     when specifying the texture data."
> +    *
> +    * However, there is a special case when either format or internalformat is
> +    * GL_BGRA (from EXT_texture_format_BGRA8888). It comes from the fact that
> +    * _mesa_base_tex_format() returns a base format of GL_RGBA for GL_BGRA.
> +    * This makes perfect sense if you're asking the question, "what channels
> +    * does this format have?" However, the code below also checks if two
> +    * internal formats match in the ES3 sense, so it doesn't work well for
> +    * GL_BGRA. Hence, we bypass the resolution of the effective internal format
> +    * altogether, if we have GL_BGRA in either format or internalformat.
>      */
> -   if (_mesa_is_enum_format_unsized(internalFormat)) {
> +   if (_mesa_is_enum_format_unsized(internalFormat) &&
> +       internalFormat != GL_BGRA && format != GL_BGRA) {
>        GLenum effectiveInternalFormat =
>           _mesa_es3_effective_internal_format_for_format_and_type(format, type);
>  
>        if (effectiveInternalFormat == GL_NONE)
>           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);
> -      }
> +      GLenum baseInternalFormat =
> +         _mesa_base_tex_format(ctx, effectiveInternalFormat);
>  
>        if (internalFormat != baseInternalFormat)
>           return GL_INVALID_OPERATION;
>