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

[Tapani Pälli]

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.

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