[Mesa-dev] [PATCH 1/4] glx: Call __glXInitVertexArrayState() with a usable gc.

[Ian Romanick]

On 06/23/2016 11:15 AM, Matt Turner wrote:
> From: Colin McDonald <cjmmail10-bz at yahoo.co.uk>
> 
> For each indirect context the indirect vertex array state must be initialised
> by __glXInitVertexArrayState in indirect_vertex_array.c.  As noted in the
> routine header it requires that the glx context has been setup prior to the
> call, in order to test the server version and extensions.
> 
> Currently __glXInitVertexArrayState is called from indirect_bind_context in
> indirect_glx.c, as follows:
> 
>    state = gc->client_state_private;
>    if (state->array_state == NULL) {
>       glGetString(GL_EXTENSIONS);
>       glGetString(GL_VERSION);
>       __glXInitVertexArrayState(gc);
>    }
> 
> But, the gc context is not yet usable at this stage, so the server queries
> fail, and __glXInitVertexArrayState is called without the server version and
> extension information it needs.  This breaks multi-texturing as
> glXInitVertexArrayState doesn't get GL_MAX_TEXTURE_UNITS.  It probably also
> breaks setup of other arrays: fog, secondary colour, vertex attributes.
> 
> To fix this I have moved the call to __glXInitVertexArrayState to the end of
> MakeContextCurrent in glxcurrent.c, where the glx context is usable.
> 
> Fixes a regression caused by commit 4fbdde889c. Fixes ARB_vertex_program
> usage in the arbvparray Mesa demo when run with indirect GLX and also
> the tex-skipped-unit piglit test when run with indirect GLX.
> 
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61907
> ---
>  src/glx/glxcurrent.c   | 12 ++++++++++++
>  src/glx/indirect_glx.c |  8 --------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
> index f2e3865..d119326 100644
> --- a/src/glx/glxcurrent.c
> +++ b/src/glx/glxcurrent.c
> @@ -252,6 +252,18 @@ MakeContextCurrent(Display * dpy, GLXDrawable draw,
>  
>     __glXUnlock();
>  
> +   /* The indirect vertex array state must to be initialised after we
> +    * have setup the context, as it needs to query server attributes.
> +    */
> +   if (gc && !gc->isDirect) {
> +      __GLXattribute *state = gc->client_state_private;
> +      if (state && state->array_state == NULL) {
> +         glGetString(GL_EXTENSIONS);
> +         glGetString(GL_VERSION);
> +         __glXInitVertexArrayState(gc);
> +      }
> +   }
> +

This is a pretty ugly layering violation, but it may be the best way.
What's the precise problem?  Is it that __glXSetCurrentContext(gc)
hasn't happened yet or is it that __glXUnlock() hasn't happened yet?

>     return GL_TRUE;
>  }
>  
> diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
> index bb121f8..26825fb 100644
> --- a/src/glx/indirect_glx.c
> +++ b/src/glx/indirect_glx.c
> @@ -131,7 +131,6 @@ indirect_bind_context(struct glx_context *gc, struct glx_context *old,
>  		      GLXDrawable draw, GLXDrawable read)
>  {
>     GLXContextTag tag;
> -   __GLXattribute *state;
>     Display *dpy = gc->psc->dpy;
>     int opcode = __glXSetupForCommand(dpy);
>     Bool sent;
> @@ -150,13 +149,6 @@ indirect_bind_context(struct glx_context *gc, struct glx_context *old,
>        IndirectAPI = __glXNewIndirectAPI();
>     _glapi_set_dispatch(IndirectAPI);
>  
> -   state = gc->client_state_private;
> -   if (state->array_state == NULL) {
> -      glGetString(GL_EXTENSIONS);
> -      glGetString(GL_VERSION);
> -      __glXInitVertexArrayState(gc);
> -   }
> -
>     return !sent;
>  }
>  
>