From c878acf6cf4c0e4532d57538f9f5892616f9b610 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 4 Oct 2010 14:12:21 +0100 Subject: [PATCH] cogl-x11: Trap glXDestroyPixmap() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are ordering issues in the pixmap destruction with current and past X11 server, Mesa and dri2. Under some circumstances, an X pixmap might be destroyed with the GLX pixmap still referencing it, and thus the X server will decide to destroy the GLX pixmap as well; then, when Cogl tries to destroy the GLX pixmap, it gets BadDrawable errors. Clutter 1.2 used to trap + sync all calls to glXDestroyPixmap(), but then we assumed that the ordering issue had been solved. So, we're back to square 1. I left a Big Fat Comment™ right above the glXDestroyPixmap() call referencing the bug and the reasoning behind the trap, so that we don't go and remove it in the future without checking that the issue has been in fact solved. http://bugzilla.clutter-project.org/show_bug.cgi?id=2324 --- cogl/winsys/cogl-texture-pixmap-x11.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/cogl/winsys/cogl-texture-pixmap-x11.c b/cogl/winsys/cogl-texture-pixmap-x11.c index 56097ea15..be0c30229 100644 --- a/cogl/winsys/cogl-texture-pixmap-x11.c +++ b/cogl/winsys/cogl-texture-pixmap-x11.c @@ -888,12 +888,34 @@ _cogl_texture_pixmap_x11_free_glx_pixmap (CoglTexturePixmapX11 *tex_pixmap) { if (tex_pixmap->glx_pixmap) { + CoglXlibTrapState trap_state; + _COGL_GET_CONTEXT (ctx, NO_RETVAL); if (tex_pixmap->pixmap_bound) glXReleaseTexImage (_cogl_xlib_get_display (), tex_pixmap->glx_pixmap, GLX_FRONT_LEFT_EXT); + + /* FIXME - we need to trap errors and synchronize here because + * of ordering issues between the XPixmap destruction and the + * GLXPixmap destruction. + * + * If the X pixmap is destroyed, the GLX pixmap is destroyed as + * well immediately, and thus, when Cogl calls glXDestroyPixmap() + * it'll cause a BadDrawable error. + * + * this is technically a bug in the X server, which should not + * destroy either pixmaps until the call to glXDestroyPixmap(); so + * at some point we should revisit this code and remove the + * trap+sync after verifying that the destruction is indeed safe. + * + * for reference, see: + * http://bugzilla.clutter-project.org/show_bug.cgi?id=2324 + */ + _cogl_xlib_trap_errors (&trap_state); glXDestroyPixmap (_cogl_xlib_get_display (), tex_pixmap->glx_pixmap); + XSync (_cogl_xlib_get_display (), False); + _cogl_xlib_untrap_errors (&trap_state); tex_pixmap->glx_pixmap = None; tex_pixmap->pixmap_bound = FALSE;