[Mesa-dev] [PATCH 3/6] mesa/st: Factor out array and buffer setup from st_atom_array.c.

Chris Wilson chris at chris-wilson.co.uk
Fri Nov 23 15:12:38 UTC 2018


Quoting Mathias.Froehlich at gmx.net (2018-11-23 08:07:29)
> From: Mathias Fröhlich <mathias.froehlich at web.de>
> 
> Factor out vertex array setup routines from the array state atom.
> The factored functions will be used in feedback rendering in the
> next change.
> 
> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
> ---
>  src/mesa/state_tracker/st_atom.h       | 17 ++++++
>  src/mesa/state_tracker/st_atom_array.c | 79 +++++++++++++++++++-------
>  2 files changed, 74 insertions(+), 22 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_atom.h b/src/mesa/state_tracker/st_atom.h
> index 9f3ca38c19..901e9b6d43 100644
> --- a/src/mesa/state_tracker/st_atom.h
> +++ b/src/mesa/state_tracker/st_atom.h
> @@ -37,6 +37,10 @@
>  #include "main/glheader.h"
>  
>  struct st_context;
> +struct st_vertex_program;
> +struct st_vp_variant;
> +struct pipe_vertex_buffer;
> +struct pipe_vertex_element;
>  
>  /**
>   * Enumeration of state tracker pipelines.
> @@ -57,6 +61,19 @@ GLuint st_compare_func_to_pipe(GLenum func);
>  enum pipe_format
>  st_pipe_vertex_format(const struct gl_vertex_format *glformat);
>  
> +void
> +st_setup_arrays(struct st_context *st,
> +                const struct st_vertex_program *vp,
> +                const struct st_vp_variant *vp_variant,
> +                struct pipe_vertex_element *velements,
> +                struct pipe_vertex_buffer *vbuffer, unsigned *num_vbuffers);
> +
> +void
> +st_setup_current(struct st_context *st,
> +                 const struct st_vertex_program *vp,
> +                 const struct st_vp_variant *vp_variant,
> +                 struct pipe_vertex_element *velements,
> +                 struct pipe_vertex_buffer *vbuffer, unsigned *num_vbuffers);
>  
>  /* Define ST_NEW_xxx_INDEX */
>  enum {
> diff --git a/src/mesa/state_tracker/st_atom_array.c b/src/mesa/state_tracker/st_atom_array.c
> index cd00529ddf..ac9bd727df 100644
> --- a/src/mesa/state_tracker/st_atom_array.c
> +++ b/src/mesa/state_tracker/st_atom_array.c
> @@ -384,25 +384,17 @@ set_vertex_attribs(struct st_context *st,
>  }
>  
>  void
> -st_update_array(struct st_context *st)
> +st_setup_arrays(struct st_context *st,
> +                const struct st_vertex_program *vp,
> +                const struct st_vp_variant *vp_variant,
> +                struct pipe_vertex_element *velements,
> +                struct pipe_vertex_buffer *vbuffer, unsigned *num_vbuffers)
>  {
>     struct gl_context *ctx = st->ctx;
> -   /* vertex program validation must be done before this */
> -   const struct st_vertex_program *vp = st->vp;
> -   /* _NEW_PROGRAM, ST_NEW_VS_STATE */
> -   const GLbitfield inputs_read = st->vp_variant->vert_attrib_mask;
>     const struct gl_vertex_array_object *vao = ctx->Array._DrawVAO;
> +   const GLbitfield inputs_read = vp_variant->vert_attrib_mask;
>     const ubyte *input_to_index = vp->input_to_index;
>  
> -   struct pipe_vertex_buffer vbuffer[PIPE_MAX_ATTRIBS];
> -   struct pipe_vertex_element velements[PIPE_MAX_ATTRIBS];
> -   unsigned num_vbuffers = 0;
> -
> -   st->vertex_array_out_of_memory = FALSE;
> -   st->draw_needs_minmax_index = false;
> -
> -   /* _NEW_PROGRAM */
> -   /* ST_NEW_VERTEX_ARRAYS alias ctx->DriverFlags.NewArray */
>     /* Process attribute array data. */
>     GLbitfield mask = inputs_read & _mesa_draw_array_bits(ctx);
>     while (mask) {
> @@ -410,7 +402,7 @@ st_update_array(struct st_context *st)
>        const gl_vert_attrib i = ffs(mask) - 1;
>        const struct gl_vertex_buffer_binding *const binding
>           = _mesa_draw_buffer_binding(vao, i);
> -      const unsigned bufidx = num_vbuffers++;
> +      const unsigned bufidx = (*num_vbuffers)++;
>  
>        if (_mesa_is_bufferobj(binding->BufferObj)) {
>           struct st_buffer_object *stobj = st_buffer_object(binding->BufferObj);
> @@ -452,16 +444,28 @@ st_update_array(struct st_context *st)
>                                 input_to_index[attr]);
>        }
>     }
> +}
> +
> +void
> +st_setup_current(struct st_context *st,
> +                 const struct st_vertex_program *vp,
> +                 const struct st_vp_variant *vp_variant,
> +                 struct pipe_vertex_element *velements,
> +                 struct pipe_vertex_buffer *vbuffer, unsigned *num_vbuffers)
> +{
> +   struct gl_context *ctx = st->ctx;
> +   const GLbitfield inputs_read = vp_variant->vert_attrib_mask;
>  
> -   const unsigned first_current_vbuffer = num_vbuffers;
> -   /* _NEW_PROGRAM | _NEW_CURRENT_ATTRIB */
>     /* Process values that should have better been uniforms in the application */
>     GLbitfield curmask = inputs_read & _mesa_draw_current_bits(ctx);
>     if (curmask) {
> +      /* vertex program validation must be done before this */
> +      const struct st_vertex_program *vp = st->vp;
> +      const ubyte *input_to_index = vp->input_to_index;
>        /* For each attribute, upload the maximum possible size. */
>        GLubyte data[VERT_ATTRIB_MAX * sizeof(GLdouble) * 4];
>        GLubyte *cursor = data;
> -      const unsigned bufidx = num_vbuffers++;
> +      const unsigned bufidx = (*num_vbuffers)++;
>        unsigned max_alignment = 1;
>  
>        while (curmask) {
> @@ -504,12 +508,43 @@ st_update_array(struct st_context *st)
>           u_upload_unmap(st->pipe->stream_uploader);
>        }
>     }
> +}
>  
> -   const unsigned num_inputs = st->vp_variant->num_inputs;
> -   set_vertex_attribs(st, vbuffer, num_vbuffers, velements, num_inputs);
> +void
> +st_update_array(struct st_context *st)
> +{
> +   /* vertex program validation must be done before this */
> +   /* _NEW_PROGRAM, ST_NEW_VS_STATE */
> +   const struct st_vertex_program *vp = st->vp;
> +   const struct st_vp_variant *vp_variant = st->vp_variant;
>  
> -   /* Unreference uploaded zero-stride vertex buffers. */
> -   for (unsigned i = first_current_vbuffer; i < num_vbuffers; ++i) {
> +   struct pipe_vertex_buffer vbuffer[PIPE_MAX_ATTRIBS];
> +   unsigned num_vbuffers = 0, first_upload_vbuffer;
> +   struct pipe_vertex_element velements[PIPE_MAX_ATTRIBS];
> +   unsigned num_velements;

Something to note here is that valgrind reports
(piglit/bin/drawoverhead):

==492== Use of uninitialised value of size 8
==492==    at 0x6EB8FA9: cso_hash_find_node (cso_hash.c:198)
==492==    by 0x6EB9026: cso_hash_insert (cso_hash.c:213)
==492==    by 0x6EB6730: cso_set_vertex_elements (cso_context.c:1092)
==492==    by 0x714C76F: set_vertex_attribs (st_atom_array.c:384)
==492==    by 0x714C76F: st_update_array (st_atom_array.c:512)
==492==    by 0x71073F3: st_validate_state (st_atom.c:261)
==492==    by 0x70615D1: prepare_draw (st_draw.c:123)
==492==    by 0x70615D1: st_draw_vbo (st_draw.c:149)
==492==    by 0x70F5BF4: _mesa_validated_drawrangeelements (draw.c:850)
==492==    by 0x70F5BF4: _mesa_validated_drawrangeelements (draw.c:782)
==492==    by 0x70F5F32: _mesa_DrawElements (draw.c:1004)
==492==    by 0x48F6C74: stub_glDrawElements (piglit-dispatch-gen.c:12618)
==492==    by 0x10B3F4: draw (drawoverhead.c:275)
==492==    by 0x10D070: perf_measure_rate (common.c:56)
==492==    by 0x10C69E: perf_run (drawoverhead.c:645)
==492==  Uninitialised value was created by a stack allocation
==492==    at 0x714C25D: st_update_array (st_atom_array.c:389)

from velements being used sparsely.

Does your patch prevent this?
-Chris


More information about the mesa-dev mailing list