610852 - colors swapped when running Firefox under VNC

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Something regressed in Firefox builds about two weeks ago such that running Firefox under VNC results in swapped colors (red and blue swapped, I think) in everything (Web pages, UI, etc.).

The regression happened between Linux x86-64 nightlies 2010-10-27-03-mozilla-central and 2010-10-28-03-mozilla-central, which corresponds to:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2f456d0310fa&tochange=fe4898e97431

Steps to reproduce:
 * start a VNC server (e.g., vncserver -depth 24 -geometry 1400x1050 :11) on a Ubuntu 10.10 machine
 * VNC to it
 * run Firefox in that VNC session

I haven't checked yet that things are ok on the same machine when not VNC'd; I probably should check that.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Appears to be a regression from 
http://hg.mozilla.org/mozilla-central/rev/8fc4f0abe03f

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #1)
> Appears to be a regression from 
> http://hg.mozilla.org/mozilla-central/rev/8fc4f0abe03f

Note that I confirmed this by locally reverting this changeset; doing so makes the bug go away.

Comment 3

[Joe Drew (not getting mail)]

I don't think this hard-blocks unless we're also wrong on that machine without VNC (dbaron, ping), but Oleg, I'd really love to have a fix to this.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #3)
> I don't think this hard-blocks unless we're also wrong on that machine without
> VNC (dbaron, ping), but Oleg, I'd really love to have a fix to this.

I tried, but real X wasn't even starting up (hooray for upgrading to Ubuntu 10.10 before it was ready).

Comment 5

[Ginn Chen]

Attachment: patch

I guess this patch will fix the problem.
Tested on a SPARC machine with xBGR frame buffer, but I didn't test it with VNC server.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I don't think I'm an appropriate reviewer for this code.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This patch does fix the problem I was seeing, though.

Comment 8

[Joe Drew (not getting mail)]

Comment on attachment 491798 [details] [diff] [review]
patch

This looks a little hardcode-y; Karl, what do you think?

Comment 9

[Oleg Romashin (:romaxa)]

btw, I'm not original author of that code, I just moved it from Gtk to be common for Qt and GTK.

Comment 10

[Karl Tomlinson (:karlt)]

Why did doing moving code cause a change in behavior in GTK builds?
Why is this code getting used in Firefox builds?

Comment 11

[Oleg Romashin (:romaxa)]

that is very interesting question, beause that code should be enabled only for Mobile (switching rendering pipeline to image surface)
basically when
return gfxPlatform::GetPlatform()->
    ScreenReferenceSurface()->GetType() == gfxASurface::SurfaceTypeImage

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 491798 [details] [diff] [review]
patch

This check is correct, so r=me with the warning changed to "Unsupported XShm Image format".  (The layout of ImageFormatRGB24 is predefined and so hard-coded values are needed in some form.)

The check would be better performed on aVisual and aDepth, before doing the whole Shm dance, but that could already be said of existing code.
Ideally we'd add a function that does the inverse of gfxXlibSurface::FindVisual.

It is appropriate to add this check, though the real regression from bug 606910 is still not understood.

Comment 13

[Karl Tomlinson (:karlt)]

(In reply to comment #11)
> return gfxPlatform::GetPlatform()->
>     ScreenReferenceSurface()->GetType() == gfxASurface::SurfaceTypeImage

The old test was UseClientSideRendering() which was a compile-time switch.
ScreenReferenceSurface which comes from CreateOffscreenSurface.  It will return an image surface if the platform does not have RENDER.  Client side rendering would be appropriate if RENDER is not available.

David and Ginn: is RENDER in the list of extensions from xdpyinfo in the environments where you see this bug?

Comment 14

[Ginn Chen]

(In reply to comment #13)
> (In reply to comment #11)
> > return gfxPlatform::GetPlatform()->
> >     ScreenReferenceSurface()->GetType() == gfxASurface::SurfaceTypeImage
> 
> The old test was UseClientSideRendering() which was a compile-time switch.
> ScreenReferenceSurface which comes from CreateOffscreenSurface.  It will return
> an image surface if the platform does not have RENDER.  Client side rendering
> would be appropriate if RENDER is not available.
> 

That's what I've observed.
It returns SurfaceTypeXlib if RENDER is available, returns SurfaceTypeImage if RENDER is not available.

> David and Ginn: is RENDER in the list of extensions from xdpyinfo in the
> environments where you see this bug?

On my SPARC machine with Xsun. RENDER is not available.

Comment 15

[Ginn Chen]

http://hg.mozilla.org/mozilla-central/rev/9fcdd4f43581