[Mesa-dev] [PATCH] mesa: Reject VertexAttribPointer() with GL_BGRA and !normalized.

[Ian Romanick]

On 07/23/2013 03:50 PM, Matt Turner wrote:
> On Mon, Jul 15, 2013 at 6:37 AM, Fredrik Höglund <fredrik at kde.org> wrote:
>> On Monday 15 July 2013, Kenneth Graunke wrote:
>>> Fixes Piglit's ARB_vertex_attrib_bgra/api-errors test.
>>>
>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>> ---
>>>   src/mesa/main/varray.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
>>> index 529d933..48f15bd 100644
>>> --- a/src/mesa/main/varray.c
>>> +++ b/src/mesa/main/varray.c
>>> @@ -463,6 +463,16 @@ _mesa_VertexAttribPointer(GLuint index, GLint size, GLenum type,
>>>         return;
>>>      }
>>>
>>> +   /* From the GL_ARB_vertex_array_bgra specification:
>>> +    * "The error INVALID_VALUE is generated by VertexAttribPointer if
>>> +    *  size is BGRA and normalized is FALSE."
>>> +    */
>>> +   if (size == GL_BGRA && !normalized) {
>>> +      _mesa_error(ctx, GL_INVALID_VALUE,
>>> +                  "glVertexAttribPointer(BGRA and !normalized)");
>>> +      return;
>>> +   }
>>> +
>>
>> I think this code belongs in update_array(), since it already handles other
>> BGRA errors, and also checks if the extension is supported.
>>
>> This also reminds me that we need to decide if we should make the error
>> code depend on the GL version and entry point.
>>
>> Both OpenGL 3.3 and GL_ARB_vertex_attrib_binding changes the error code
>> to GL_INVALID_OPERATION. The extension only does it for the entry points
>> it adds.
>>
>> I personally don't think it's worth adding the additional complexity for
>> that since the conditions under which the errors to be generated haven't
>> changed.
>
> Ian,
>
> I remember you mentioning when we were showing the interns how to
> write piglit tests that the return value seemed wrong. Can you
> elaborate here? This is holding up Fredrik's ARB_vertex_attrib_binding
> patches.

I believe the ARB_vertex_array_bgra should have specified 
INVALID_OPERATION as the error code.  Section 2.5 (GL Errors) provides 
the general guidance that INVALID_VALUE is generated for "Numeric 
argument out of range".  This is error is that a GLboolean is false 
instead of true.  Clearly false is in range for GLboolean, so 
INVALID_VALUE is not appropriate.

Since OpenGL 3.3 corrects the error code, we should follow that behavior.

> Thanks,
> Matt