Hi Brian, Thanks for the review so far! Two comments/questions below:
On Friday, 16 February 2018 00:27:16 CET Brian Paul wrote: > > +/** > > + * Initialize inputs. > > + */ > > +void > > +_vbo_array_init(struct vbo_inputs *inputs); > > + > > + > > +/** > > + * Update the gl_vertex_array array inside the vbo_inputs structure > > + * provided the current _VPMode, the provided vao and > > + * the vao's enabled arrays filtered by the filter bitmask. > > + */ > > +void > > +_vbo_update_inputs(struct gl_context *ctx, struct vbo_inputs *inputs); > > If that struct and the two prototypes aren't going to be used outside > the VBO module, they should go into vbo_private.h Well, currently yes. But in a later step I plan to push the gl_vertex_array[] thing into the backend drivers, switch over the individual drivers to direct _DrawVAO use and fade out that vbo_inputs struct for most drivers. Remember the binding information present in the VAO struct shall be reused without rescanning the inputs where they stem from. Consequently, the tool functions here will be called from those individual driver that will still use the gl_vertex_array struct and those drivers need a public entry function. That is also the reason why there is a seperate init function for this almost trivial currently vbo module internal task. The init function would also be of public use in that later step. Finally that is even the reason for the seperate 'struct vbo_inputs' that could be as well open coded in the vbo module. But pushing that into driver backends is much more easy when this is a seperate tool struct with methods on it. So if you really require me to make them private now I can move them now, but a later patch will move them back here or somewhere similar. > > + /* The rest must be current inputs. */ > > + update_current_inputs(ctx, inputs, ~enable & VERT_BIT_ALL); > > The & VERT_BIT_ALL isn't really needed, is it? I guess there's no harm > though. Currently this is a logical noop and I think the compiler detects that. Originally the code stems from the time where the mask was 33 bits and there it was required. In the case we do further play with the VERT_ATTRIB_* list and VERT_BITs it may again be required an I like to be prepared for that, especially if its that easy. I will take care of the rest of the comments! best Mathias _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev