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

[Ian Romanick]

On 07/30/2013 09:57 AM, Ian Romanick wrote:
> 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.

Any chance of an updated patch in time for 9.2? :)

>> Thanks,
>> Matt
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev