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

[Mathias.Froehlich at gmx.net]

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.

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;
 }
-- 
2.7.4