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

[Marek Olšák]

You still have to check all enabled vertex attributes. If you don't want to
loop, use bitmasks. See u_vbuf.c as an example of how to avoid loops.

Marek
On Oct 21, 2015 2:33 PM, "Lofstedt, Marta" <marta.lofstedt at intel.com> wrote:

> > -----Original Message-----
> > From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
> > Behalf Of Tapani Pälli
> > Sent: Wednesday, October 21, 2015 1:25 PM
> > To: Marek Olšák
> > Cc: mesa-dev at lists.freedesktop.org
> > Subject: Re: [Mesa-dev] [PATCH 2/4] mesa: Draw Indirect is not allowed
> > when no vertex array binding exists.
> >
> > 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.
>
> If you have access to the CTS it is these tests that this fixed:
> ES31-CTS.draw_indirect.negative-noVBO-arrays
> ES31-CTS.draw_indirect.negative-noVBO-elements
>
> My understanding is as Tapanis above, I was trying to come up with a method
> of not needing to loop through the VertexAttribPointers.
> Also, I have mis-quoted the spec. I should have only quoted the:
>  "or any enabled vertex arrays" and limit to gles 3.1.
>
> >
> > > 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
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151021/5ec17443/attachment-0001.html>