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

[Eduardo Lima Mitev]

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;
-- 
2.5.3