vbo: fix GL_LINE_LOOP stray line bug

When long GL_LINE_LOOP primitives don't fit in one vertex buffer they
have to be split across buffers.  The code to do this was basically correct
but drivers had to pay special attention to the _mesa_prim::begin,end flags
in order to draw the sections of the line loop properly.  Apparently, the
only drivers to do this were those using the old 'tnl' module for software
vertex processing.

Now we convert the split pieces of GL_LINE_LOOP prims into GL_LINE_STRIP
primitives so that drivers don't have to worry about the special begin/end
flags.  The only time a driver will get a GL_LINE_LOOP prim is when the
whole thing fits in one vertex buffer.

Mostly fixes bug 81174, but not completely.  There's another bug somewhere
in the src/gallium/auxiliary/draw/ code.  If the piglit lineloop test is
run with -count 4096, rendering is correct, but with -count 4097 there are
stray lines.  4096 is a magic number in the draw code (search for "4096").

Also note that this does not fix long line loops in display lists.  The
next patch fixes that.

v2: fix incorrect -1 in vbo_compute_max_verts(), per Charmaine.  Remove
incorrect assertion which was added in vbo_copy_vertices().

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81174
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49779
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=28130

Reviewed-by: Marek Olšák <[email protected]>
Reviewed-by: Jose Fonseca <[email protected]>
Reviewed-by: Sinclair Yeh <[email protected]>

3 files changed, 56 insertions, 3 deletions

diff --git a/src/mesa/vbo/vbo_context.h b/src/mesa/vbo/vbo_context.h
index 1e85335c107..e6b9d890d5f 100644
--- a/src/mesa/vbo/vbo_context.h
+++ b/src/mesa/vbo/vbo_context.h
@@ -205,8 +205,14 @@ vbo_get_default_vals_as_union(GLenum format)
 static inline unsigned
 vbo_compute_max_verts(const struct vbo_exec_context *exec)
 {
-   return (VBO_VERT_BUFFER_SIZE - exec->vtx.buffer_used) /
-          (exec->vtx.vertex_size * sizeof(GLfloat));
+   unsigned n = (VBO_VERT_BUFFER_SIZE - exec->vtx.buffer_used) /
+      (exec->vtx.vertex_size * sizeof(GLfloat));
+   assert(n > 0);
+   /* Subtract one so we're always sure to have room for an extra
+    * vertex for GL_LINE_LOOP -> GL_LINE_STRIP conversion.
+    */
+   n--;
+   return n;
 }
 
 
diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
index f26bf405d56..a23d5aa08aa 100644
--- a/src/mesa/vbo/vbo_exec_api.c
+++ b/src/mesa/vbo/vbo_exec_api.c
@@ -61,7 +61,8 @@ static void reset_attrfv( struct vbo_exec_context *exec );
 
 /**
  * Close off the last primitive, execute the buffer, restart the
- * primitive.  
+ * primitive.  This is called when we fill a vertex buffer before
+ * hitting glEnd.
  */
 static void vbo_exec_wrap_buffers( struct vbo_exec_context *exec )
 {
@@ -81,6 +82,22 @@ static void vbo_exec_wrap_buffers( struct vbo_exec_context *exec )
 
       last_count = last_prim->count;
 
+      /* Special handling for wrapping GL_LINE_LOOP */
+      if (last_prim->mode == GL_LINE_LOOP &&
+          last_count > 0 &&
+          !last_prim->end) {
+         /* draw this section of the incomplete line loop as a line strip */
+         last_prim->mode = GL_LINE_STRIP;
+         if (!last_prim->begin) {
+            /* This is not the first section of the line loop, so don't
+             * draw the 0th vertex.  We're saving it until we draw the
+             * very last section of the loop.
+             */
+            last_prim->start++;
+            last_prim->count--;
+         }
+      }
+
       /* Execute the buffer and save copied vertices.
        */
       if (exec->vtx.vert_count)
@@ -96,6 +113,7 @@ static void vbo_exec_wrap_buffers( struct vbo_exec_context *exec )
 
       if (_mesa_inside_begin_end(exec->ctx)) {
 	 exec->vtx.prim[0].mode = exec->ctx->Driver.CurrentExecPrimitive;
+	 exec->vtx.prim[0].begin = 0;
 	 exec->vtx.prim[0].start = 0;
 	 exec->vtx.prim[0].count = 0;
 	 exec->vtx.prim_count++;
@@ -825,6 +843,24 @@ static void GLAPIENTRY vbo_exec_End( void )
       last_prim->end = 1;
       last_prim->count = exec->vtx.vert_count - last_prim->start;
 
+      /* Special handling for GL_LINE_LOOP */
+      if (last_prim->mode == GL_LINE_LOOP && last_prim->begin == 0) {
+         /* We're finishing drawing a line loop.  Append 0th vertex onto
+          * end of vertex buffer so we can draw it as a line strip.
+          */
+         const fi_type *src = exec->vtx.buffer_map;
+         fi_type *dst = exec->vtx.buffer_map +
+            exec->vtx.vert_count * exec->vtx.vertex_size;
+
+         /* copy 0th vertex to end of buffer */
+         memcpy(dst, src, exec->vtx.vertex_size * sizeof(fi_type));
+
+         assert(last_prim->start == 0);
+         last_prim->start++;  /* skip vertex0 */
+         /* note that last_prim->count stays unchanged */
+         last_prim->mode = GL_LINE_STRIP;
+      }
+
       try_vbo_merge(exec);
    }
 
diff --git a/src/mesa/vbo/vbo_exec_draw.c b/src/mesa/vbo/vbo_exec_draw.c
index f6a1e4bdfad..ed5d9e947b0 100644
--- a/src/mesa/vbo/vbo_exec_draw.c
+++ b/src/mesa/vbo/vbo_exec_draw.c
@@ -109,6 +109,17 @@ vbo_copy_vertices( struct vbo_exec_context *exec )
 	 return 1;
       }
    case GL_LINE_LOOP:
+      if (last_prim->begin == 0) {
+         /* We're dealing with the second or later section of a split/wrapped
+          * GL_LINE_LOOP.  Since we're converting line loops to line strips,
+          * we've already increment the last_prim->start counter by one to
+          * skip the 0th vertex in the loop.  We need to undo that (effectively
+          * subtract one from last_prim->start) so that we copy the 0th vertex
+          * to the next vertex buffer.
+          */
+         src -= sz;
+      }
+      /* fall-through */
    case GL_TRIANGLE_FAN:
    case GL_POLYGON:
       if (nr == 0) {