[Mesa-dev] [PATCH] vbo: signal _NEW_ARRAY when transitioning between glBegin/End, glDrawArrays

[Jose Fonseca]

Looks good to me Brian.

I assume it is already impossible for the arrays to change in such subtle manner within a single draw method, without provoking _NEW_ARRAY to be set.

Jose


----- Original Message -----
> This fixes a regression seen with the isosurf demo when switching
> between
> glBegin/End and glDrawArrays (do it several times).  The problem was
> the
> driver wasn't getting _NEW_ARRAY when the arrays were subtly changed:
> (vertex3f, normal3f) vs. (normal3f, vertex3f).
> 
> This patch fixes that by signaling _NEW_ARRAY whenever we transition
> between glBegin/End and glDrawArrays mode.
> 
> The patch also fixes up the initialization of the map_vp_none[] array
> to stop putting strange values in the last five elements of the
> array.
> ---
>  src/mesa/vbo/vbo_context.c    |   11 ++++-------
>  src/mesa/vbo/vbo_exec.h       |   33
>  +++++++++++++++++++++++++++++++++
>  src/mesa/vbo/vbo_exec_api.c   |    2 ++
>  src/mesa/vbo/vbo_exec_array.c |    4 ++++
>  4 files changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/vbo/vbo_context.c b/src/mesa/vbo/vbo_context.c
> index b2e6bbc..d83f2fd 100644
> --- a/src/mesa/vbo/vbo_context.c
> +++ b/src/mesa/vbo/vbo_context.c
> @@ -176,17 +176,14 @@ GLboolean _vbo_CreateContext( struct gl_context
> *ctx )
>     {
>        GLuint i;
>  
> -      /* When no vertex program, pull in the material attributes in
> -       * the generic range.
> -       */
> -      for (i = 0; i < VERT_ATTRIB_FF_MAX; i++)
> +      /* identity mapping */
> +      for (i = 0; i < Elements(vbo->map_vp_none); i++)
>  	 vbo->map_vp_none[i] = i;
> +      /* map material attribs to generic slots */
>        for (i = 0; i < NR_MAT_ATTRIBS; i++)
>  	 vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)]
>              = VBO_ATTRIB_MAT_FRONT_AMBIENT + i;
> -      for (i = NR_MAT_ATTRIBS; i < VERT_ATTRIB_GENERIC_MAX; i++)
> -	 vbo->map_vp_none[VERT_ATTRIB_GENERIC(i)] = i;
> -
> +
>        for (i = 0; i < Elements(vbo->map_vp_arb); i++)
>  	 vbo->map_vp_arb[i] = i;
>     }
> diff --git a/src/mesa/vbo/vbo_exec.h b/src/mesa/vbo/vbo_exec.h
> index cfed8e8..339461d 100644
> --- a/src/mesa/vbo/vbo_exec.h
> +++ b/src/mesa/vbo/vbo_exec.h
> @@ -78,12 +78,23 @@ struct vbo_exec_copied_vtx {
>  };
>  
>  
> +enum draw_method
> +{
> +   DRAW_NONE,
> +   DRAW_BEGIN_END,
> +   DRAW_ELEMENTS,
> +   DRAW_ARRAYS
> +};
> +
> +
>  struct vbo_exec_context
>  {
>     struct gl_context *ctx;
>     GLvertexformat vtxfmt;
>     GLvertexformat vtxfmt_noop;
>  
> +   enum draw_method last_draw_method;
> +
>     struct {
>        struct gl_buffer_object *bufferobj;
>  
> @@ -164,6 +175,28 @@ void vbo_exec_array_destroy( struct
> vbo_exec_context *exec );
>  void vbo_exec_vtx_init( struct vbo_exec_context *exec );
>  void vbo_exec_vtx_destroy( struct vbo_exec_context *exec );
>  
> +
> +/**
> + * This is called by glBegin, glDrawArrays and glDrawElements (and
> + * variations of those calls).  When we transition from immediate
> mode
> + * drawing to array drawing we need to invalidate the array state.
> + *
> + * glBegin/End builds vertex arrays.  Those arrays may look
> identical
> + * to glDrawArrays arrays except that the position of the elements
> may
> + * be different.  For example, arrays of (position3v, normal3f) vs.
> arrays
> + * of (normal3f, position3f).  So we need to make sure we notify
> drivers
> + * that arrays may be changing.
> + */
> +static inline void
> +vbo_draw_method(struct vbo_exec_context *exec, enum draw_method
> method)
> +{
> +   if (exec->last_draw_method != method) {
> +      exec->ctx->NewState |= _NEW_ARRAY;
> +      exec->last_draw_method = method;
> +   }
> +}
> +
> +
>  #if FEATURE_beginend
>  
>  void vbo_exec_vtx_flush( struct vbo_exec_context *exec, GLboolean
>  unmap );
> diff --git a/src/mesa/vbo/vbo_exec_api.c
> b/src/mesa/vbo/vbo_exec_api.c
> index 4be0169..cb5f9ae 100644
> --- a/src/mesa/vbo/vbo_exec_api.c
> +++ b/src/mesa/vbo/vbo_exec_api.c
> @@ -701,6 +701,8 @@ static void GLAPIENTRY vbo_exec_Begin( GLenum
> mode )
>           return;
>        }
>  
> +      vbo_draw_method(exec, DRAW_BEGIN_END);
> +
>        if (ctx->Driver.PrepareExecBegin)
>  	 ctx->Driver.PrepareExecBegin(ctx);
>  
> diff --git a/src/mesa/vbo/vbo_exec_array.c
> b/src/mesa/vbo/vbo_exec_array.c
> index a6e41e9..e84c0f3 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -558,6 +558,8 @@ vbo_draw_arrays(struct gl_context *ctx, GLenum
> mode, GLint start,
>  
>     vbo_bind_arrays(ctx);
>  
> +   vbo_draw_method(exec, DRAW_ARRAYS);
> +
>     /* Again... because we may have changed the bitmask of per-vertex
>     varying
>      * attributes.  If we regenerate the fixed-function vertex
>      program now
>      * we may be able to prune down the number of vertex attributes
>      which we
> @@ -773,6 +775,8 @@ vbo_validated_drawrangeelements(struct gl_context
> *ctx, GLenum mode,
>  
>     vbo_bind_arrays( ctx );
>  
> +   vbo_draw_method(exec, DRAW_ELEMENTS);
> +
>     /* check for dirty state again */
>     if (ctx->NewState)
>        _mesa_update_state( ctx );
> --
> 1.7.3.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>