[Mesa-dev] [PATCH] vbo: Correctly handle attribute offsets in dlist draw.

Mathias Fröhlich Mathias.Froehlich at gmx.net
Wed Oct 12 10:10:46 UTC 2016


On Wednesday, 24 August 2016 08:32:10 CEST Mathias.Froehlich at gmx.net wrote:
> From: Mathias Fröhlich <mathias.froehlich at web.de>
> 
> Hi all,
> 
> kind of a ping with a rephrased commit message
> and the style change.
> Please review.

Ping.

best
Mathias

> 
> Thanks!
> 
> Mathias
> 
> 
> When executing a display list draw, for the offset
> list to be correct, the offset computation needs to
> accumulate all attribute size values in order.
> Specifically, if we are shuffling around the position
> and generic0 attributes, we may violate the order or
> if we do not walk the generic vbo attributes we may
> skip some of the attributes.
> Even if this is an unlikely usecase we can fix this
> by precomputing the offsets on the full attribute list
> and store the full offset list in the display list node.
> 
> v2: Formatting fix
> 
> Signed-off-by: Mathias Fröhlich <Mathias.Froehlich at web.de>
> ---
>  src/mesa/vbo/vbo_save.h      |  1 +
>  src/mesa/vbo/vbo_save_api.c  |  5 +++++
>  src/mesa/vbo/vbo_save_draw.c | 35 +++++++++++++++++------------------
>  3 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/src/mesa/vbo/vbo_save.h b/src/mesa/vbo/vbo_save.h
> index 2843b3c..a61973f 100644
> --- a/src/mesa/vbo/vbo_save.h
> +++ b/src/mesa/vbo/vbo_save.h
> @@ -64,6 +64,7 @@ struct vbo_save_vertex_list {
>     GLbitfield64 enabled; /**< mask of enabled vbo arrays. */
>     GLubyte attrsz[VBO_ATTRIB_MAX];
>     GLenum attrtype[VBO_ATTRIB_MAX];
> +   GLushort offsets[VBO_ATTRIB_MAX];
>     GLuint vertex_size;  /**< size in GLfloats */
>  
>     /* Copy of the final vertex from node->vertex_store->bufferobj.
> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
> index f648ccc..36af426 100644
> --- a/src/mesa/vbo/vbo_save_api.c
> +++ b/src/mesa/vbo/vbo_save_api.c
> @@ -415,6 +415,7 @@ _save_compile_vertex_list(struct gl_context *ctx)
>  {
>     struct vbo_save_context *save = &vbo_context(ctx)->save;
>     struct vbo_save_vertex_list *node;
> +   GLushort offset = 0;
>  
>     /* Allocate space for this structure in the display list currently
>      * being compiled.
> @@ -436,6 +437,10 @@ _save_compile_vertex_list(struct gl_context *ctx)
>     node->vertex_size = save->vertex_size;
>     node->buffer_offset =
>        (save->buffer - save->vertex_store->buffer) * sizeof(GLfloat);
> +   for (unsigned i = 0; i < VBO_ATTRIB_MAX; ++i) {
> +      node->offsets[i] = offset;
> +      offset += node->attrsz[i] * sizeof(GLfloat);
> +   }
>     node->count = save->vert_count;
>     node->wrap_count = save->copied.nr;
>     node->dangling_attr_ref = save->dangling_attr_ref;
> diff --git a/src/mesa/vbo/vbo_save_draw.c b/src/mesa/vbo/vbo_save_draw.c
> index 507ab82..e69c108 100644
> --- a/src/mesa/vbo/vbo_save_draw.c
> +++ b/src/mesa/vbo/vbo_save_draw.c
> @@ -26,6 +26,7 @@
>   *    Keith Whitwell <keithw at vmware.com>
>   */
>  
> +#include <stdbool.h>
>  #include "main/glheader.h"
>  #include "main/bufferobj.h"
>  #include "main/context.h"
> @@ -136,15 +137,10 @@ static void vbo_bind_vertex_list(struct gl_context 
*ctx,
>     struct vbo_context *vbo = vbo_context(ctx);
>     struct vbo_save_context *save = &vbo->save;
>     struct gl_client_array *arrays = save->arrays;
> -   GLuint buffer_offset = node->buffer_offset;
>     const GLuint *map;
>     GLuint attr;
> -   GLubyte node_attrsz[VBO_ATTRIB_MAX];  /* copy of node->attrsz[] */
> -   GLenum node_attrtype[VBO_ATTRIB_MAX];  /* copy of node->attrtype[] */
>     GLbitfield64 varying_inputs = 0x0;
> -
> -   memcpy(node_attrsz, node->attrsz, sizeof(node->attrsz));
> -   memcpy(node_attrtype, node->attrtype, sizeof(node->attrtype));
> +   bool generic_from_pos = false;
>  
>     /* Install the default (ie Current) attributes first, then overlay
>      * all active ones.
> @@ -176,10 +172,7 @@ static void vbo_bind_vertex_list(struct gl_context 
*ctx,
>         */
>        if ((ctx->VertexProgram._Current->Base.InputsRead & VERT_BIT_POS) == 
0 &&
>            (ctx->VertexProgram._Current->Base.InputsRead & 
VERT_BIT_GENERIC0)) {
> -         save->inputs[VERT_ATTRIB_GENERIC0] = save->inputs[0];
> -         node_attrsz[VERT_ATTRIB_GENERIC0] = node_attrsz[0];
> -         node_attrtype[VERT_ATTRIB_GENERIC0] = node_attrtype[0];
> -         node_attrsz[0] = 0;
> +         generic_from_pos = true;
>        }
>        break;
>     default:
> @@ -188,30 +181,36 @@ static void vbo_bind_vertex_list(struct gl_context 
*ctx,
>  
>     for (attr = 0; attr < VERT_ATTRIB_MAX; attr++) {
>        const GLuint src = map[attr];
> +      const GLubyte size = node->attrsz[src];
>  
> -      if (node_attrsz[src]) {
> +      if (size) {
>           /* override the default array set above */
>           save->inputs[attr] = &arrays[attr];
>  
> -	 arrays[attr].Ptr = (const GLubyte *) NULL + buffer_offset;
> -	 arrays[attr].Size = node_attrsz[src];
> +         const uintptr_t buffer_offset = node->buffer_offset;
> +         arrays[attr].Ptr = ADD_POINTERS(buffer_offset, node-
>offsets[src]);
> +         arrays[attr].Size = size;
>  	 arrays[attr].StrideB = node->vertex_size * sizeof(GLfloat);
> -         arrays[attr].Type = node_attrtype[src];
> -         arrays[attr].Integer =
> -               vbo_attrtype_to_integer_flag(node_attrtype[src]);
> +         const GLenum type = node->attrtype[src];
> +         arrays[attr].Type = type;
> +         arrays[attr].Integer = vbo_attrtype_to_integer_flag(type);
>           arrays[attr].Format = GL_RGBA;
> -         arrays[attr]._ElementSize = arrays[attr].Size * sizeof(GLfloat);
> +         arrays[attr]._ElementSize = size * sizeof(GLfloat);
>           _mesa_reference_buffer_object(ctx,
>                                         &arrays[attr].BufferObj,
>                                         node->vertex_store->bufferobj);
>  	 
>  	 assert(arrays[attr].BufferObj->Name);
>  
> -	 buffer_offset += node_attrsz[src] * sizeof(GLfloat);
>           varying_inputs |= VERT_BIT(attr);
>        }
>     }
>  
> +   if (generic_from_pos) {
> +      varying_inputs |= (varying_inputs & VERT_BIT_POS) << 
VERT_ATTRIB_GENERIC0;
> +      save->inputs[VERT_ATTRIB_GENERIC0] = save->inputs[VERT_ATTRIB_POS];
> +   }
> +
>     _mesa_set_varying_vp_inputs( ctx, varying_inputs );
>     ctx->NewDriverState |= ctx->DriverFlags.NewArray;
>  }
> 




More information about the mesa-dev mailing list