On 11/23/2011 11:56 PM, Mathias Fröhlich wrote:

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.

OK, let's leave the name as-is for the time being.

-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to