[Mesa-dev] [PATCH 1/4] vbo: create a new draw function interface for indirect draws

[Ilia Mirkin]

On Mon, Jan 4, 2016 at 10:10 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 12/31/2015 11:55 AM, Ilia Mirkin wrote:
>> This is optional for now in the transition period, but optimally all
>> backends that support indirect draws would switch over to it and we can
>> remove the support for indirect in the "regular" draw function.
>>
>> This should allow a backend to properly support ARB_multi_draw_indirect
>> and ARB_indirect_parameters.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/mesa/vbo/vbo.h            |  15 ++++
>>  src/mesa/vbo/vbo_context.c    |   7 ++
>>  src/mesa/vbo/vbo_context.h    |   6 ++
>>  src/mesa/vbo/vbo_exec_array.c | 166 +++++++++++++++++++++++++-----------------
>>  4 files changed, 127 insertions(+), 67 deletions(-)
>>
>> diff --git a/src/mesa/vbo/vbo.h b/src/mesa/vbo/vbo.h
>> index cef3b8c..0c63bf3 100644
>> --- a/src/mesa/vbo/vbo.h
>> +++ b/src/mesa/vbo/vbo.h
>> @@ -110,6 +110,18 @@ typedef void (*vbo_draw_func)( struct gl_context *ctx,
>>                              struct gl_buffer_object *indirect);
>>
>>
>> +typedef void (*vbo_indirect_draw_func)(
>> +   struct gl_context *ctx,
>> +   GLuint mode,
>> +   struct gl_buffer_object *indirect_data,
>> +   GLsizeiptr indirect_offset,
>> +   unsigned draw_count,
>> +   unsigned stride,
>> +   struct gl_buffer_object *indirect_params,
>> +   GLsizeiptr indirect_params_offset,
>> +   const struct _mesa_index_buffer *ib);
>> +
>> +
>>
>>
>>  /* Utility function to cope with various constraints on tnl modules or
>> @@ -179,6 +191,9 @@ void vbo_always_unmap_buffers(struct gl_context *ctx);
>>
>>  void vbo_set_draw_func(struct gl_context *ctx, vbo_draw_func func);
>>
>> +void vbo_set_indirect_draw_func(struct gl_context *ctx,
>> +                                vbo_indirect_draw_func func);
>> +
>>  void vbo_check_buffers_are_unmapped(struct gl_context *ctx);
>>
>>  void vbo_bind_arrays(struct gl_context *ctx);
>> diff --git a/src/mesa/vbo/vbo_context.c b/src/mesa/vbo/vbo_context.c
>> index 5e1a760..8a196e7 100644
>> --- a/src/mesa/vbo/vbo_context.c
>> +++ b/src/mesa/vbo/vbo_context.c
>> @@ -223,3 +223,10 @@ void vbo_set_draw_func(struct gl_context *ctx, vbo_draw_func func)
>>     vbo->draw_prims = func;
>>  }
>>
>> +
>> +void vbo_set_indirect_draw_func(struct gl_context *ctx,
>> +                                vbo_indirect_draw_func func)
>> +{
>> +   struct vbo_context *vbo = vbo_context(ctx);
>> +   vbo->draw_indirect_prims = func;
>> +}
>> diff --git a/src/mesa/vbo/vbo_context.h b/src/mesa/vbo/vbo_context.h
>> index 6293a8b..11f9b17 100644
>> --- a/src/mesa/vbo/vbo_context.h
>> +++ b/src/mesa/vbo/vbo_context.h
>> @@ -76,6 +76,12 @@ struct vbo_context {
>>      * is responsible for initiating any fallback actions required:
>>      */
>>     vbo_draw_func draw_prims;
>> +
>> +   /* Optional callback for indirect draws. This allows multidraws to not be
>> +    * broken up, as well as for the actual count to be passed in as a separate
>> +    * indirect parameter.
>> +    */
>> +   vbo_indirect_draw_func draw_indirect_prims;
>>  };
>>
>>
>> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
>> index 502b288..3da2d19 100644
>> --- a/src/mesa/vbo/vbo_exec_array.c
>> +++ b/src/mesa/vbo/vbo_exec_array.c
>> @@ -1550,23 +1550,30 @@ vbo_validated_drawarraysindirect(struct gl_context *ctx,
>>
>>     vbo_bind_arrays(ctx);
>>
>> -   memset(prim, 0, sizeof(prim));
>> -   prim[0].begin = 1;
>> -   prim[0].end = 1;
>> -   prim[0].mode = mode;
>> -   prim[0].is_indirect = 1;
>> -   prim[0].indirect_offset = (GLsizeiptr)indirect;
>> -
>> -   /* NOTE: We do NOT want to handle primitive restart here, nor perform any
>> -    * other checks that require knowledge of the values in the command buffer.
>> -    * That would defeat the whole purpose of this function.
>> -    */
>> +   if (vbo->draw_indirect_prims) {
>
> My expectation is that we'll want to transition quickly to whatever new
> interface we decide upon.  Would it be possible to make the existing

Yes, I just didn't want to be on the hook for converting/testing i965.
I think that st/mesa and i965 are going to be the only two users of
this interface.

> loops over vbo->draw_prims be a separate function that is the default
> implementation of vbo->draw_indirect_prims?  At the very least, that

That's... a very good idea :) I'll go and do that shortly and will
resend this patch and that one.

> would make the callers a bit cleaner.  We also wouldn't punish "proper"
> implementations with the extra test, and the indirection ought to
> provide more incentive to "improper" implementations to get on it. :)
>
> I haven't looked very closely at the actual interface yet.  Most of my
> day back at work was going through e-mail, and going to buy a new
> keyboard with a "D" key that isn't all worn out. :)

Sure, no problem. Do you think you'll have time to look at it in the
next day or two though?

Allow me to put in a good word for the Das keyboard -- pleasure to
type on as long as you don't need split/etc.

  -ilia