Bug 991

Note that there were some alternative solutions, one of which involved a hack
inside Xlib, but that only hid the problem and didn't actually solve it.

Composite currently exposes both a depth-24 RGB visual and a depth-32 ARGB
visual.  I added the depth-24 RGB visual to get Glitz working on my 16-bit
screen (with software Mesa); I suggest that this visual be made optional within
the server; I don't expect it will be all that useful to people.  It might even
want to live as a compile-time option.

It's the depth-32 ARGB visual which causes the most pain though.  Applications
which stumble across it end up doing the wrong thing almost without exception.

Looking at the Xlib API, I think we can change the Composite extension to report
new visuals separately which should eliminate application interoperability issues.  

Within the X server, the only way I can see to easily effect this change is to
"hide" visuals placed in the connection setup block which is delivered to each
client.  This isolates the change to a few lines of code, instead of
distributing the change throughout the X server.

(In reply to comment #2)
> Within the X server, the only way I can see to easily effect this change is to
> "hide" visuals placed in the connection setup block which is delivered to each
> client.  This isolates the change to a few lines of code, instead of
> distributing the change throughout the X server.

Either that or maintain a completely separate list of "extended" visuals for
Composite.  The benefit of the latter approach is that you might not need to
alter the visual structure (i.e., you could keep alphaMask and offsetAlpha in a
separate "extended" visual structure) and would thereby avoid any potential ABI
issues surrounding the visual structure change (assuming this is what is causing
the ABI problems).  The downside to this approach is that it is a design change
and would require reworking the code quite a bit.

There are several places in the code which "walk" the list of visuals per-screen
to map visual ids to visuals.  A separate list will invalidate all of that code,
hence my suggestion that we simply "hide" the new visuals from clients in the
connection setup and otherwise leave things alone.

As noted in email, the additions to the visual structure were an attempt to get
'naïve' clients working with an ARGB visual by having AllocColor (et al) return
pixel values with alpha components pre-set.  That turned out to be a lot less
useful than I was expecting because essentially no applications bother to ask
the server to compute pixel values for TrueColor visuals, so clients have to be
fixed in any case.  We should consider simply removing those additional fields
from the Visual structure and leaving the advertisement of the extended visuals
to the Composite extension which can simply use a PictureFormat instead of a
Visual to transmit the intra-pixel layout to clients.

Just adding a note from this morning's release wranglers call...

Keith said he would disable the extra visuals in the connection block today so
that the testing can proceed (if that does not cause other problems with
Composite) and then will extend composite extension to expose the extra visuals
later this week.

Ok, I implemented the complete proposed solution -- hiding the visuals in the
connection block and adding the visuals through the Composite extension.

However, this breaks the Render extension, and any other subsystem which expects
to find all of the available visuals in the (exposed) Screen/Depth/Visual
structures which are part of the public Xlib API.

I propose that we go back to the old solution where these new visuals *are*
exposed in the core protocol and that we do provide the Xlib environment
variable hack to hide them for certain applications.  I will reduce the set of
new visuals to just the depth-32 ARGB visual so that applications don't
accidentally stumble upon the depth 24 RGB visual which just runs slowly.

Given that Composite will be disabled by default, I think this will provide
sufficient transition notice for any broken applications, and provide people who
do enable the extension to work around any troubles they do find in the meantime.

Here's a detailed analysis of why Flash breaks with the ARGB visual present.

Around 10 o'clock on Aug 13, Alan Coopersmith wrote:

> Can someone send an explanation between now and Monday of what the
> bug in Flash is and what they need to do with the way they handle
> visuals to fix it?   (Perhaps even a simple test case - is it just
> build the CVS head, turn on composite and try to run Flash?)

The test case is simple -- run mozilla against an X server with the ARGB 
visual to a site using flash ( http://www.kimpossible.com is my daughter's 
favorite at the moment)

The sympotom is an attempt by Flash to use PutImage with a depth 32 image
to a 16 bit window.

Here's a bit of an xscope trace:

         ............REQUEST: PutImage
             sequence number: 0000009e
                      format: ZPixmap
              request length: ff56
                    drawable: DWB 02800001
                          gc: GXC 0280001a
                            function: Copy
                          plane-mask: ffffffff
                          foreground: 00000000
                          background: 00000001
                       width: 02f8
                      height: 0056
                       dst-x: 0
                       dst-y: 0
                    left-pad: 00
                       depth: 20
         ............REQUEST: GetInputFocus
             sequence number: 0000009f
              request length: 0001
388.21:                                           64 bytes <-- X11 Server 7
                                         ..............ERROR: Match
                                             sequence number: 009e
                                                minor opcode: 0000
                                                major opcode: 48

And here's a stack trace from mozilla:

#41 0x404ffd11 in gdk_x_error (display=0x0, error=0x40965edc)
    at gdkmain-x11.c:530
#42 0x40692d64 in _XError (dpy=0x8838cf0, rep=0xbfffb2b0) at XlibInt.c:2909
#43 0x406912e3 in _XReply (dpy=0x8838cf0, rep=0xbfffb2b0, extra=0, discard=1)
    at XlibInt.c:1839
#44 0x4068c3b4 in XSync (dpy=0x8838cf0, discard=0) at Sync.c:45
#45 0x4068c475 in _XSyncFunction (dpy=0x0) at Synchro.c:34
#46 0x406853a5 in XPutImage (dpy=0x8838cf0, d=41943041, gc=0x868b6e8,
    image=0x868b760, req_xoffset=0, req_yoffset=0, x=0, y=0, req_width=760,
    req_height=86) at PutImage.c:1037
