[Mesa-dev] [PATCH] Unified st_context::draw_vbo

[Brian Paul]

On Fri, Jul 16, 2010 at 4:42 PM, Jerome Glisse <glisse at freedesktop.org> wrote:
> On 07/16/2010 03:18 PM, Chia-I Wu wrote:
>>
>> On Sat, Jul 17, 2010 at 2:28 AM, Jakob Bornecrantz<wallbraker at gmail.com>
>>  wrote:
>>>
>>> On Fri, Jul 16, 2010 at 11:10 AM, Chia-I Wu<olvaffe at gmail.com>  wrote:
>>>>
>>>> Hi all,
>>>>
>>>> This patch series replaces
>>>>
>>>>  st_context::draw_arrays
>>>>  st_context::draw_elements,
>>>>  st_context::draw_arrays_instanced
>>>>  st_context::draw_elements_instanced
>>>>  st_context::draw_range_elements
>>>>
>>>> by a single
>>>>
>>>>  st_context::draw_vbo(struct pipe_context *pipe,
>>>>                       const struct pipe_draw_info *info);
>>>>
>>>> The series is too big in size and I have to push it to
>>>>
>>>>  http://cgit.freedesktop.org/~olv/mesa/log/?h=gallium-unified-draw
>>>>
>>>> Since the drivers no longer need to implement all the drawing
>>>> variants, the diffstat says
>>>>
>>>>  29 files changed, 359 insertions(+), 1061 deletions(-)
>>>>
>>>> Its main user (st_draw_vbo) is also simplified.
>>>>
>>>> There are 4 commits in the branch.  The first commit adds the new
>>>> function prototype to the header file.  The second commit implements
>>>> draw_vbo for all pipe drivers.  The third commit converts all state
>>>> trackers (st/mesa, st/vega, and util_draw_vertex_buffer) to use
>>>> draw_vbo.  The last commit removes all other variants from the
>>>> interface.
>>>>
>>>> This looks like a nice cleanup to have to me.  Suggestion?
>>>
>>> Hi Chia-I
>>>
>>> First off it should be pipe_context:: instead of st_context:: since
>>> st_context is the state tracker and not the pipe driver. So when I
>>> first read your email I thought you where unifying the mesa state
>>> tracker and not unifying the draw call in the gallium interface.
>>
>> Aaahhh, my bad.  It should be pipe_context. :(
>>>
>>> I'm also not so sure with passing arguments to a function in a struct.
>>
>> Hmm.  Given the number of arguments needed, a struct seems easier to be
>> passed
>> around, and to grow for new fields in the future.
>>>
>>> But I like the general idea of unifying all the draw functions into a
>>> single one or maybe two.
>
>>
>>
>> Two for DrawElements and DrawArrays respectively?  I chose one because,
>> from a
>> quick look, the difference between the two calls is whether indexing is
>> enabled
>> for most hardware.
>
> I prefer one function, from quick look change to r600 are good (but it's
> pretty trivial for r600 as it was already unified in the driver like
> for nouveau)

Yeah, this looks pretty good.  It makes the interface smaller and
should make it easier to add other draw variations in the future.  You
should wait for Keith's feedback too though.

If we go ahead with this it might be a good idea to add a
util_init_draw_info() function to initialize pipe_draw_info objects.
It would assign reasonable defaults for fields that might not be set
otherwise and give us a bit of future-proofing.

Oh, and you'll have to update the gallium docs.

-Brian