Hi Brian, > Looks good to me. Definitely seems like another step in the right > direction. > > Just two minor comments: > > 1. A lot of local vars are named "glarray" and "glbinding". I'd suggest > dropping the "gl" part since it doesn't really help with readability > (everything's "gl" in this code :-) ).
Will do! It initially helped me to grep for this kind of stuff. The i965 parts do so with the 'struct gl_vertex_array *glarray' probably to distinguish from intel driver internal array/attrib/binding structs. But yes, may be I can then also get rid of some newlines ... > 2. In a future patch you could probably simplify st_pipe_vertex_format() > to be: > > enum pipe_format > st_pipe_vertex_format(const struct gl_array_attributes *attribs) Definitely. The next lower layer parts will get some cleanup once all the surrounding stuff is in place. Expecially the gallium backends gain from direct _DrawVAO use as in some next step I will scan for duplicate bindings in the VAO and do only use the net information about that unique binding set when translating to gallium. I am still experimenting with this to see what fits best for gallium and i965. I also had one attempt to factor out a struct gl_vertex_format that only describes exactly that. That could then be fed into st_pipe_vertex_format and the equivalent brw_get_vertex_surface_type in the i965 driver. But lets finish at first the complete switch to _DrawVAO. This current patch aims to mostly prepare those current well cpu optimized backend drivers to not use gl_vertex_array anymore and for that the current values must be represented as aequivalent structs as they appear in the VAO. > Reviewed-by: Brian Paul <bri...@vmware.com> Thanks for the review!! best Mathias _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev