[Mesa-dev] [PATCH] st/mesa: fix a bug in and re-org setup_interleaved_attribs()

[Brian Paul]

From: Brian Paul <brianp at vmware.com>

We were mis-computing the size of the user-space vertex buffer in
some circumstances.  This led to a failed assertion at u_inlines.h:222
when using the VMware svga driver.

For example, if we had arrays such as:

array[0]: element_offset = 12, stride = 24
array[1]: element_offset = 0, stride = 24

We'd mistakenly compute 'bytes' to be 12 bytes too small.

I've reorganized the function too.  By time it's called, we know that
we've got interleaved arrays either all in one VBO or all in user memory
and the stride is equal for all arrays.

Move the code that lived inside the attr==0 test after the loop.

In the loop we compute the true vertex size.  That size factors into the
pipe->redefine_user_buffer() call later.  Using the vertex size instead
of array[0]'s element_offset fixes the failed assertion.
---
 src/mesa/state_tracker/st_draw.c |  107 +++++++++++++++++++++++++++-----------
 1 files changed, 76 insertions(+), 31 deletions(-)

diff --git a/src/mesa/state_tracker/st_draw.c b/src/mesa/state_tracker/st_draw.c
index 20ba993..64d2c83 100644
--- a/src/mesa/state_tracker/st_draw.c
+++ b/src/mesa/state_tracker/st_draw.c
@@ -353,9 +353,26 @@ setup_interleaved_attribs(struct gl_context *ctx,
    struct pipe_context *pipe = st->pipe;
    GLuint attr;
    const GLubyte *low_addr = NULL;
-
-   /* Find the lowest address of the arrays we're drawing */
+   GLboolean usingVBO;      /* all arrays in a VBO? */
+   struct gl_buffer_object *bufobj;
+   GLuint user_buffer_size = 0;
+   GLuint vertex_size = 0;  /* bytes per vertex, in bytes */
+   GLsizei stride;
+
+   /* Find the lowest address of the arrays we're drawing,
+    * Init bufobj and stride.
+    */
    if (vpv->num_inputs) {
+      const GLuint mesaAttr0 = vp->index_to_input[0];
+      const struct gl_client_array *array = arrays[mesaAttr0];
+
+      /* Since we're doing interleaved arrays, we know there'll be at most
+       * one buffer object and the stride will be the same for all arrays.
+       * Grab them now.
+       */
+      bufobj = array->BufferObj;
+      stride = array->StrideB;
+
       low_addr = arrays[vp->index_to_input[0]]->Ptr;
 
       for (attr = 1; attr < vpv->num_inputs; attr++) {
@@ -363,44 +380,24 @@ setup_interleaved_attribs(struct gl_context *ctx,
          low_addr = MIN2(low_addr, start);
       }
    }
+   else {
+      /* not sure we'll ever have zero inputs, but play it safe */
+      bufobj = NULL;
+      stride = 0;
+      low_addr = 0;
+   }
+
+   /* are the arrays in user space? */
+   usingVBO = bufobj && _mesa_is_bufferobj(bufobj);
 
    for (attr = 0; attr < vpv->num_inputs; attr++) {
       const GLuint mesaAttr = vp->index_to_input[attr];
       const struct gl_client_array *array = arrays[mesaAttr];
-      struct gl_buffer_object *bufobj = array->BufferObj;
-      struct st_buffer_object *stobj = st_buffer_object(bufobj);
       unsigned src_offset = (unsigned) (array->Ptr - low_addr);
       GLuint element_size = array->_ElementSize;
-      GLsizei stride = array->StrideB;
 
       assert(element_size == array->Size * _mesa_sizeof_type(array->Type));
 
-      if (attr == 0) {
-         if (bufobj && _mesa_is_bufferobj(bufobj)) {
-            vbuffer->buffer = NULL;
-            pipe_resource_reference(&vbuffer->buffer, stobj->buffer);
-            vbuffer->buffer_offset = pointer_to_offset(low_addr);
-         }
-         else {
-            uint divisor = array->InstanceDivisor;
-            uint last_index = divisor ? num_instances / divisor : max_index;
-            uint bytes = src_offset + stride * last_index + element_size;
-
-            vbuffer->buffer = pipe_user_buffer_create(pipe->screen,
-                                                      (void*) low_addr,
-                                                      bytes,
-                                                      PIPE_BIND_VERTEX_BUFFER);
-            vbuffer->buffer_offset = 0;
-
-            /* Track user vertex buffers. */
-            pipe_resource_reference(&st->user_attrib[0].buffer, vbuffer->buffer);
-            st->user_attrib[0].element_size = element_size;
-            st->user_attrib[0].stride = stride;
-            st->num_user_attribs = 1;
-         }
-         vbuffer->stride = stride; /* in bytes */
-      }
-
       velements[attr].src_offset = src_offset;
       velements[attr].instance_divisor = array->InstanceDivisor;
       velements[attr].vertex_buffer_index = 0;
@@ -409,6 +406,54 @@ setup_interleaved_attribs(struct gl_context *ctx,
                                                          array->Format,
                                                          array->Normalized);
       assert(velements[attr].src_format);
+
+      if (!usingVBO) {
+         /* how many bytes referenced by this attribute array? */
+         uint divisor = array->InstanceDivisor;
+         uint last_index = divisor ? num_instances / divisor : max_index;
+         uint bytes = src_offset + stride * last_index + element_size;
+
+         user_buffer_size = MAX2(user_buffer_size, bytes);
+
+         /* update vertex size */
+         vertex_size = MAX2(vertex_size, src_offset + element_size);
+      }
+   }
+
+   /*
+    * Return the vbuffer info and setup user-space attrib info, if needed.
+    */
+   if (vpv->num_inputs == 0) {
+      /* just defensive coding here */
+      vbuffer->buffer = NULL;
+      vbuffer->buffer_offset = 0;
+      vbuffer->stride = 0;
+      st->num_user_attribs = 0;
+   }
+   else if (usingVBO) {
+      /* all interleaved arrays in a VBO */
+      struct st_buffer_object *stobj = st_buffer_object(bufobj);
+
+      vbuffer->buffer = NULL;
+      pipe_resource_reference(&vbuffer->buffer, stobj->buffer);
+      vbuffer->buffer_offset = pointer_to_offset(low_addr);
+      vbuffer->stride = stride;
+      st->num_user_attribs = 0;
+   }
+   else {
+      /* all interleaved arrays in user memory */
+      vbuffer->buffer = pipe_user_buffer_create(pipe->screen,
+                                                (void*) low_addr,
+                                                user_buffer_size,
+                                                PIPE_BIND_VERTEX_BUFFER);
+      vbuffer->buffer_offset = 0;
+      vbuffer->stride = stride;
+
+      /* Track user vertex buffers. */
+      pipe_resource_reference(&st->user_attrib[0].buffer, vbuffer->buffer);
+      st->user_attrib[0].element_size = vertex_size;
+      st->user_attrib[0].stride = stride;
+      st->num_user_attribs = 1;
    }
 }
 
-- 
1.7.3.4