[Mesa-dev] [PATCH 2/4] mesa: Draw Indirect is not allowed when no vertex array binding exists.

[Tapani Pälli]

On 10/21/2015 01:41 PM, Marek Olšák wrote:
> On Wed, Oct 21, 2015 at 7:16 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> On 10/20/2015 08:54 PM, Marek Olšák wrote:
>>> On Tue, Oct 20, 2015 at 4:19 PM, Marta Lofstedt
>>> <marta.lofstedt at linux.intel.com> wrote:
>>>> From: Marta Lofstedt <marta.lofstedt at intel.com>
>>>>
>>>> OpenGL ES 3.1 spec. section 10.5:
>>>> "An INVALID_OPERATION error is generated if zero is bound
>>>> to VERTEX_ARRAY_BINDING, DRAW_INDIRECT_BUFFER or to
>>>> any enabled vertex array."
>>>>
>>>> Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
>>>> ---
>>>>    src/mesa/main/api_validate.c | 14 ++++++++++++++
>>>>    1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>>>> index c5628f5..7062cbd 100644
>>>> --- a/src/mesa/main/api_validate.c
>>>> +++ b/src/mesa/main/api_validate.c
>>>> @@ -711,6 +711,20 @@ valid_draw_indirect(struct gl_context *ctx,
>>>>          return GL_FALSE;
>>>>       }
>>>>
>>>> +   /*
>>>> +    * OpenGL ES 3.1 spec. section 10.5:
>>>> +    * "An INVALID_OPERATION error is generated if zero is bound to
>>>> +    * VERTEX_ARRAY_BINDING, DRAW_INDIRECT_BUFFER or to any enabled
>>>> +    * vertex array."
>>>> +    * OpenGL 4.5 spec. section 10.4:
>>>> +    * "An INVALID_OPERATION error is generated if  zero is bound to
>>>> +    * DRAW_INDIRECT_BUFFER, or if  no element array buffer is bound"
>>>> +    */
>>>> +   if (!_mesa_is_bufferobj(ctx->Array.ArrayBufferObj)) {
>>>> +      _mesa_error(ctx, GL_INVALID_OPERATION,
>>>> +                  "%s(No VBO is bound)", name);
>>>> +   }
>>> NAK.
>>>
>>> VERTEX_ARRAY_BINDING is a VAO. Array.ArrayBufferObj is from glBindBuffer.
>>
>> This check is valid, it is not against VERTEX_ARRAY_BINDING. Note "any
>> enabled vertex array", we hit this weird situation when client has a VAO
>> bound and has enabled vertex attrib array but has not bound any VBO to it.
> No, it's invalid. The check has absolutely nothing to do with enabled
> vertex arrays and draw calls. Absolutely nothing. glBindBuffer changes
> a latched state, which means it doesn't do anything by itself, it only
> affects other functions that change states. The functions affected by
> glBindBuffer(GL_ARRAY_BUFFER, ..) are glVertexAttribPointer, etc. not
> glDraw*. If you called glBindBuffer(GL_ARRAY_BUFFER, ..) right before
> a Draw call, it wouldn't do anything to vertex arrays and buffers, but
> it would pass the check.

OK my understanding was that reason why this change fixes the bug is 
that ctx->Array.ArrayBufferObj is 0 for the default VAO and never 0 when 
vertex array buffer binding has been set, and this would happen when we 
would have an VBO bound. I will spend some more time to understand this.

> Now, where does this patch check "enabled vertex arrays"? Nowhere. It
> doesn't check VERTEX_ARRAY_BINDING, it doesn't check
> DRAW_INDIRECT_BUFFER, and it doesn't check enabled vertex arrays. That
> whole comment is completely useless there.
>
> Sorry if I'm too direct, but you should really think more before
> making such statements and giving Reviewed-by.

// Tapani