[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.palli at 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