Closed
Bug 1171682
Opened 9 years ago
Closed 9 years ago
Disable accelerated WebGL in Safe Mode
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: jrmuizel, Assigned: BenWa)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
1.28 KB,
patch
|
jgilbert
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
281 bytes,
patch
|
Details | Diff | Splinter Review |
Accelerated WebGL isn't safe
If it comes in handy - there is gfxPlatform::InSafeMode() method we can use.
layers.acceleration.disabled is the preference "Use hardware acceleration if available" checkbox. The code that checks that preference, also takes the same action if the safe mode is on. Accelerated WebGL checks neither.
Updated•9 years ago
|
Summary: Disable accelerated WebGL in Safe Mode → Disable accelerated WebGL in Safe Mode or when layers.acceleration.disabled is set
Updated•9 years ago
|
OS: Unspecified → All
Benoit, can you get us a quick patch for this? I'd like it uplifted to 39 and 40 and the timing is very tight.
Assignee: jmuizelaar → bgirard
Flags: needinfo?(bgirard)
Assignee | ||
Comment 4•9 years ago
|
||
So I tried the obvious thing. It doesn't work. Turns out it's because e10s content process don't currently get the safe mode state. So in this bug we will land a patch assuming that it works fine and uplift to 39 and 40. In a follow-up e10s bug we can look into how to handle safe mode checks for e10s content processes.
Flags: needinfo?(bgirard)
Assignee | ||
Comment 5•9 years ago
|
||
Ideally we want this to fallback to software webgl when that is ready: bug 1167651
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8622697 -
Flags: review?(jgilbert)
Updated•9 years ago
|
Attachment #8622697 -
Flags: review?(jgilbert) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Turning off WebGL in safe mode is pretty simple. What should we do when 'layers.acceleration.disabled' is set? If we have WebGL software maybe we want to fallback to that like the bug says. If we don't have software webgl then what should we do? Have no WebGL or stay on accelerated. Keep in mind that we have a lot of users that disable OGL because of certain driver/performance issue but can still run WebGL fine, or just because it's debugging response to a bug. If we do that we run the risk of lowering our WebGL usage. That may be ok, just want us to know that going in.
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Assignee | ||
Comment 9•9 years ago
|
||
[Tracking Requested - why for this release]: Tracking per Comment 3. This would be a low risk change and provide some slight stability benefit.
tracking-firefox39:
--- → ?
tracking-firefox40:
--- → ?
Assignee | ||
Comment 11•9 years ago
|
||
I feel like overloading layers.acceleration to also impact webgl is a bad idea. webgl.disabled already exists. Each feature has their own disable flag and it allows us to test combo that exist in the while easily like software layers and hardware WebGL. If this is something we really want, and I'm not sure it is, we should maybe introduce something like gfx.acceleration.disabled which would disable hardware acceleration for layers, webgl and could even disable things like hardware video decoders. TL;DR: I suggest we only disable webgl in safe mode and do nothing else.
Flags: needinfo?(jmuizelaar)
Assignee | ||
Comment 12•9 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #3) > Benoit, can you get us a quick patch for this? I'd like it uplifted to 39 > and 40 and the timing is very tight. Are you sure we want part 1 in 39/Beta? I don't see any user impact linked on this bug and we're been shipping this for years. On the other hand the patch is trivial so it's not a big deal to uplift.
Flags: needinfo?(milan)
Reporter | ||
Comment 13•9 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #11) > I feel like overloading layers.acceleration to also impact webgl is a bad > idea. webgl.disabled already exists. Each feature has their own disable flag > and it allows us to test combo that exist in the while easily like software > layers and hardware WebGL. > > If this is something we really want, and I'm not sure it is, we should maybe > introduce something like gfx.acceleration.disabled which would disable > hardware acceleration for layers, webgl and could even disable things like > hardware video decoders. > > TL;DR: I suggest we only disable webgl in safe mode and do nothing else. I agree.
Flags: needinfo?(jmuizelaar)
Yes to putting this on 39. Safemode needs to be safer. As for the other topic - we already have a problem - we have a prominent user interface item that says "Use hardware acceleration when available", and if checked off, we use hardware acceleration for WebGL. That doesn't feel like "don't use hardware acceleration". I will spin off another bug for this, as it may take more discussion.
Flags: needinfo?(milan)
I imagine we don't need "leave-open" anymore?
Summary: Disable accelerated WebGL in Safe Mode or when layers.acceleration.disabled is set → Disable accelerated WebGL in Safe Mode
Assignee | ||
Comment 16•9 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #14) > I will spin off another bug for this, as it may take more discussion. Please, lets decouple these decision. The safemode bit is a no brainer.
Keywords: leave-open
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8622697 [details] [diff] [review] part 1: Trival case + uplift candidate Approval Request Comment [Feature/regressing bug #]: This bug, disabling a feature in safe mode. [User impact if declined]: Higher risk in safe mode [Describe test coverage new/current, TreeHerder]: We'd need to add a 'safe' variant to our test matrix. I don't think this is realistic. [Risks and why]: Fairly low, we already support disabling safe mode. The patch is very simple. [String/UUID change made/needed]: none
Attachment #8622697 -
Flags: approval-mozilla-beta?
Attachment #8622697 -
Flags: approval-mozilla-aurora?
Adding a tracking flag for Firefox40. We should take this fix as it is making Firefox safer when running in safe mode. Milan, I was reading some MDN documentation on WebGL and there was a mention of webgl.disable_extensions (A Boolean property that, when true, disables all WebGL extensions. This is false by default). I am just wondering, if WebGL extensions also need to be disabled separately. Thoughts?
Assignee | ||
Comment 19•9 years ago
|
||
For safe mode? This patch should be sufficient.
Benoit, yes I did mean in safe mode.
Updated•9 years ago
|
Flags: needinfo?(milan)
Comment 21•9 years ago
|
||
Comment on attachment 8622697 [details] [diff] [review] part 1: Trival case + uplift candidate Approved for upplift to beta and aurora.
Attachment #8622697 -
Flags: approval-mozilla-beta?
Attachment #8622697 -
Flags: approval-mozilla-beta+
Attachment #8622697 -
Flags: approval-mozilla-aurora?
Attachment #8622697 -
Flags: approval-mozilla-aurora+
Updated•9 years ago
|
status-firefox39:
--- → affected
Assignee | ||
Comment 22•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/f940d6d04ccd
Updated•9 years ago
|
status-firefox41:
--- → affected
Assignee | ||
Comment 23•9 years ago
|
||
Backout: https://hg.mozilla.org/releases/mozilla-beta/rev/f069ca975718 I didn't notice that gfxPlatform::InSafeMode() was missing from the beta branch. I'll do a try run
Assignee | ||
Comment 25•9 years ago
|
||
Waiting on try to open
Assignee | ||
Comment 26•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b654c86e54c
Assignee | ||
Comment 27•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f25650d08aa
Comment 29•9 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #10) > https://hg.mozilla.org/mozilla-central/rev/f4367d79d062 Let's move any follow-up work to a new bug to avoid a tracking nightmare.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•