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

[Marek Olšák]

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.

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.

Marek