[Mesa-dev] [PATCH 09/10] vbo: fix GL_LINE_LOOP stray line bug
Sinclair Yeh
syeh at vmware.com
Sun Oct 18 14:06:42 PDT 2015
On Fri, Oct 16, 2015 at 03:25:16PM -0600, Brian Paul wrote:
> 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://bugs.freedesktop.org/show_bug.cgi?id=81174
> ---
> 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));
We may need to check for a negaive result here just in case
buffer_used is very close or equal to VBO_VERT_BUFFER_SIZE.
> }
>
> 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.
> + */
I don't understand some of the prim fields, so I'm assuming "start" is
an index into the VBO, i.e. [0 ... start ... n] where 'n' is index to
the last vertex.
With that assumption, if we do the below, then don't we miss the
[start -> start + 1] strip?
> + 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) {
> --
> 1.9.1
>
More information about the mesa-dev
mailing list