#47 0x4614f085 in PlatformBitBuffer::BltToScreen ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#48 0x4611eabd in CorePlayer::DrawScreen ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#49 0x461973cb in PlatformPlayer::NsSetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#50 0x4619b34e in NPP_SetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#51 0x46199580 in Private_SetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#52 0x42149127 in ns4xPluginInstance::InitializePlugin ()
   from /usr/lib/mozilla-firefox/components/libgkplugin.so

And here's where that image is created:

#0  XCreateImage (dpy=0x8850658, visual=0x86a7868, depth=32, format=2,
    offset=0, data=0x1f <Address 0x1f out of bounds>, width=728, height=31,
    xpad=32, image_bytes_per_line=0) at ImUtil.c:328
#1  0x44f8090c in PlatformBitBuffer::CreateXImage ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#2  0x44f80d2d in PlatformBitBuffer::CreateScreenBits ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#3  0x44f80622 in PlatformBitBuffer::PlatformBitBuffer ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#4  0x44f5134e in CorePlayer::UpdateBuffer ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#5  0x44f50a43 in CorePlayer::DrawScreen ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#6  0x44fc93cb in PlatformPlayer::NsSetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#7  0x44fcd34e in NPP_SetWindow ()

The specified visual is the ARGB visual and was selected by the flash 
player which appears to have a fixed list of formats to try and doesn't 
appear to know which visual is associated with the output window.

Here's where the visual was selected:

#0  XMatchVisualInfo (dpy=0x8873268, screen=0, depth=32, class=4,
    visual_info=0xbfffb430) at VisUtil.c:202
#1  0x4633aa80 in UnixCommonPlayer::ChoosePixmapFormat ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#2  0x4637d373 in PlatformPlayer::NsSetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#3  0x4638134e in NPP_SetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#4  0x4637f580 in Private_SetWindow ()
   from /usr/lib/flashplugin-nonfree/libflashplayer.so
#5  0x42149127 in ns4xPluginInstance::InitializePlugin ()
   from /usr/lib/mozilla-firefox/components/libgkplugin.so

As we can see, it's looking for a depth 32 TrueColor visual, which does 
not match the visual for the window into which this image will be put.

I suspect what they should be doing is using GetWindowAttributes to 
discover the visual for their window and using that instead of attempting 
to get at it via this round-about route.

Keith committed the libX11 workaround, which needs to be documented for the
release notes.

this was documented in the 6.8 release notes:

http://xorg.freedesktop.org/X11R6.8.0/doc/RELNOTES5.html