1088858 - Backport ANGLE fixes to make WebGL work on Windows in Firefox 33, 34

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

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

We've had a couple of severe WebGL regressions on Windows in Firefox 33/34.

See:
bug 1085203 - affects Google Maps, makes street labels invisible
bug 1083178 - affects Cesium, makes some Cesium shaders fail to compile

At least the Cesium  bug 1083178 is known for sure to be caused by a ANGLE bug that was fixed in upstream ANGLE in early July, for which we never backported the patch. That's the object of the current bug.

The ANGLE bug was imported into our tree in the 33 train in early July, in bug 1010371, unfortunately just because the bug was fixed upstream.

The upstream fix cset is:

commit 787fc03f15a0c4440d023659f267175fa3979808
Author: Jamie Madill <[e-mail]>
Date:   Mon Jul 7 12:49:45 2014 -0400

    Fix HLSL compiler error with else-rewriting in functions.

    In functions with return types where we would use if-else rewriting,
    we would potentially generate a spurious HLSL error that warned of
    branches with no return value in the function.

    This was causing a maps regression where overlays would not draw
    in Earth mode.

    BUG=346463
    BUG=391697

    Change-Id: I9f4fa959057a3a2dab6cdd98f8381b5871cabf03
    Reviewed-on: https://chromium-review.googlesource.com/206824
    Tested-by: Jamie Madill <[e-mail]>
    Reviewed-by: Nicolas Capens <[e-mail]>
    Reviewed-by: Shannon Woods <[e-mail]>

We also need to backport this other cset which is a prereq for it to build:

commit 4cfb1e890ac39201145316ad8f3196d456283389
Author: Jamie Madill <[e-mail]>
Date:   Mon Jul 7 12:49:23 2014 -0400

    Add a new TIntermRaw node type to translator.

    This raw node stores text strings that we directly copy to the
    output. This allows for more tricky substitutions that don't fit
    in to the HLSL/GLSL shared parsing model.

    BUG=346463
    BUG=391697

    Change-Id: Ibbde6db4fc98ef6d892f219631ca1a258a902a86
    Reviewed-on: https://chromium-review.googlesource.com/206823
    Tested-by: Jamie Madill <[e-mail]>
    Reviewed-by: Nicolas Capens <[e-mail]>
    Reviewed-by: Shannon Woods <[e-mail]>


We don't know yet for sure that this will also fix the Google Maps bug 1085203  but we're hoping.

Comment 1

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

Reason to hope for Google Maps is the above-pasted commit message saying "This was causing a maps regression where overlays would not draw in Earth mode."

Comment 2

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

Attachment: ANGLE fixes for Firefox 33 (two ANGLE changesets)

Comment 3

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

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8e75c056deec

Comment 4

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

Attachment: ANGLE fixes for Firefox 33 (two ANGLE changesets) (r=jrmuizel)

Accidentally committed a debugging change as well.

Comment 5

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

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f6325d97745d

Comment 6

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

Comment on attachment 8511291 [details] [diff] [review]
ANGLE fixes for Firefox 33 (two ANGLE changesets) (r=jrmuizel)

Approval Request Comment
[Feature/regressing bug #]: bug 1010371 - ANGLE update in Firefox 33
[User impact if declined]: Major WebGL breakage on Windows. Google Maps is affected, supposed to be fixed by this ANGLE cset, not confirmed to be fixed. Cesium is affected, and confirmed to be fixed by this ANGLE cset. The nature of the bug suggests that a lot of content could be affected (any shader with nontrivial if...else... control flow)
[Describe test coverage new/current, TBPL]: None I guess for that particular bug, since we let that slip through. On the other hand, we have plenty of other WebGL tests, so we can hope that any regression would have a high probability to be caught.
[Risks and why]: Medium risk? Medium sized patch in code that we don't really understand, but it was reviewed upstream and we've been using that code in a newer version of ANGLE in Gecko 35+.
[String/UUID change made/needed]: none

Comment 7

[Jeff Muizelaar [:jrmuizel]]

(In reply to Benoit Jacob [:bjacob] from comment #6)
> Comment on attachment 8511291 [details] [diff] [review]
> ANGLE fixes for Firefox 33 (two ANGLE changesets) (r=jrmuizel)
> 
> Approval Request Comment
> [Feature/regressing bug #]: bug 1010371 - ANGLE update in Firefox 33
> [User impact if declined]: Major WebGL breakage on Windows. Google Maps is
> affected, supposed to be fixed by this ANGLE cset, not confirmed to be
> fixed. Cesium is affected, and confirmed to be fixed by this ANGLE cset.

We have no confirmed that this doesn't fix the Google Maps problem.

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8511291 [details] [diff] [review]
ANGLE fixes for Firefox 33 (two ANGLE changesets) (r=jrmuizel)

Let's ship this change in Beta4 and test on the Beta channel before considering this for release. 

bjacob - Given that you have flagged this as medium risk, please be prepared to respond if this causes significant breakage on Beta.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/85e56f19a5a1

Comment 10

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

OK :)

Comment 11

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Marking this for verification in Firefox 34 Beta 4 so we can check bug 1085203 and perform some testing for WebGL.

Comment 12

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Testing was performed with the latest Firefox 34 Beta 4 build (BuildID=20141027152126) on the following environments:
a) Windows 8.1 x64 (Intel HD 2500) - WebGL exploratory testing
b) Windows 8.1 x86 (ATI Radeon 3000) - verification of dependencies (bug 1085203, bug 1083178) + WebGL exploratory testing
c) Windows 7 x64 (NVIDIA GeForce 210) - verification of dependencies (bug 1085203, bug 1083178) + WebGL exploratory testing
d) Windows 7 x64 (NVIDIA GeForce GT 610) - verification of dependencies (bug 1085203, bug 1083178)

Test results:
1. Exploratory testing - issues seen: bug 1087932, bug 1090338. Both issues appeared in Firefox 33, and are specific only to some hardware configurations.
2. Bug 1083178 - no longer reproduces in Firefox 34 Beta 4.
3. Bug 1085203 - still reproduces in Firefox 34 Beta 4.

Bug 1085203 notes:
- some configurations (a, b, and c) are affected by bug 1090338, so the bug cannot properly be verified
- configuration d is not affected by bug 1090338, but bug 1085203 still reproduces
- with Hardware Acceleration disabled, bug 1085203 does NOT reproduce on configuration d, but still reproduces on configuration b

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/d49ad0a834a8

Comment 14

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Since bug 1085203 has now been verified, and remaining issues issues are tracked separately, I'm also marking this as Verified based on comment 12.

Comment 15

[Luke]

Yes, Google Maps is working now, but for Windows users with ATI hardware that's about it. On both my XP desktop and my Win 7 laptop nearly every WebGL app is still completely broken on FF >= 33. See:

Bug 1093037
Bug 1084239
Bug 1122465
Bug 1106582
Bug 1109708
Bug 1089140
Bug 1109708
Bug 1086704