[Mesa-dev] [PATCH 09/10] vbo: fix GL_LINE_LOOP stray line bug

Roland Scheidegger sroland at vmware.com
Sat Oct 17 05:20:34 PDT 2015


FWIW this probably fixes
https://bugs.freedesktop.org/show_bug.cgi?id=49779 and
https://bugs.freedesktop.org/show_bug.cgi?id=28130 (in contrast to 81174
which as you noted suffers both from a vbo and draw issue). (I believe
the issue in draw is pretty much the same, since things are split with
too large buffers we end up with multiple (incorrect) line loops.)

Roland


Am 16.10.2015 um 23:25 schrieb Brian Paul:
> 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.
> 
> Most 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.
> 
> Bugzilla: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D81174&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=czG7W6Vef1aFb_D_GLNGA4H62ZBA3g5g8aoKQ6VxP98&s=Cu81rzD4sulAjES4I4QraL70LU5mqxScNcy3-9wGvqc&e= 
> ---
>  src/mesa/vbo/vbo_context.h   |  5 ++++-
>  src/mesa/vbo/vbo_exec_api.c  | 38 +++++++++++++++++++++++++++++++++++++-
>  src/mesa/vbo/vbo_exec_draw.c | 12 ++++++++++++
>  3 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/vbo/vbo_context.h b/src/mesa/vbo/vbo_context.h
> index 1e85335..28f43b4 100644
> --- a/src/mesa/vbo/vbo_context.h
> +++ b/src/mesa/vbo/vbo_context.h
> @@ -205,7 +205,10 @@ 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) /
> +   /* Subtract one so we're always sure to have room for an extra
> +    * vertex for GL_LINE_LOOP -> GL_LINE_STRIP conversion.
> +    */
> +   return (VBO_VERT_BUFFER_SIZE - exec->vtx.buffer_used - 1) /
>            (exec->vtx.vertex_size * sizeof(GLfloat));
>  }
>  
> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
> index 0116f56..3c25ec0 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 )
>  {
> @@ -83,6 +84,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)
> @@ -98,6 +115,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++;
> @@ -827,6 +845,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 397fc09..f91f7c8 100644
> --- a/src/mesa/vbo/vbo_exec_draw.c
> +++ b/src/mesa/vbo/vbo_exec_draw.c
> @@ -110,6 +110,18 @@ 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.
> +          */
> +         assert(last_prim->start > 0);
> +         src -= exec->vtx.vertex_size;
> +      }
> +      /* fall-through */
>     case GL_TRIANGLE_FAN:
>     case GL_POLYGON:
>        if (nr == 0) {
> 



More information about the mesa-dev mailing list