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

[Brian Paul]

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?

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.

-Brian