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

[Chia-I Wu]

On Sat, Jul 17, 2010 at 8:38 AM, Brian Paul <brian.e.paul at gmail.com> wrote:
> 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.
Sounds good.  A reasonable default for instance_count should be 1.  Other
fields could stay 0.  I will also add util_draw_arrays(mode, start, count) for
simple non-indexed drawing.
> Oh, and you'll have to update the gallium docs.
Right.  Will do.
> -Brian
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>

-- 
olv at LunarG.com