[Mesa-dev] [PATCH] vbo: optimize some display list drawing (v2)

[Ian Romanick]

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

On 01/15/2018 11:37 AM, Brian Paul wrote:
> The vbo_save_vertex_list structure records one or more glBegin/End
> primitives which all have the same vertex format.
> 
> To draw these primitives, we setup the vertex array state, then
> issue the drawing command.  Before, the 'start' vertex was typically
> zero and we used the vertex array pointer to indicate where the
> vertex data starts.
> 
> This patch checks if the vertex buffer offset is an exact multiple of
> the vertex size.  If so, that means we can use zero-based vertex array
> pointers and use the draw's start value to indicate where the vertex
> data starts.
> 
> This means a series of display list drawing commands may have
> identical vertex array state.  This will get filtered out by the
> Gallium CSO module so we can issue a tight series of drawing commands
> without state changes to the device.
> 
> Note that this also works for a series of glCallList commands (not
> just one list that contains multiple glBegin/End pairs).
> 
> No Piglit or conform changes.
> 
> v2: minor fixes suggested by Ian.
> ---
>  src/mesa/vbo/vbo_save.h      | 14 ++++++++++++++
>  src/mesa/vbo/vbo_save_api.c  | 14 ++++++++++++++
>  src/mesa/vbo/vbo_save_draw.c | 12 ++++++++++++
>  3 files changed, 40 insertions(+)
> 
> diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
> index 9d13e0a..274d667 100644
> --- a/src/mesa/vbo/vbo_save.h
> +++ b/src/mesa/vbo/vbo_save.h
> @@ -86,6 +86,20 @@ struct vbo_save_vertex_list {
>     struct vbo_save_primitive_store *prim_store;
>  };
>  
> +
> +/**
> + * Is the vertex lists's buffer offset an exact multiple of the
> + * vertex size (in bytes)?  This is used to check for a vertex array /
> + * drawing optimization.
> + */
> +static inline bool
> +aligned_vertex_buffer_offset(const struct vbo_save_vertex_list *node)
> +{
> +   unsigned vertex_size = node->vertex_size * sizeof(GLfloat); /* in bytes */
> +   return vertex_size != 0 && node->buffer_offset % vertex_size == 0;
> +}
> +
> +
>  /* These buffers should be a reasonable size to support upload to
>   * hardware.  Current vbo implementation will re-upload on any
>   * changes, so don't make too big or apps which dynamically create
> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
> index 42d883f..1c57544 100644
> --- a/src/mesa/vbo/vbo_save_api.c
> +++ b/src/mesa/vbo/vbo_save_api.c
> @@ -546,6 +546,20 @@ compile_vertex_list(struct gl_context *ctx)
>        save->prim_store = alloc_prim_store();
>     }
>  
> +   /*
> +    * If the vertex buffer offset is a multiple of the vertex size,
> +    * we can use the _mesa_prim::start value to indicate where the
> +    * vertices starts, instead of the buffer offset.  Also see the
> +    * bind_vertex_list() function.
> +    */
> +   if (aligned_vertex_buffer_offset(node)) {
> +      const unsigned start_offset =
> +         node->buffer_offset / (node->vertex_size * sizeof(GLfloat));
> +      for (unsigned i = 0; i < save->prim_count; i++) {
> +         save->prims[i].start += start_offset;
> +      }
> +   }
> +
>     /* Reset our structures for the next run of vertices:
>      */
>     reset_counters(ctx);
> diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
> index 1694a04..b63a9a8 100644
> --- a/src/mesa/vbo/vbo_save_draw.c
> +++ b/src/mesa/vbo/vbo_save_draw.c
> @@ -146,6 +146,18 @@ bind_vertex_list(struct gl_context *ctx,
>     memcpy(node_attrsz, node->attrsz, sizeof(node->attrsz));
>     memcpy(node_attrtype, node->attrtype, sizeof(node->attrtype));
>  
> +   if (aligned_vertex_buffer_offset(node)) {
> +      /* The vertex size is an exact multiple of the buffer offset.
> +       * This means that we can use zero-based vertex attribute pointers
> +       * and specify the start of the primitive with the _mesa_prim::start
> +       * field.  This results in issuing several draw calls with identical
> +       * vertex attribute information.  This can result in fewer state
> +       * changes in drivers.  In particular, the Gallium CSO module will
> +       * filter out redundant vertex buffer changes.
> +       */
> +      buffer_offset = 0;
> +   }
> +
>     /* Install the default (ie Current) attributes first, then overlay
>      * all active ones.
>      */
>