On 5 December 2017 at 21:22, Adam Jackson <a...@redhat.com> wrote:
> Somewhat terrifyingly, we never sent this for direct contexts, which
> means the server never knew the context/drawable bindings.
>
> To handle this sanely, pull the request code up out of the indirect
> backend, and rewrite the context switch path to call it as appropriate.
> This attempts to preserve the existing behavior of not calling unbind()
> on the context if its refcount would not drop to zero, which is why the
> diff is a little uglier than I'd like.
>
> Fixes glx-make-context, glx-multi-context-single-window and
> glx-query-drawable-glx_fbconfig_id-window with both direct and indirect
> contexts. glx-make-glxdrawable-current regresses, but I believe the test
> is in error, as the fbconfig it uses to create the context is just
> "something double-buffered and rgba" as opposed to "corresponds to the
> visual I used to create the windows".
>
> v2: Style fixups (Ian Romanick)
>
Adam, can we make this 1/3 and cc stable?
> Signed-off-by: Adam Jackson <a...@redhat.com>
> ---
> src/glx/glxcurrent.c | 182
> +++++++++++++++++++++++++++++++++++++------------
> src/glx/indirect_glx.c | 140 +++++++------------------------------
> 2 files changed, 162 insertions(+), 160 deletions(-)
>
> diff --git a/src/glx/glxcurrent.c b/src/glx/glxcurrent.c
> index 9f8bf7cee1..0ce80670d2 100644
> --- a/src/glx/glxcurrent.c
> +++ b/src/glx/glxcurrent.c
> @@ -165,17 +165,85 @@ glXGetCurrentDrawable(void)
> return gc->currentDrawable;
> }
>
> -/**
> - * Make a particular context current.
> - *
> - * \note This is in this file so that it can access dummyContext.
> - */
> +static Bool
> +SendMakeCurrentRequest(Display * dpy, GLXContextID gc_id,
> + GLXContextTag gc_tag, GLXDrawable draw,
> + GLXDrawable read, GLXContextTag *out_tag)
> +{
Nit: I would keep the code move a separate patch, I might be too
pedantic though ;-)
> static Bool
> MakeContextCurrent(Display * dpy, GLXDrawable draw,
> GLXDrawable read, GLXContext gc_user)
> {
> struct glx_context *gc = (struct glx_context *) gc_user;
> struct glx_context *oldGC = __glXGetCurrentContext();
> + Bool ret = GL_FALSE;
Bool is True False. Then again I would drop the variable and
__glXUnlock/return as applicable.
> - if (gc->vtable->bind(gc, oldGC, draw, read) != Success) {
> + if (gc->vtable->bind(gc, gc, draw, read) != Success) {
> __glXSetCurrentContextNull();
This line seems inconsistent/wrong.
The glXMakeCurrent manpage says "If False is returned, the previously
current rendering context and drawable (if any) remain unchanged."
> + if (gc->thread_refcount == 0) {
> + SetGC(gc, dpy, draw, read);
> + }
Nit: drop the brackets for the single like if statement.
> + if (state != NULL && state->array_state != NULL)
> + return Success;
>
> - return !sent;
> + return BadAlloc;
Nit: diverge the exception (error path) keeping the main codeflow straight.
if (state == NULL || state->array_state == NULL)
return BadAlloc;
return Success;
> }
>
> static void
> indirect_unbind_context(struct glx_context *gc, struct glx_context *new)
> {
Nit: add a comment to link "nothing to do here - MakeContextCurrent
sends the required protocol as applicable"
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
