Re: [Mesa-dev] [PATCH] mesa: refactor target error checking in glGetTexLevelParameter

[Tapani Pälli]

On 08/13/2015 03:47 PM, Timothy Arceri wrote:

On Thu, 2015-08-13 at 15:01 +0300, Tapani Pälli wrote:

On 08/13/2015 02:51 PM, Timothy Arceri wrote:

On Thu, 2015-08-13 at 14:05 +0300, Tapani Pälli wrote:

With non-dsa functions we need to do target error checking before
_mesa_get_current_tex_object which would just call _mesa_problem without
raising GL_INVALID_ENUM error. In other places of Mesa, target gets
checked
before this call. For dsa functions, we do not need this check. Texture
has been created already with a proper target value.

Fixes failures in:
     ES31-CTS.texture_storage_multisample.APIGLGetTexLevelParameterifv.*

Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
---
   src/mesa/main/texparam.c | 26 +++++++++++++++++++-------
   1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
index c0611c3..81c209f 100644
--- a/src/mesa/main/texparam.c
+++ b/src/mesa/main/texparam.c
@@ -1562,6 +1562,19 @@ invalid_pname:
                  _mesa_enum_to_string(pname));
   }

+static bool
+valid_tex_level_parameteriv_target(struct gl_context *ctx, GLenum
target,
+                                   bool dsa)
+{
+   const char *suffix = dsa ? "ture" : "";
+   if (!legal_get_tex_level_parameter_target(ctx, target, dsa)) {
+      _mesa_error(ctx, GL_INVALID_ENUM,
+                  "glGetTex%sLevelParameter[if]v(target=%s)", suffix,
+                  _mesa_enum_to_string(target));
+      return false;
+   }
+   return true;
+}

   /**
    * This isn't exposed to the rest of the driver because it is a part
of the
@@ -1585,13 +1598,6 @@ get_tex_level_parameteriv(struct gl_context *ctx,
         return;
      }

-   if (!legal_get_tex_level_parameter_target(ctx, target, dsa)) {
-      _mesa_error(ctx, GL_INVALID_ENUM,
-                  "glGetTex%sLevelParameter[if]v(target=%s)", suffix,
-                  _mesa_enum_to_string(target));
-      return;
-   }
-_mesa_GetTexLevelParameterfv( GLenum target, GLint
level,
      GLint iparam;
      GET_CURRENT_CONTEXT(ctx);

+   if (!valid_tex_level_parameteriv_target(ctx, target, false))
+      return;
+
      texObj = _mesa_get_current_tex_object(ctx, target);
      if (!texObj)
         return;
@@ -1636,6 +1645,9 @@ _mesa_GetTexLevelParameteriv( GLenum target, GLint
level,
      struct gl_texture_object *texObj;
      GET_CURRENT_CONTEXT(ctx);

+   if (!valid_tex_level_parameteriv_target(ctx, target, false))
+      return;
+
      texObj = _mesa_get_current_tex_object(ctx, target);
      if (!texObj)
         return;

I think you forgot to add calls to the new validation for the dsa
functions.

Nope, the motivation for leaving out the checks for dsa functions is
that the texture target enum has been validated already for existing
textures, it cannot be invalid at this point. At least this is how I
understand it.

I don't think thats right. The dsa functions are just passed a texture id, the
id may find you a texture that exists but you need to make sure the texture is
a valid type for use with the LevelParameter*

Ah right, yep a check is required. I'll send a v2

// Tapani

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev