[Mesa-dev] [PATCH v2 02/23] mesa: add KHR_no_error support for glVertexArrayVertexBuffers()

[Samuel Pitoiset]

On 06/06/2017 12:57 PM, Samuel Pitoiset wrote:
> 
> 
> On 06/06/2017 01:04 AM, Fredrik Höglund wrote:
>> On Monday 05 June 2017, Samuel Pitoiset wrote:
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
>>> ---
>>>   src/mapi/glapi/gen/ARB_direct_state_access.xml |  2 +-
>>>   src/mesa/main/varray.c                         | 15 +++++++++++++++
>>>   src/mesa/main/varray.h                         |  6 ++++++
>>>   3 files changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml 
>>> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
>>> index f13a1444a9..cb24d7981c 100644
>>> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
>>> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
>>> @@ -607,7 +607,7 @@
>>>         <param name="stride" type="GLsizei" />
>>>      </function>
>>> -   <function name="VertexArrayVertexBuffers">
>>> +   <function name="VertexArrayVertexBuffers" no_error="true">
>>>         <param name="vaobj" type="GLuint" />
>>>         <param name="first" type="GLuint" />
>>>         <param name="count" type="GLsizei" />
>>> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
>>> index c2c771c173..0cc8b56c64 100644
>>> --- a/src/mesa/main/varray.c
>>> +++ b/src/mesa/main/varray.c
>>> @@ -2273,6 +2273,21 @@ _mesa_BindVertexBuffers(GLuint first, GLsizei 
>>> count, const GLuint *buffers,
>>>   void GLAPIENTRY
>>> +_mesa_VertexArrayVertexBuffers_no_error(GLuint vaobj, GLuint first,
>>> +                                        GLsizei count, const GLuint 
>>> *buffers,
>>> +                                        const GLintptr *offsets,
>>> +                                        const GLsizei *strides)
>>> +{
>>> +   GET_CURRENT_CONTEXT(ctx);
>>> +
>>> +   struct gl_vertex_array_object *vao = _mesa_lookup_vao(ctx, vaobj);
>>
>> _mesa_lookup_vao_err() has an optimization for DSA entry points
>> in that it keeps a reference to the last looked up VAO in the context,
>> and checks if the ID of that object matches the requested ID before
>> it calls _mesa_HashLookup(). The idea is that when a client makes a
>> series of calls like this:
>>
>>     glVertexArrayAttribFormat(vaobj, ...);
>>     glVertexArrayAttribBinding(vaobj, ...);
>>     glEnableVertexArrayAttrib(vaobj, ...);
>>
>> We only look up the object once.
>>
>> By using _mesa_lookup_vao() here, you bypass that optimization.
>>
>> That being said, glVertexArrayVertexBuffers() may be the one vertex
>> array function that doesn't benefit from that optimization.
> 
> Your are right, thanks for reporting this. I will probably change this 
> to try to keep that optimization.

Oh well, that optimization has been removed to 
glVertexArrayVertexBuffer() when Timothy added KHR_no_error.

I would prefer to keep this patch as-is for now and rework that area in 
a separate series. One thing we can do too is to replace _mesa_HashTable 
with an ordinary hash table, and probably some other improvements here 
and there.

> 
>>
>>> +   vertex_array_vertex_buffers(ctx, vao, first, count,
>>> +                               buffers, offsets, strides, true,
>>> +                               "glVertexArrayVertexBuffers");
>>> +}
>>> +
>>> +
>>> +void GLAPIENTRY
>>>   _mesa_VertexArrayVertexBuffers(GLuint vaobj, GLuint first, GLsizei 
>>> count,
>>>                                  const GLuint *buffers,
>>>                                  const GLintptr *offsets, const 
>>> GLsizei *strides)
>>> diff --git a/src/mesa/main/varray.h b/src/mesa/main/varray.h
>>> index 24e37a9bf7..90ca8483ca 100644
>>> --- a/src/mesa/main/varray.h
>>> +++ b/src/mesa/main/varray.h
>>> @@ -379,6 +379,12 @@ extern void GLAPIENTRY
>>>   _mesa_BindVertexBuffers(GLuint first, GLsizei count, const GLuint 
>>> *buffers,
>>>                           const GLintptr *offsets, const GLsizei 
>>> *strides);
>>> +void GLAPIENTRY
>>> +_mesa_VertexArrayVertexBuffers_no_error(GLuint vaobj, GLuint first,
>>> +                                        GLsizei count, const GLuint 
>>> *buffers,
>>> +                                        const GLintptr *offsets,
>>> +                                        const GLsizei *strides);
>>> +
>>>   extern void GLAPIENTRY
>>>   _mesa_VertexArrayVertexBuffers(GLuint vaobj, GLuint first, GLsizei 
>>> count,
>>>                                  const GLuint *buffers,
>>>
>>