[Mesa-dev] [PATCH 5/4] mesa: Refactor error checking for GL_TEXTURE_BASE_LEVEL vs texture targets

[Ian Romanick]

On 01/20/2016 08:29 PM, Jason Ekstrand wrote:
> 
> On Jan 20, 2016 6:20 PM, "Ian Romanick" <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>>
>> From: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
>>
>> Add a big spec quotation justifying the error generated, which has
>> changed over the GL versions.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com
> <mailto:ian.d.romanick at intel.com>>
>> ---
>>
>> I intended to send this out with the other four, but I selected the
>> wrong SHA from the list.
>>
>>  src/mesa/main/texparam.c | 31 ++++++++++++++++++++++++-------
>>  1 file changed, 24 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
>> index 89f286c..ee50b5a 100644
>> --- a/src/mesa/main/texparam.c
>> +++ b/src/mesa/main/texparam.c
>> @@ -369,8 +369,31 @@ set_tex_parameteri(struct gl_context *ctx,
>>        if (texObj->BaseLevel == params[0])
>>           return GL_FALSE;
>>
>> +      /* Section 8.10 (Texture Parameters) of the OpenGL 4.5 Core
> Profile spec
>> +       * says:
>> +       *
>> +       *     "An INVALID_OPERATION error is generated if the
> effective target
>> +       *     is either TEXTURE_2D_MULTISAMPLE or
> TEXTURE_2D_MULTISAMPLE_ARRAY,
>> +       *     and pname TEXTURE_BASE_LEVEL is set to a value other
> than zero.
>> +       *
>> +       *     ...
>> +       *
>> +       *     An INVALID_OPERATION error is generated if the effective
> target
>> +       *     is TEXTURE_RECTANGLE and pname TEXTURE_BASE_LEVEL is set
> to any
>> +       *     value other than zero."
> 
> Do we really need the "pname"s?  IMHO, they make it harder to read.

I'm not sure what you mean.  The pnames in the spec quote or ... ?

> Other than that, the series looks good.
> 
> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com
> <mailto:jason.ekstrand at intel.com>>
> 
>> +       *
>> +       * Note that section 3.8.8 (Texture Parameters) of the OpenGL
> 3.3 Core
>> +       * Profile spec said:
>> +       *
>> +       *     "The error INVALID_VALUE is generated if
> TEXTURE_BASE_LEVEL is
>> +       *     set to any value other than zero."
>> +       *
>> +       * We take the 4.5 language as a correction to the 3.3, and we
> implement
>> +       * that on all GL versions.
>> +       */
>>        if ((texObj->Target == GL_TEXTURE_2D_MULTISAMPLE ||
>> -           texObj->Target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY) &&
> params[0] != 0)
>> +           texObj->Target == GL_TEXTURE_2D_MULTISAMPLE_ARRAY ||
>> +           texObj->Target == GL_TEXTURE_RECTANGLE) && params[0] != 0)
>>           goto invalid_operation;
>>
>>        if (params[0] < 0) {
>> @@ -378,12 +401,6 @@ set_tex_parameteri(struct gl_context *ctx,
>>                       "glTex%sParameter(param=%d)", suffix, params[0]);
>>           return GL_FALSE;
>>        }
>> -      if (texObj->Target == GL_TEXTURE_RECTANGLE_ARB && params[0] != 0) {
>> -         _mesa_error(ctx, GL_INVALID_OPERATION,
>> -                     "glTex%sParameter(target=%s, param=%d)", suffix,
>> -                     _mesa_enum_to_string(texObj->Target), params[0]);
>> -         return GL_FALSE;
>> -      }
>>        incomplete(ctx, texObj);
>>
>>        /** See note about ARB_texture_storage below */
>> --
>> 2.5.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>