[Cogl] [PATCH] Bind the dummy surface or drawable when current onscreen is destroyed

[Robert Bragg]

This looks good to land to me:

Reviewed-by: Robert Bragg <robert at linux.intel.com>

thanks,
- Robert

On Thu, Jan 24, 2013 at 4:32 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Similar to commit 2c0cfdefbb9d1 for the SDL2 winsys, the GLX and EGL
> window systems need to bind the dummy surface or drawable when the
> currently bound onscreen is destroyed so that there will always be a
> valid context bound.
>
> Previously I got the idea that this would not be necessary on GLX
> because the documentation for glXDestroyDrawable states that the
> drawable won't actually be destroyed if it is currently bound until it
> becomes unbound. However it doesn't say what happens if the underlying
> X window is also destroyed and after testing it seems this causes a
> segfault in Mesa in GLX and an XError for EGLX.
> ---
>  cogl/winsys/cogl-winsys-egl.c | 17 ++++++++++++++++-
>  cogl/winsys/cogl-winsys-glx.c | 27 ++++++++++++++++++++++++++-
>  2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/cogl/winsys/cogl-winsys-egl.c b/cogl/winsys/cogl-winsys-egl.c
> index cd6ae3a..223aa84 100644
> --- a/cogl/winsys/cogl-winsys-egl.c
> +++ b/cogl/winsys/cogl-winsys-egl.c
> @@ -3,7 +3,7 @@
>   *
>   * An object oriented GL/GLES Abstraction/Utility Layer
>   *
> - * Copyright (C) 2007,2008,2009,2010,2011 Intel Corporation.
> + * Copyright (C) 2007,2008,2009,2010,2011,2013 Intel Corporation.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -625,6 +625,7 @@ _cogl_winsys_onscreen_deinit (CoglOnscreen *onscreen)
>  {
>    CoglFramebuffer *framebuffer = COGL_FRAMEBUFFER (onscreen);
>    CoglContext *context = framebuffer->context;
> +  CoglDisplayEGL *egl_display = context->display->winsys;
>    CoglRenderer *renderer = context->display->renderer;
>    CoglRendererEGL *egl_renderer = renderer->winsys;
>    CoglOnscreenEGL *egl_onscreen = onscreen->winsys;
> @@ -632,8 +633,22 @@ _cogl_winsys_onscreen_deinit (CoglOnscreen *onscreen)
>    /* If we never successfully allocated then there's nothing to do */
>    if (egl_onscreen == NULL)
>      return;
> +
>    if (egl_onscreen->egl_surface != EGL_NO_SURFACE)
>      {
> +      /* Cogl always needs a valid context bound to something so if we
> +       * are destroying the onscreen that is currently bound we'll
> +       * switch back to the dummy drawable. */
> +      if (egl_display->dummy_surface != EGL_NO_SURFACE &&
> +          (egl_display->current_draw_surface == egl_onscreen->egl_surface ||
> +           egl_display->current_read_surface == egl_onscreen->egl_surface))
> +        {
> +          _cogl_winsys_egl_make_current (context->display,
> +                                         egl_display->dummy_surface,
> +                                         egl_display->dummy_surface,
> +                                         egl_display->current_context);
> +        }
> +
>        if (eglDestroySurface (egl_renderer->edpy, egl_onscreen->egl_surface)
>            == EGL_FALSE)
>          g_warning ("Failed to destroy EGL surface");
> diff --git a/cogl/winsys/cogl-winsys-glx.c b/cogl/winsys/cogl-winsys-glx.c
> index 3a0abba..6301c38 100644
> --- a/cogl/winsys/cogl-winsys-glx.c
> +++ b/cogl/winsys/cogl-winsys-glx.c
> @@ -3,7 +3,7 @@
>   *
>   * An object oriented GL/GLES Abstraction/Utility Layer
>   *
> - * Copyright (C) 2007,2008,2009,2010,2011 Intel Corporation.
> + * Copyright (C) 2007,2008,2009,2010,2011,2013 Intel Corporation.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -1036,12 +1036,15 @@ _cogl_winsys_onscreen_deinit (CoglOnscreen *onscreen)
>  {
>    CoglFramebuffer *framebuffer = COGL_FRAMEBUFFER (onscreen);
>    CoglContext *context = framebuffer->context;
> +  CoglContextGLX *glx_context = context->winsys;
> +  CoglGLXDisplay *glx_display = context->display->winsys;
>    CoglXlibRenderer *xlib_renderer =
>      _cogl_xlib_renderer_get_data (context->display->renderer);
>    CoglGLXRenderer *glx_renderer = context->display->renderer->winsys;
>    CoglXlibTrapState old_state;
>    CoglOnscreenXlib *xlib_onscreen = onscreen->winsys;
>    CoglOnscreenGLX *glx_onscreen = onscreen->winsys;
> +  GLXDrawable drawable;
>
>    /* If we never successfully allocated then there's nothing to do */
>    if (glx_onscreen == NULL)
> @@ -1049,6 +1052,28 @@ _cogl_winsys_onscreen_deinit (CoglOnscreen *onscreen)
>
>    _cogl_xlib_renderer_trap_errors (context->display->renderer, &old_state);
>
> +  drawable =
> +    glx_onscreen->glxwin == None ? xlib_onscreen->xwin : glx_onscreen->glxwin;
> +
> +  /* Cogl always needs a valid context bound to something so if we are
> +   * destroying the onscreen that is currently bound we'll switch back
> +   * to the dummy drawable. Although the documentation for
> +   * glXDestroyWindow states that a currently bound window won't
> +   * actually be destroyed until it is unbound, it looks like this
> +   * doesn't work if the X window itself is destroyed */
> +  if (drawable == glx_context->current_drawable)
> +    {
> +      GLXDrawable dummy_drawable = (glx_display->dummy_glxwin == None ?
> +                                    glx_display->dummy_xwin :
> +                                    glx_display->dummy_glxwin);
> +
> +      glx_renderer->glXMakeContextCurrent (xlib_renderer->xdpy,
> +                                           dummy_drawable,
> +                                           dummy_drawable,
> +                                           glx_display->glx_context);
> +      glx_context->current_drawable = dummy_drawable;
> +    }
> +
>    if (glx_onscreen->glxwin != None)
>      {
>        glx_renderer->glXDestroyWindow (xlib_renderer->xdpy,
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl