[Mesa-dev] [RFC PATCH 1/3] gallium: decrease the size of pipe_vertex_buffer - 24 -> 16 bytes

[Marek Olšák]

On Wed, Apr 12, 2017 at 5:59 AM, Brian Paul <brianp at vmware.com> wrote:
> On 04/11/2017 02:15 PM, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> New interface:
>>
>> union pipe_buffer_binding {
>>     struct pipe_resource *buffer;  /**< the actual buffer */
>>     const void *user_buffer;  /**< pointer to a user buffer */
>> };
>>
>> struct pipe_vertex_buffer
>> {
>>     uint16_t stride;    /**< stride to same attrib in next vertex, in
>> bytes */
>>     bool is_user_buffer;
>>     unsigned buffer_offset;  /**< offset to start of data in buffer, in
>> bytes */
>>     union pipe_buffer_binding u;
>> };
>> ---
>>   src/gallium/include/pipe/p_state.h | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/include/pipe/p_state.h
>> b/src/gallium/include/pipe/p_state.h
>> index ce9ca34..9576f18 100644
>> --- a/src/gallium/include/pipe/p_state.h
>> +++ b/src/gallium/include/pipe/p_state.h
>> @@ -534,6 +534,11 @@ struct pipe_transfer
>>   };
>>
>>
>> +union pipe_buffer_binding {
>> +   struct pipe_resource *buffer;  /**< the actual buffer */
>> +   const void *user_buffer;  /**< pointer to a user buffer */
>> +};
>> +
>>
>>   /**
>>    * A vertex buffer.  Typically, all the vertex data/attributes for
>> @@ -542,10 +547,10 @@ struct pipe_transfer
>>    */
>>   struct pipe_vertex_buffer
>>   {
>> -   unsigned stride;    /**< stride to same attrib in next vertex, in
>> bytes */
>> +   uint16_t stride;    /**< stride to same attrib in next vertex, in
>> bytes */
>> +   bool is_user_buffer;
>>      unsigned buffer_offset;  /**< offset to start of data in buffer, in
>> bytes */
>> -   struct pipe_resource *buffer;  /**< the actual buffer */
>> -   const void *user_buffer;  /**< pointer to a user buffer if buffer ==
>> NULL */
>> +   union pipe_buffer_binding u;
>>   };
>
>
> Why have the separate pipe_buffer_binding union at all?  Do you anticipate
> it being needed elsewhere in Gallium?

No, I don't.

>
> How about something like this:
>
> struct pipe_vertex_buffer
> {
>    uint16_t stride;
>    bool is_user_buffer;
>    ...
>    union {
>       struct pipe_resource *resource;
>       const void *user;
>    } buffer;
> };
>
> Similarly in patch 3:
>
> struct pipe_draw_info
> {
>    ...
>    union {
>       struct pipe_resource *resource;
>       const void *user;
>    } index;
> };
>
> If you really want the separate union type, can I suggest calling it
> "pipe_buffer_union" instead?  I don't know what "binding" has to do with it.

OK.

Marek