Re: [PATCH 2/2] winex11.drv: Make the GL context current even if no drawables were given.

[Alexandre Julliard]

Charles Davis  writes:
> @@ -1833,26 +1833,29 @@ static BOOL glxdrv_wglMakeCurrent(HDC hdc, struct wgl_context *ctx)
>          return TRUE;
>      }
> - if ((gl = get_gl_drawable( WindowFromDC( hdc ), hdc )))
> - if (!hdc || (gl = get_gl_drawable( WindowFromDC( hdc ), hdc )))
> {
> -
>    if (ctx->fmt != gl->format)
> -
>    if (gl && ctx->fmt != gl->format)
>    {
>        WARN( "mismatched pixel format hdc %p %p ctx %p %p\n", hdc, gl->format, ctx, ctx->fmt );
>        SetLastError( ERROR_INVALID_PIXEL_FORMAT );
>        goto done;
>    }
>
> -
>    TRACE("hdc %p drawable %lx fmt %p ctx %p %s\n", hdc, gl->drawable, gl->format, ctx->ctx,
> -
>          debugstr_fbconfig( gl->format->fbconfig ));
> -
>    if (gl)
> -
>        TRACE("hdc %p drawable %lx fmt %p ctx %p %s\n", hdc, gl->drawable, gl->format, ctx->ctx,
> -
>              debugstr_fbconfig( gl->format->fbconfig ));
> -
>    else
> -
>        TRACE("ctx %p (no drawable)\n", ctx->ctx);
>
> -
>    ret = pglXMakeCurrent(gdi_display, gl->drawable, ctx->ctx);
> -
>    ret = pglXMakeCurrent(gdi_display, gl ? gl->drawable : None, ctx->ctx);
>    if (ret)
>    {
>        NtCurrentTeb()->glContext = ctx;
>        ctx->has_been_current = TRUE;
>        ctx->hdc = hdc;
> -
>        ctx->drawables[0] = gl->drawable;
> -
>        ctx->drawables[1] = gl->drawable;
> -
>        ctx->drawables[0] = gl ? gl->drawable : 0;
> -
>        ctx->drawables[1] = gl ? gl->drawable : 0;

You really want a separate code path instead of adding pointer checks
everywhere.

[PATCH 2/2] winex11.drv: Make the GL context current even if no drawables were given.

[Ken Thomases]

On Feb 23, 2016, at 3:07 AM, Alexandre Julliard  wrote:
> Charles Davis  writes:
> > @@ -1833,26 +1833,29 @@ static BOOL glxdrv_wglMakeCurrent(HDC hdc, struct wgl_context *ctx)
> >         return TRUE;
> >     }
> > - if ((gl = get_gl_drawable( WindowFromDC( hdc ), hdc )))
> > - if (!hdc || (gl = get_gl_drawable( WindowFromDC( hdc ), hdc )))
> > {
> > -
> >    if (ctx->fmt != gl->format)
> > -
> >    if (gl && ctx->fmt != gl->format)
> >   {
> >       WARN( "mismatched pixel format hdc %p %p ctx %p %p\n", hdc, gl->format, ctx, ctx->fmt );
> >       SetLastError( ERROR_INVALID_PIXEL_FORMAT );
> >       goto done;
> >   }
> >
> > -
> >    TRACE("hdc %p drawable %lx fmt %p ctx %p %s\n", hdc, gl->drawable, gl->format, ctx->ctx,
> > -
> >          debugstr_fbconfig( gl->format->fbconfig ));
> > -
> >    if (gl)
> > -
> >        TRACE("hdc %p drawable %lx fmt %p ctx %p %s\n", hdc, gl->drawable, gl->format, ctx->ctx,
> > -
> >              debugstr_fbconfig( gl->format->fbconfig ));
> > -
> >    else
> > -
> >        TRACE("ctx %p (no drawable)\n", ctx->ctx);
> >
> > -
> >    ret = pglXMakeCurrent(gdi_display, gl->drawable, ctx->ctx);
> > -
> >    ret = pglXMakeCurrent(gdi_display, gl ? gl->drawable : None, ctx->ctx);
> >   if (ret)
> >   {
> >       NtCurrentTeb()->glContext = ctx;
> >       ctx->has_been_current = TRUE;
> >       ctx->hdc = hdc;
> > -
> >        ctx->drawables[0] = gl->drawable;
> > -
> >        ctx->drawables[1] = gl->drawable;
> > -
> >        ctx->drawables[0] = gl ? gl->drawable : 0;
> > -
> >        ctx->drawables[1] = gl ? gl->drawable : 0;
>
> You really want a separate code path instead of adding pointer checks
> everywhere.

Not to mention that if hdc is NULL, then gl is just uninitialized and all of those checks are invalid, as is the release_gl_drawable() call at the end of the function.

That said, some of the existing code path can be shared by creating a "drawable" variable (of type Drawable).

-Ken