[Mesa-dev] [PATCH] i965: Remove interleaved user array upload optimization

[Ian Romanick]

From: Ian Romanick <ian.d.romanick at intel.com>

The checks to determine when the data can be uploaded in an interleaved
fashion can be tricked by certain data layouts.  For example,

    float data[...];

    glVertexAttribPointer(0, 4, GL_FLOAT, GL_FALSE, 16, &data[0]);
    glVertexAttribPointer(1, 4, GL_FLOAT, GL_FALSE, 16, &data[4]);
    glDrawArrays(GL_POINTS, 0, 1);

will hit the interleaved path with an incorrect size (16 bytes instead
of 32 bytes).  As a result, the data for attribute 1 never gets
uploaded.  The single element draw case is the only sensible case I can
think of for non-interleaved-that-looks-like-interleaved data, but there
may be others as well.

I don't see an easy way to fix the checks, so I have chosen to just rip
out the optimization.  This may cause performance issues on applications
that use large arrays of data that are not in VBOs.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Cc: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_draw_upload.c | 37 +----------------------------
 1 file changed, 1 insertion(+), 36 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c b/src/mesa/drivers/dri/i965/brw_draw_upload.c
index 2ded14b..9d46fee 100644
--- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
@@ -403,8 +403,6 @@ static void brw_prepare_vertices(struct brw_context *brw)
    struct intel_context *intel = intel_context(ctx);
    /* CACHE_NEW_VS_PROG */
    GLbitfield64 vs_inputs = brw->vs.prog_data->inputs_read;
-   const unsigned char *ptr = NULL;
-   GLuint interleaved = 0;
    unsigned int min_index = brw->vb.min_index + brw->basevertex;
    unsigned int max_index = brw->vb.max_index + brw->basevertex;
    int delta, i, j;
@@ -498,19 +496,8 @@ static void brw_prepare_vertices(struct brw_context *brw)
 	  */
 	 assert(input->offset < brw->vb.buffers[input->buffer].bo->size);
       } else {
-	 /* Queue the buffer object up to be uploaded in the next pass,
-	  * when we've decided if we're doing interleaved or not.
+	 /* Queue the buffer object up to be uploaded in the next pass.
 	  */
-	 if (nr_uploads == 0) {
-	    interleaved = glarray->StrideB;
-	    ptr = glarray->Ptr;
-	 }
-	 else if (interleaved != glarray->StrideB ||
-		  (uintptr_t)(glarray->Ptr - ptr) > interleaved)
-	 {
-	    interleaved = 0;
-	 }
-
 	 upload[nr_uploads++] = input;
       }
    }
@@ -527,28 +514,6 @@ static void brw_prepare_vertices(struct brw_context *brw)
    }
 
    /* Handle any arrays to be uploaded. */
-   if (nr_uploads > 1) {
-      if (interleaved) {
-	 struct brw_vertex_buffer *buffer = &brw->vb.buffers[j];
-	 /* All uploads are interleaved, so upload the arrays together as
-	  * interleaved.  First, upload the contents and set up upload[0].
-	  */
-	 copy_array_to_vbo_array(brw, upload[0], min_index, max_index,
-				 buffer, interleaved);
-	 buffer->offset -= delta * interleaved;
-
-	 for (i = 0; i < nr_uploads; i++) {
-	    /* Then, just point upload[i] at upload[0]'s buffer. */
-	    upload[i]->offset =
-	       ((const unsigned char *)upload[i]->glarray->Ptr - ptr);
-	    upload[i]->buffer = j;
-	 }
-	 j++;
-
-	 nr_uploads = 0;
-      }
-   }
-   /* Upload non-interleaved arrays */
    for (i = 0; i < nr_uploads; i++) {
       struct brw_vertex_buffer *buffer = &brw->vb.buffers[j];
       if (upload[i]->glarray->InstanceDivisor == 0) {
-- 
1.8.1.4