Re: [Mesa-dev] [PATCH 6/6] mesa: Use VERT_ATTRIB_* indexed array in gl_array_object.

2011-11-28 Thread Mathias Fröhlich

Hi,

On Sunday, November 27, 2011 22:52:29 Eric Anholt wrote:
 Not an issue for this patch, but I noticed that there's this
 _mesa_update_array_object_max_element that does something similar, but
 doesn't follow this rule and would trash the _MaxElement value.  Some
 day, the body of this function should probably be moved to that one, and
 state.c should call it.
 
 It's also a bit disturbing that the code for which array to look at
 based on program state is duplicated between vbo and here.
Thanks, agreed.

There is really a huge pile of cleanup possible in this area.
I had included some of the 'easy cleanups' in the v1 series but it turned out 
that there is some surprising clause in one of the extension specs that I just 
missed. So for now I resorted to the bare minimum to get the array change in. 
Then start thinking about cleanups enabled with this change.

 Other than the one patch with comments, the series overall looks like a
 great clenaup and is:
 
 Reviewed-by: Eric Anholt e...@anholt.net
Thanks for looking into this!

Mathias
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 6/6] mesa: Use VERT_ATTRIB_* indexed array in gl_array_object.

2011-11-27 Thread Eric Anholt
On Sun, 27 Nov 2011 18:19:00 +0100, Mathias Fröhlich 
mathias.froehl...@gmx.net wrote:
 Replace the distinct struct gl_client_array members in gl_array_object by
 an array of gl_client_arrays indexed by VERT_ATTRIB_*.
 Renumber the vertex attributes slightly to keep the old semantics of the
 distinct array members. Make use of the upper 32 bits in VERT_BIT_*.
 Update all occurances of the distinct struct members with the array
 equivalents.
 
 Signed-off-by: Mathias Froehlich mathias.froehl...@web.de

 diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c
 index f57579c..6c0dc7c 100644
 --- a/src/mesa/main/state.c
 +++ b/src/mesa/main/state.c
 @@ -83,98 +83,101 @@ update_arrays( struct gl_context *ctx )
 struct gl_array_object *arrayObj = ctx-Array.ArrayObj;
 GLuint i, min = ~0;
  
 -   /* find min of _MaxElement values for all enabled arrays */
 +   /* find min of _MaxElement values for all enabled arrays.
 +* Note that the generic arrays always take precedence over
 +* the legacy arrays.
 +*/

Not an issue for this patch, but I noticed that there's this
_mesa_update_array_object_max_element that does something similar, but
doesn't follow this rule and would trash the _MaxElement value.  Some
day, the body of this function should probably be moved to that one, and
state.c should call it.

It's also a bit disturbing that the code for which array to look at
based on program state is duplicated between vbo and here.

Other than the one patch with comments, the series overall looks like a
great clenaup and is:

Reviewed-by: Eric Anholt e...@anholt.net


pgpxsME4zfxT4.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev