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