Don't use the ESSL backend of ANGLE

RESOLVED FIXED in Firefox 9

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

({verified-beta})

unspecified
mozilla11
verified-beta
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox9+ fixed, firefox10+ fixed)

Details

Attachments

(2 attachments)

Created attachment 581036 [details] [diff] [review]
don't use ESSL backend

We don't need a ESSL backend in ANGLE, since we don't currently perform shader transformations on platforms that use OpenGL ES.

So we used to just happily use the GLSL backend everywhere, and actually that used to be all what there was, but a few months ago I saw that new ESSL backend and I though: oh, shiny! And I proudly wrote that line of code:

           gl->IsGLES2() ? SH_ESSL_OUTPUT : SH_GLSL_OUTPUT,

Now it turns out that we've had terrible WebGL crashiness on Android lately (see e.g. bug 684863 and bug 709712) and revertion to just

           SH_GLSL_OUTPUT,

fixes all of these crashes.

Again, it doesn't matter that we're getting GLSL output, on ES we just discard it and use the original source (stripped from comments) to feed glShaderSource. So no functional behavior change will happen as a result from this change. This is true as long as we don't use any shader transformation on ES. (We currently only do on Mac, which uses desktop GL).
Attachment #581036 - Flags: review?(jgilbert)
Attachment #581036 - Flags: review?(jgilbert) → review+
(Assignee)

Comment 1

5 years ago
Created attachment 581075 [details] [diff] [review]
don't use ESSL backend, for landing on mozilla-beta
Attachment #581075 - Flags: review+
Attachment #581075 - Flags: approval-mozilla-beta?

Comment 2

5 years ago
Comment on attachment 581075 [details] [diff] [review]
don't use ESSL backend, for landing on mozilla-beta

[Triage Comment]
Approving for beta given the WebGL strategic landscape on mobile, and the fact that this is now even lower risk (Android specific).
Attachment #581075 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

5 years ago
tracking-firefox10: --- → +
tracking-firefox9: --- → +
(Assignee)

Comment 3

5 years ago
http://hg.mozilla.org/releases/mozilla-beta/rev/d2f1a400e430

keeping open for central and aurora
I verified this on Firefox9 Beta6 and didn't see any crashes.
Mozilla /5.0 (Android;Linux armv7l;rv:9.0) Gecko/20111212 Firefox/9.0 Fennec/9.0 
Device: HTC Desire (Android 2.2)
Keywords: verified-beta

Updated

5 years ago
status-firefox9: --- → fixed
(Assignee)

Comment 5

5 years ago
Comment on attachment 581075 [details] [diff] [review]
don't use ESSL backend, for landing on mozilla-beta

OK, there only remains to land this on Aurora. Please!
Attachment #581075 - Flags: approval-mozilla-aurora?
I accidentally left myself as the committer of record when landing this in mozilla-central. I understand that this will go on my permanent record.

http://hg.mozilla.org/mozilla-central/rev/dc12262285a4
Assignee: nobody → bjacob
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla9

Comment 7

5 years ago
We need this on aurora as well.
status-firefox10: --- → affected
Target Milestone: mozilla9 → mozilla11
(Assignee)

Comment 8

5 years ago
That's why there is a approval? flag on this patch. Please grant!

Comment 9

5 years ago
Comment on attachment 581075 [details] [diff] [review]
don't use ESSL backend, for landing on mozilla-beta

Whoops, must have missed it.
Attachment #581075 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/61ff68ce08f4

Updated

5 years ago
status-firefox10: affected → fixed
You need to log in before you can comment on or make changes to this bug.