Code review comment for lp://qastaging/~sil2100/compiz/revert_3427_0.9.8

Now that I've had a look at the context, this is definitely unsafe for LLVMpipe and other drivers that require that the pixmap be valid on the server side in order to be bound to a texture:

bool
cgl::bindTexImageGLX (ServerGrabInterface *serverGrabInterface,
        Pixmap x11Pixmap,
        GLXPixmap glxPixmap,
        const cgl::PixmapCheckValidityFunc &checkPixmapValidity,
        const cgl::BindTexImageEXTFunc &bindTexImageEXT,
        const cgl::WaitGLXFunc &waitGLX,
        cgl::PixmapSource source)
{
#ifndef LP_1030891_NOT_FIXED
    ServerLock lock (serverGrabInterface);

    waitGLX ();
#endif

    /* External pixmaps can disappear on us, but not
     * while we have a server grab at least */
    if (source == cgl::ExternallyManaged)
    {
#ifdef LP_1030891_NOT_FIXED
 ServerLock lock (serverGrabInterface);
#endif
 if (!checkPixmapValidity (x11Pixmap))
     return false;
    }

    bindTexImageEXT (glxPixmap);

    return true;
}

IF checkPixmapValidity succeeds, and then the scheduler interrupts us between that and the end of bindTexImageEXT, and the application that controls the external pixmap frees and it causes a server sync, then those drivers will crash due to the race condition.

On the other side of the coin, older versions of VirtualBox will hang or crash in the driver when the very first pixmap to be bound is done so while a server grab is active, because the driver is (IMO) broken and does XOpenDisplay using glXBindTexImageEXT.

I've been assured, and have checked in the code that newer versions of VirtualBox do not do this (they call XOpenDisplay in glXCreateContext).

The most appropriate workaround in that case would probably be to manually create, bind, unbind and free a pixmap within the opengl plugin startup code. It would look something like this:

Pixmap p = XCreatePixmap (dpy, root, 1, 1, CopyFromParent);
unsigned int width, height, depth;
Window root;
int x, y;

XGetWindowAttributes (dpy, p, &x, &y, &width, &height, &depth);

GLXPixmap glxP = GL::createPixmap (dpy, p, GLScreen::get (screen)->priv->fbConfigs[depth]);
GLuint tex;
glGenTextures (1, &tex);
glBindTexture (GL_TEXTURE_2D, tex);
GL::bindTexImage (dpy, glxP);
GL::releaseTexImage (dpy, glXP);
GL::destroyPixmap (dpy, glXP);
glDeleteTextures (tex);
XFreePixmap (dpy, p);

That's only off the top of my head - have a look at TfpTexture for exactly how it is done.