On Mon, 2010-07-19 at 00:46 -0700, Chia-I Wu wrote: > [change the subject because of typo and it is not about the patch] > > On Sun, Jul 18, 2010 at 7:31 PM, Keith Whitwell <kei...@vmware.com> wrote: > > If we're reorganising these interfaces, there are a couple of things I'd > > like to see done differently. In particular, within your draw_info > > struct there are a couple of entities which are often better handled as > > persistent state - namely the primitive mode and index buffer binding. > > > > Right now we've got a single call which sets as much state as > > glMultiDrawElementsIndexed, but which can only draw a single primitive. > > > > The ways out of this situation are to either move state out of > > per-primitive transaction, or to modify the interface to accept >1 > > primitive at a time. > > > > My preference would be to move primitive mode & index buffer (and other > > stuff too?) out of your info struct and turn them into new dynamic state > > calls. > > Having done that, it may still be worth considering passing >1 of your > > info structs to the new unified entrypoints... > I considered moving the index buffer state out, but I am not sure how to > formulate int. Consider having a method that supports glMultiDrawElements. > If > the indices pointers point to different buffers in client memory, we need > multiple index buffers.
I think the meaning of those pointers when an element array object is bound is just different offsets into that buffer, as below. > Even if all indices pointers are offsets to the > currently bound index buffer, which may not be aligned to the index size, it > seems to me that multiple index buffers are still desired. I don't really see why that's the case -- GL itself is happy having just a single element array binding point - we're just discussing promoting the same concept into gallium. So basically I think you're worrying about something which isn't an actual requirement. > As i965 does not seem to support multiple index buffers, I am favoring single > index buffer, and skiping direct support for glMultiDrawElements in > pipe_context at the moment. Then, the index buffer state would look like > > struct pipe_index_buffer > { > unsigned index_size; /**< size of an index, in bytes */ > unsigned offset; /**< offset to start of data in buffer */ > struct pipe_resource *buffer; /**< the actual buffer */ > }; This looks good. > pipe_context::draw_vbo is unchanged, but pipe_draw_info is changed to > > struct pipe_draw_info > { > unsigned mode; > unsigned start; > unsigned count; > > unsigned first_instance; > unsigned instance_count; > > int index_bias; > unsigned min_index; > unsigned max_index; > }; > > The last 3 fields are redundant for non-indexed drawing, but they still make > sense (min_index == start, max_index == start + count - 1). Having the fields > in the struct and having a single draw_vbo allow us to support indexed and > non-indexed drawing in a single loop in st_draw_vbo. > > The struct can be extended to include "index buffer index" if multiple index > buffers are desired. The purpose of using a struct here as the argument is > that it is easier to be passed around inside a pipe driver and is easier to > extend. > > I keep mode in the struct so that if in the future, the imaginary > glMultiModeMultiDrawElementsInstancedBaseVertex is introduced, we can support > it by changing draw_vbo to take an array of pipe_draw_info. Hardware (i965) > also seems to send the primitive type along with the draw command. It does, but the driver also sets up a bunch of hardware state in a primitive dependent fashion. I don't really feel strongly about primitive in/out of the struct, but it does seem to be something which ends up being treated as state inside most drivers - ie changes to primitive or reduced_primitive end up triggering an update_state(), while usually the rest of the data there does not. > This is my current thoughts. Suggestions appreciated. I like this a lot. I don't mind either way on mode, so this looks like a good change to me. I guess if we move enough stuff into state, the number of arguments to this call get small enough that it no longer seems necessary to use a struct... Keith _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev