[3/6] glx: Move vertex array protocol state into the indirect backend

Submitted by Adam Jackson on Nov. 14, 2017, 8:13 p.m.

Details

Message ID 20171114201306.28276-4-ajax@redhat.com
State New
Headers show
Series "glx: Implement GLX_EXT_no_config_context" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Adam Jackson Nov. 14, 2017, 8:13 p.m.
Only relevant for indirect contexts, so let's get that code out of the
common path.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 src/glx/glxcurrent.c   | 12 ------------
 src/glx/indirect_glx.c | 18 +++++++++++++++---
 2 files changed, 15 insertions(+), 15 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
index fd04929b89..9f8bf7cee1 100644
--- a/src/glx/glxcurrent.c
+++ b/src/glx/glxcurrent.c
@@ -238,18 +238,6 @@  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);
-      }
-   }
-
    return GL_TRUE;
 }
 
diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
index cfae12f6c0..b552b5768a 100644
--- a/src/glx/indirect_glx.c
+++ b/src/glx/indirect_glx.c
@@ -148,9 +148,21 @@  indirect_bind_context(struct glx_context *gc, struct glx_context *old,
    sent = SendMakeCurrentRequest(dpy, gc->xid, tag, draw, read,
 				 &gc->currentContextTag);
 
-   if (!IndirectAPI)
-      IndirectAPI = __glXNewIndirectAPI();
-   _glapi_set_dispatch(IndirectAPI);
+   if (sent) {
+      if (!IndirectAPI)
+         IndirectAPI = __glXNewIndirectAPI();
+      _glapi_set_dispatch(IndirectAPI);
+
+      /* The indirect vertex array state must to be initialised after we
+       * have setup the context, as it needs to query server attributes.
+       */
+      __GLXattribute *state = gc->client_state_private;
+      if (state && state->array_state == NULL) {
+         glGetString(GL_EXTENSIONS);
+         glGetString(GL_VERSION);
+         __glXInitVertexArrayState(gc);
+      }
+   }
 
    return !sent;
 }

Comments

On 11/14/2017 12:13 PM, Adam Jackson wrote:
> Only relevant for indirect contexts, so let's get that code out of the
> common path.
> 
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  src/glx/glxcurrent.c   | 12 ------------
>  src/glx/indirect_glx.c | 18 +++++++++++++++---
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
> index fd04929b89..9f8bf7cee1 100644
> --- a/src/glx/glxcurrent.c
> +++ b/src/glx/glxcurrent.c
> @@ -238,18 +238,6 @@ 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);
> -      }
> -   }
> -
>     return GL_TRUE;
>  }
>  
> diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
> index cfae12f6c0..b552b5768a 100644
> --- a/src/glx/indirect_glx.c
> +++ b/src/glx/indirect_glx.c
> @@ -148,9 +148,21 @@ indirect_bind_context(struct glx_context *gc, struct glx_context *old,
>     sent = SendMakeCurrentRequest(dpy, gc->xid, tag, draw, read,
>  				 &gc->currentContextTag);
>  
> -   if (!IndirectAPI)
> -      IndirectAPI = __glXNewIndirectAPI();
> -   _glapi_set_dispatch(IndirectAPI);
> +   if (sent) {
> +      if (!IndirectAPI)
> +         IndirectAPI = __glXNewIndirectAPI();
> +      _glapi_set_dispatch(IndirectAPI);
> +
> +      /* The indirect vertex array state must to be initialised after we
> +       * have setup the context, as it needs to query server attributes.
> +       */
> +      __GLXattribute *state = gc->client_state_private;
> +      if (state && state->array_state == NULL) {
> +         glGetString(GL_EXTENSIONS);
> +         glGetString(GL_VERSION);
> +         __glXInitVertexArrayState(gc);
> +      }
> +   }

This is where this code used to be, but commit d57c85c1 moved it.  Does
this not re-regress things?  I guess wrapping it in 'if (sent)' seems
like it ought to be enough to prevent the original problem.

>  
>     return !sent;
>  }
>
On Tue, 2017-11-14 at 13:15 -0800, Ian Romanick wrote:
> On 11/14/2017 12:13 PM, Adam Jackson wrote:
> > diff --git a/src/glx/indirect_glx.c b/src/glx/indirect_glx.c
> > index cfae12f6c0..b552b5768a 100644
> > --- a/src/glx/indirect_glx.c
> > +++ b/src/glx/indirect_glx.c
> > @@ -148,9 +148,21 @@ indirect_bind_context(struct glx_context *gc, struct glx_context *old,
> >     sent = SendMakeCurrentRequest(dpy, gc->xid, tag, draw, read,
> >  				 &gc->currentContextTag);
> >  
> > -   if (!IndirectAPI)
> > -      IndirectAPI = __glXNewIndirectAPI();
> > -   _glapi_set_dispatch(IndirectAPI);
> > +   if (sent) {
> > +      if (!IndirectAPI)
> > +         IndirectAPI = __glXNewIndirectAPI();
> > +      _glapi_set_dispatch(IndirectAPI);
> > +
> > +      /* The indirect vertex array state must to be initialised after we
> > +       * have setup the context, as it needs to query server attributes.
> > +       */
> > +      __GLXattribute *state = gc->client_state_private;
> > +      if (state && state->array_state == NULL) {
> > +         glGetString(GL_EXTENSIONS);
> > +         glGetString(GL_VERSION);
> > +         __glXInitVertexArrayState(gc);
> > +      }
> > +   }
> 
> This is where this code used to be, but commit d57c85c1 moved it.  Does
> this not re-regress things?  I guess wrapping it in 'if (sent)' seems
> like it ought to be enough to prevent the original problem.

Eep, good catch. I think this would indeed re-regress things. We're
calling ::bind before __glXSetCurrentContext - in fact with the dummy
context bound - which means those glGetStrings would probably fizzle.

I'll dig into this a bit further, hopefully a real piglit run will
shake out this and anything else remaining.

- ajax