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!!



mesa-dev mailing list

Reply via email to