[Mesa-dev] [PATCH 03/11] mesa: Implement _mesa_all_varyings_in_vbos.

[Mathias Fröhlich]

Hi,

On Thursday, June 23, 2016 16:53:59 Fredrik Höglund wrote:
> On Friday 17 June 2016, Mathias.Froehlich at gmx.net wrote:
> > From: Mathias Fröhlich <mathias.froehlich at web.de>
> > 
> > Implement the equivalent of vbo_all_varyings_in_vbos for
> > vertex array objects.
> > 
> > Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
> > ---
> >  src/mesa/main/arrayobj.c | 35 +++++++++++++++++++++++++++++++++++
> >  src/mesa/main/arrayobj.h |  4 ++++
> >  2 files changed, 39 insertions(+)
> > 
> > diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> > index 9c3451e..041ee63 100644
> > --- a/src/mesa/main/arrayobj.c
> > +++ b/src/mesa/main/arrayobj.c
> > @@ -359,6 +359,41 @@ _mesa_update_vao_client_arrays(struct gl_context *ctx,
> >  }
> >  
> >  
> > +bool
> > +_mesa_all_varyings_in_vbos(const struct gl_vertex_array_object *vao)
> > +{
> > +   /* Walk those enabled arrays that have the default vbo attached */
> > +   GLbitfield64 mask = vao->_Enabled & ~vao->VertexAttribBufferMask;
> > +
> > +   while (mask) {
> > +      /** We do not use u_bit_scan64 as we can here walk
> > +       *  multiple attrib arrays at once
> > +       */
> > +      const int i = ffsll(mask) - 1;
> > +      const struct gl_vertex_attrib_array *attrib_array =
> > +         &vao->VertexAttrib[i];
> > +      const struct gl_vertex_buffer_binding *buffer_binding =
> > +         &vao->VertexBinding[attrib_array->VertexBinding];
> > +
> > +      /* Only enabled arrays shall appear in the _Enabled bitmask */
> > +      assert(attrib_array->Enabled);
> > +      /* We have already masked out vao->VertexAttribBufferMask  */
> > +      assert(!_mesa_is_bufferobj(buffer_binding->BufferObj));
> > +
> > +      /* Bail out once we find the first non vbo with a non zero stride */
> > +      if (buffer_binding->Stride != 0)
> > +         return false;
> 
> I'm not sure if this is correct.  The default value for Stride is 16,
> not 0.  The only way Stride can be zero in a binding point that doesn't
> have a buffer object bound is if the user has explicitly called
> glBindVertexBuffer() with both the buffer and stride parameters set
> to zero.
> 
> StrideB in gl_client_array on the other hand is always zero when the array
> is one of the currval arrays managed by the VBO context.  It is never zero
> when the array is a user array that has been specified with gl*Pointer().
> 
> I think the point of vbo_all_varyings_in_vbos() is to return false if any
> enabled array doesn't have a VBO bound, and is not one of the currval
> arrays.

Additionally vbo_all_varyings_in_vbos() treats zero stride user
arrays like a current vertex attribute values. Already because
vbo_all_varyings_in_vbos() does not distinguish between a user
zero stride array and a current attribute value which is presented likewise.
Also I believe it's legal to call glBindVertexBuffer() with zero buffer
and stride - or am I wrong here?
So IMO what you write would result in a change of behavior.

Mathias