Merge lp://qastaging/~compiz-team/compiz/compiz.fix_1016367 into lp://qastaging/compiz/0.9.8

Looks OK and works OK on intel. I want to test the NVIDIA driver yet...

Are you completely sure the server grab is necessary? I can't see it having a penalty in my brief intel testing.

It looks like it:

http://developer.download.nvidia.com/opengl/specs/GLX_EXT_texture_from_pixmap.txt

5. Should users be required to re-bind the drawable to a texture after
    the drawable has been rendered to?

    It is difficult to define what the contents of the texture would be if
    we don't require this. Also, requiring this would allow implementations
    to perform an implicit copy at this point if they could not support
    texturing directly out of renderable memory.

    The problem with defining the contents of the texture after rendering
    has occured to the associated drawable is that there is no way to
    synchronize the use of the buffer as a source and as a destination.
    Direct OpenGL rendering is not necessarily done in the same command
    stream as X rendering. At the time the pixmap is used as the source
    for a texturing operation, it could be in a state halfway through a
    copyarea operation in which half of it is say, white, and half is the
    result of the copyarea operation. How is this defined? Worse, some
    other OpenGL application could be halfway through a frame of rendering
    when the composite manager sources from it. The buffer might just
    contain the results of a "glClear" operation at that point.

    To gurantee tear-free rendering, a composite manager would run as follows:

    -receive request for compositing:
    XGrabServer()
    glXWaitX() or XSync()
    glXBindTexImageEXT()

    <Do rendering/compositing>

    glXReleaseTexImageEXT()
    XUngrabServer()

    Apps that don't synchronize like this would get what's available,
    and that may or may not be what they expect.

I'm not sure if this documentation is 100% correct however. The drivers implement glXBindTexImageEXT through a GPU-to-GPU copy, so the server grab is only required for the duration of that copy.

KWin seems to hold a server grab while it does the entire rendering pass. I think that's dangerous in the context of Unity, since Unity has another X connection, and if it does an X call while compiz has the server grab you'll get a nice deadlock.

By the way, I really dislike when a destructor does something outside of its own class (side-effects) like ServerLock sg does.

The reader needs to know what ServerLock does and that it will magically and silently ungrab the server between line 44 and 45. That's bad for maintainability. An implicitly called function like a destructor should never touch anything outside its own object.

> By the way, I really dislike when a destructor does something outside of its
> own class (side-effects) like ServerLock sg does.
>
> The reader needs to know what ServerLock does and that it will magically and
> silently ungrab the server between line 44 and 45. That's bad for
> maintainability. An implicitly called function like a destructor should never
> touch anything outside its own object.

I think this is an exception to the rule. Server grabs, like mutexes are a critical resource. If they are not released, for example, if an exception is thrown, then the entire system deadlocks (seriously). As such, the destructor enforces this rule.

In this instance, ServerLock is taking ownership of something that provides a ServerGrabInterface * and manipulating it such that it is locked on construction, and unlocked on destruction.

It may be better to change the design of ServerLock somewhat so that it operates on a wrapper object which in itself controls a ServerGrabInterface. Though I think that's only repeating the same design.

Added commit message