[Mesa-dev] [PATCH 08/10] mesa: Use VERT_ATTRIB_* indexed array in gl_array_object

[Mathias Fröhlich]

Hi Brian,

Thanks for looking into the changes.

On Wednesday, November 23, 2011 17:24:23 Brian Paul wrote:
> I think there's a few places where we need to map GLenums for vertex
> arrays to VERT_ATTRIB_x values.  Maybe there should be a helper
> function for that:
> 
> gl_vert_attrib
> array_enum_to_vert_attrib(GLenum vertexArray)
> {
>     switch (vertexArray) {
>     case GL_VERTEX_ARRAY:
>        return VERT_ATTRIB_POS;
>     case GL_VERTEX_COLOR:
>        return VERT_ATTRIB_COLOR0;
>     ...
> }
> 
> Then the code for all those switch cases could be collapsed together.

I agree that this is a good utility function to have.

The motivation of this particular change is to be as mechanic as possible. 
This change is already huge enough and provides plenty of opportunities to 
break something.

So if you don't mind I would prefer to keep this change seperate from a 
possible followup providing and using the above utility function.

> > -   struct gl_client_array VertexAttrib[MAX_VERTEX_GENERIC_ATTRIBS];
> > +   /** Vertex attribute arrays */
> > +   struct gl_client_array VertexAttrib[VERT_ATTRIB_MAX];
> 
> At some point it might be nice to rename VertexAttrib to simply be
> Array.  It would be a bit more concise.

Ok, I could easily fold that into this current change the total amount of 
changes is hardly the same.
In fact during implementation of this patch set, I had that variable named 
differently to make sure I really get all transitions right by giving the 
compiler a chance to point me to any old, different indexed use of this 
variable.
Hmm, just one thing before I really apply this change to my tree: It's also 
easier to grep for VertexAttrib and find meaningful answers than grep for Array 
and find just the same answers - even with \<Array\> you are flooded.
So, if you tell me that you don't care for grep the updated patch contains 
this change.

> > --- a/src/mesa/main/state.c
> > +++ b/src/mesa/main/state.c
> > @@ -87,94 +87,94 @@ update_arrays( struct gl_context *ctx )
> > 
> >      /* 0 */
> >      if (ctx->VertexProgram._Current
> > 
> > -&&  arrayObj->VertexAttrib[VERT_ATTRIB_POS].Enabled) {
> > -      min = update_min(min,&arrayObj->VertexAttrib[VERT_ATTRIB_POS]);
> > +&&  arrayObj->VertexAttrib[VERT_ATTRIB_GENERIC0].Enabled) {
> > +      min =
> > update_min(min,&arrayObj->VertexAttrib[VERT_ATTRIB_GENERIC0]);
> > 
> >      }
> > 
> > -   else if (arrayObj->Vertex.Enabled) {
> > -      min = update_min(min,&arrayObj->Vertex);
> > +   else if (arrayObj->VertexAttrib[VERT_ATTRIB_POS].Enabled) {
> > +      min = update_min(min,&arrayObj->VertexAttrib[VERT_ATTRIB_POS]);
> > 
> >      }
> 
> Hmmm, do the generic arrays take precedence over the legacy arrays?  I
> don't remember off-hand.  There should probably be a comment about
> that either way.

Yes, in any variant of shader programs, the generic arrays are stronger than 
the conventional ones.

Mathias