[Mesa-dev] [PATCH 1/8] mesa: Implement helper functions to map and unmap a VAO.

[Brian Paul]

The series looks great.  Just a few minor suggestions below.

Reviewed-by: Brian Paul <brianp at vmware.com>

BTW, I happened to look at _mesa_primitive_restart_index().  It seems we 
could/should add an assertion in there:

assert(index_size == 1 ||
        index_size == 2 ||
        index_size == 4);

right?

-Brian


On 03/05/2019 01:13 AM, Mathias.Froehlich at gmx.net wrote:
> From: Mathias Fröhlich <mathias.froehlich at web.de>
> 
> Provide a set of functions that maps or unmaps all VBOs held
> in a VAO. The functions will be used in the following patches.
> 
> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
> ---
>   src/mesa/main/arrayobj.c | 84 ++++++++++++++++++++++++++++++++++++++++
>   src/mesa/main/arrayobj.h | 18 +++++++++
>   2 files changed, 102 insertions(+)
> 
> diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
> index 68d30aa9b1f..1f6c6904739 100644
> --- a/src/mesa/main/arrayobj.c
> +++ b/src/mesa/main/arrayobj.c
> @@ -913,6 +913,90 @@ _mesa_all_buffers_are_unmapped(const struct gl_vertex_array_object *vao)
>      return true;
>   }
>   
> +
> +/**
> + * Map buffer objects used in attribute arrays.
> + */
> +void
> +_mesa_vao_map_arrays(struct gl_context *ctx, struct gl_vertex_array_object *vao,
> +                     GLbitfield access)
> +{
> +   GLbitfield mask = vao->Enabled & vao->VertexAttribBufferMask;
> +   while (mask) {
> +      /* Do not use u_bit_scan as we can walk multiple attrib arrays at once */
> +      const gl_vert_attrib attr = ffs(mask) - 1;
> +      const GLubyte bindex = vao->VertexAttrib[attr].BufferBindingIndex;
> +      struct gl_vertex_buffer_binding *binding = &vao->BufferBinding[bindex];
> +      mask &= ~binding->_BoundArrays;
> +
> +      struct gl_buffer_object *bo = binding->BufferObj;
> +      assert(_mesa_is_bufferobj(bo));
> +      if (_mesa_bufferobj_mapped(bo, MAP_INTERNAL))
> +         continue;
> +
> +      ctx->Driver.MapBufferRange(ctx, 0, bo->Size, access, bo, MAP_INTERNAL);
> +   }
> +}
> +
> +
> +/**
> + * Map buffer objects used in the vao,
> + * attribute arrays and index buffer.

Combine onto one line.

> + */
> +void
> +_mesa_vao_map(struct gl_context *ctx, struct gl_vertex_array_object *vao,
> +              GLbitfield access)
> +{

A comment might be hlpeful here, like
     /* map the index buffer, if there is one, and not already mapped */

> +   struct gl_buffer_object *bo = vao->IndexBufferObj;
> +
> +   if (_mesa_is_bufferobj(bo) && !_mesa_bufferobj_mapped(bo, MAP_INTERNAL))
> +      ctx->Driver.MapBufferRange(ctx, 0, bo->Size, access, bo, MAP_INTERNAL);
> +
> +   _mesa_vao_map_arrays(ctx, vao, access);
> +}
> +
> +
> +/**
> + * Unmap buffer objects used in attribute arrays.
> + */
> +void
> +_mesa_vao_unmap_arrays(struct gl_context *ctx,
> +                       struct gl_vertex_array_object *vao)
> +{
> +   GLbitfield mask = vao->Enabled & vao->VertexAttribBufferMask;
> +   while (mask) {
> +      /* Do not use u_bit_scan as we can walk multiple attrib arrays at once */
> +      const gl_vert_attrib attr = ffs(mask) - 1;
> +      const GLubyte bindex = vao->VertexAttrib[attr].BufferBindingIndex;
> +      struct gl_vertex_buffer_binding *binding = &vao->BufferBinding[bindex];
> +      mask &= ~binding->_BoundArrays;
> +
> +      struct gl_buffer_object *bo = binding->BufferObj;
> +      assert(_mesa_is_bufferobj(bo));
> +      if (!_mesa_bufferobj_mapped(bo, MAP_INTERNAL))
> +         continue;
> +
> +      ctx->Driver.UnmapBuffer(ctx, bo, MAP_INTERNAL);
> +   }
> +}
> +
> +
> +/**
> + * Unmap buffer objects used in the vao,
> + * attribute arrays and index buffer.

combine onto one line.


> + */
> +void
> +_mesa_vao_unmap(struct gl_context *ctx, struct gl_vertex_array_object *vao)
> +{
> +   struct gl_buffer_object *bo = vao->IndexBufferObj;
> +
> +   if (_mesa_is_bufferobj(bo) && _mesa_bufferobj_mapped(bo, MAP_INTERNAL))
> +      ctx->Driver.UnmapBuffer(ctx, bo, MAP_INTERNAL);
> +
> +   _mesa_vao_unmap_arrays(ctx, vao);
> +}
> +
> +
>   /**********************************************************************/
>   /* API Functions                                                      */
>   /**********************************************************************/
> diff --git a/src/mesa/main/arrayobj.h b/src/mesa/main/arrayobj.h
> index ee87b4b6ba5..7516bae9e39 100644
> --- a/src/mesa/main/arrayobj.h
> +++ b/src/mesa/main/arrayobj.h
> @@ -100,6 +100,24 @@ extern bool
>   _mesa_all_buffers_are_unmapped(const struct gl_vertex_array_object *vao);
>   
>   
> +extern void
> +_mesa_vao_map_arrays(struct gl_context *ctx, struct gl_vertex_array_object *vao,
> +                     GLbitfield access);
> +
> +extern void
> +_mesa_vao_map(struct gl_context *ctx, struct gl_vertex_array_object *vao,
> +              GLbitfield access);
> +
> +
> +extern void
> +_mesa_vao_unmap_arrays(struct gl_context *ctx,
> +                       struct gl_vertex_array_object *vao);
> +
> +extern void
> +_mesa_vao_unmap(struct gl_context *ctx,
> +                struct gl_vertex_array_object *vao);
> +
> +
>   /**
>    * Array to apply the position/generic0 aliasing map to
>    * an attribute value used in vertex processing inputs to an attribute
>