[Mesa-dev] [PATCH] vbo: reduce number of vertex buffer mappings for vertex attributes

[Marek Olšák]

This looks like a sensible thing to do, but I don't remember all the
details of the vbo module.

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Thu, Oct 15, 2015 at 9:02 PM, Brian Paul <brianp at vmware.com> wrote:
> Whenever we got a glColor, glNormal, glTexCoord, etc. call outside a
> glBegin/End pair, we'd immediately map a vertex buffer to begin
> accumulating vertex data.  In some cases, such as with display lists,
> this led to excessive vertex buffer mapping.  For example, if we have
> a display list such as:
>
> glNewList(42, GL_COMPILE);
> glBegin(prim);
> glVertex2f();
> ...
> glVertex2f();
> glEnd();
> glEndList();
>
> Then did:
>
> glColor3f();
> glCallList(42);
>
> We'd map a vertex buffer as soon as we saw glColor3f but we'd never
> actually write anything to it.  Note that the vertex position data
> was put into a vertex buffer during display list compilation.
>
> With this change, we delay mapping the vertex buffer until we actually
> have a vertex to write to it (triggered by a glVertex() call).  In the
> above case, we no longer map a vertex buffer when setting the color and
> calling the list.
>
> For drivers such as VMware's, reducing buffer mappings gives improved
> performance.
> ---
>  src/mesa/vbo/vbo_exec_api.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
> index 7ae08fe..789869a 100644
> --- a/src/mesa/vbo/vbo_exec_api.c
> +++ b/src/mesa/vbo/vbo_exec_api.c
> @@ -446,10 +446,6 @@ do {                                                                       \
>                                                                          \
>     assert(sz == 1 || sz == 2);                                          \
>                                                                          \
> -   if (unlikely(!(ctx->Driver.NeedFlush & FLUSH_UPDATE_CURRENT))) {     \
> -      vbo_exec_begin_vertices(ctx);                                    \
> -   }                                                                   \
> -                                                                        \
>     /* check if attribute size or type is changing */                    \
>     if (unlikely(exec->vtx.active_sz[A] != N * sz) ||                    \
>         unlikely(exec->vtx.attrtype[A] != T)) {                          \
> @@ -470,6 +466,15 @@ do {                                                                       \
>        /* This is a glVertex call */                                    \
>        GLuint i;                                                                \
>                                                                         \
> +      if (unlikely((ctx->Driver.NeedFlush & FLUSH_UPDATE_CURRENT) == 0)) { \
> +         vbo_exec_begin_vertices(ctx);                                  \
> +      }                                                                 \
> +                                                                        \
> +      if (unlikely(!exec->vtx.buffer_ptr)) {                            \
> +         vbo_exec_vtx_map(exec);                                        \
> +      }                                                                 \
> +      assert(exec->vtx.buffer_ptr);                                     \
> +                                                                        \
>        /* copy 32-bit words */                                           \
>        for (i = 0; i < exec->vtx.vertex_size; i++)                      \
>          exec->vtx.buffer_ptr[i] = exec->vtx.vertex[i];                 \
> @@ -482,7 +487,10 @@ do {                                                                       \
>                                                                         \
>        if (++exec->vtx.vert_count >= exec->vtx.max_vert)                        \
>          vbo_exec_vtx_wrap( exec );                                     \
> -   }                                                                   \
> +   } else {                                                             \
> +      /* we now have accumulated per-vertex attributes */               \
> +      ctx->Driver.NeedFlush |= FLUSH_UPDATE_CURRENT;                    \
> +   }                                                                    \
>  } while (0)
>
>  #define ERROR(err) _mesa_error( ctx, err, __func__ )
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev