780059 - layers.acceleration.force-enabled on Linux breaks Nightly of August 2 2012

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Alon Zakai (:azakai)]

I have been using layers.acceleration.force-enabled on all my linux machines, it seemed to do no harm and even speed things up. Today's nightly, however

1. Crashes on startup on my home laptop,

###!!! ABORT: X_GLXVendorPrivate: GLXBadPixmap; 1203 requests ago: file /builds/slave/m-cen-lnx-ntly/build/toolkit/xre/nsX11ErrorHandler.cpp, line 157
###!!! ABORT: X_GLXVendorPrivate: GLXBadPixmap; 1203 requests ago: file /builds/slave/m-cen-lnx-ntly/build/toolkit/xre/nsX11ErrorHandler.cpp, line 157

   This is a machine with NVidia and NVidia binary drivers. Crash is 100% consistent.

2. On my work laptop, Nightly shows only black - no window contents, no ui, etc. Only the menu appears (and indicates everything works except for being all black - can move between tabs etc. and the title in the menu changes). This is a machine with ATI and ATI binary drivers.

3. Fabrice told me he had similar issues with today's Nightly, also fixed by disabling accelerated layers.

Of course this is my fault for force-enabling accelerated layers ;) No argument there.

To return to a working state, I had to use another browser to download another firefox version, run that and switch the pref off in the same profile - so it's possible to recover from this without help. But I only knew to do that by guessing that accelerated layers might be related, which was a lucky guess.

As I said, it's my fault for flipping the pref. But there are probably others like me. Perhaps before we go to Aurora, we should do something to help people that hit this problem and prevent their browsers from becoming unusable?

P.S. Out of curiosity, what change landed in this Nightly that causes this?

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Can you try setting the xrender preference to false?

Comment 2

[Alon Zakai (:azakai)]

Switching acceleration to force-enabled breaks the browser like in comment #0. With that still on, flipping xrender to false unbreaks it.

cc'ing matt who bjacob said did work that might be related to this.

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

(In reply to Alon Zakai (:azakai) from comment #2)
> With that still on, flipping xrender to false unbreaks it.

Alright. So to avoid wasting more of peoples' time, we should have the xrender=false setting be implied by layers.acceleration.force-enabled.

Comment 4

[Karl Tomlinson (:karlt)]

Issue 1. in comment 0 would be caused by bug 779786, which is a regression from bug 707722.

Sounds like issue 2 is something different, but still a regression from bug 707722.

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: ATI glxinfo

There are no visuals with zero bits of alpha in the color buffer.
I guess we should permit alpha bits in fbconfigs for opaque surfaces if the visual matches the format exactly.

Comment 6

[Karl Tomlinson (:karlt)]

There's a try build here.  Please check this resolves the issue, if you are able.

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-71605c209834/try-linux/

Comment 7

[Alon Zakai (:azakai)]

With that build things work ok, on the same setup that breaks nightly.

Looks like layers acceleration is flipped off when I run it, I guess that is the mechanism?

Comment 8

[Karl Tomlinson (:karlt)]

azakai and i talked on irc and worked out that layers.acceleration.force-enabled was set to true with things working again.  (The pref was just not highlighted in about:config because that build had the pref defaulting to true.)

Comment 9

[Karl Tomlinson (:karlt)]

Attachment: tidy up API for FindVisualAndDepth

I originally wrote this function for ipc plugins and then moved it to share
with shadow layers.  I should have sorted out the API at the time because I'd
like to reuse it here.

Xlib always uses int for depths.  Having unsigned in the API leads to having to work around signed/unsigned comparison warnings.  The plugin code can have a workaround for its quirk.

"Info" in the name was bad because it suggests XVisualInfo, which this is
not.

The return code was not really necessary and just makes using the function more work.  Visual IDs don't change, so there's no reason to expect an invalid ID.

Comment 10

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 651892 [details] [diff] [review]
tidy up API for FindVisualAndDepth

Review of attachment 651892 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/ipc/PluginInstanceChild.cpp
@@ +1093,5 @@
>      mWsInfo.colormap = aWindow.colormap;
> +    int depth;
> +    FindVisualAndDepth(mWsInfo.display, aWindow.visualID,
> +                       &mWsInfo.visual, &depth);
> +    mWsInfo.depth = depth;

Do we need to check if the visual returned here was null?

Comment 11

[Karl Tomlinson (:karlt)]

Even the old code did set a null visual, when passed None for visual id so the old function returned true.  I remember that happening in practice.  IIRC the window was also None, and that was happening in an early SetWindow call and was followed by another SetWindow before any paint.

In a later patch I'm making the assumption that visual ids are valid.

Comment 12

[Karl Tomlinson (:karlt)]

Attachment: accept fbconfigs with alpha bits for opaque Pixmaps if visual matches

Most reasoning is explained in comments.  I've preferred to get the Visual id
rather than XVisualInfo from the config, so no memory allocation is required on
each iteration.  Also added a missing break to the loop so we get the most
preferred visual rather than the least preferred.

Comment 13

[Karl Tomlinson (:karlt)]

Attachment: combine two variables both indicating an alpha channel

Some tidying.

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: remove use of glXGetVisualFromFBConfig and glXCreateContext

Related cleanup.

This patch removes an unused XVisualInfo method parameter from
CreateGLContext, removes the unused glxCreateContext code, and removes
glXGetVisualFromFBConfig, preferring to get the visual id.

AFAICT the XVisualInfo structure is only really needed for glXCreateContext and
glXGetConfig, which we don't use now we have fbconfigs.  XVisualInfo does
provide info about the screen; we could get that from the Xlib Visual if we
wanted, but we don't use more than one screen.

Comment 15

[dE]

This problem is not reproducible with the latest nightly with Intel HD 2000.

Comment 16

[Kami]

I can not reproduce with:
firefox-trunk-17.0~a1~hg20120813r102211
HW:
NVIDIA Corporation -- GeForce GTX 460/PCIe/SSE2 -- 4.2.0 NVIDIA 304.32

Comment 17

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aba8e8ce7c7b
https://hg.mozilla.org/integration/mozilla-inbound/rev/953a366b4038
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ed060f2734a
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8e7d6319d50

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/aba8e8ce7c7b
https://hg.mozilla.org/mozilla-central/rev/953a366b4038
https://hg.mozilla.org/mozilla-central/rev/2ed060f2734a
https://hg.mozilla.org/mozilla-central/rev/f8e7d6319d50