Closed Bug 1268638 (whitelist-wgl) Opened 9 years ago Closed 8 years ago

Whitelist and prefer modern OpenGL drivers on Windows

Categories

(Core :: Graphics: CanvasWebGL, defect)

Product:
Core
Component:
Graphics: CanvasWebGL
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla50
Tracking Flags:
Tracking Status
firefox50 --- fixed

People

(Reporter: jgilbert, Assigned: jgilbert)

References

(Blocks 1 open bug)

Details

(Whiteboard: webgl-perf, gfx-noted)

Attachments

(2 files, 6 obsolete files)

Bug 1268638 - Bitrot.
58 bytes, text/x-review-board-request
jrmuizel
: review+
Details
Bug 1268638 - Redo backend selection dance for WebGL. -
58 bytes, text/x-review-board-request
jrmuizel
: review+
Details
We should investigate this as an alternative to using ANGLE.
Alias: whitelist-wgl
Whiteboard: webgl-perf → webgl-perf, gfx-noted
Attached file Bug 1268638 - Bitrot. —
From 9f464312dbf5e8b8d6f529e4ff205494ede5c85a Mon Sep 17 00:00:00 2001 non-blacklisted native-gl. --- dom/canvas/WebGLContext.cpp | 125 ++++++++++++++++++++++++++------------------ dom/canvas/WebGLContext.h | 7 +-- 2 files changed, 77 insertions(+), 55 deletions(-) Review commit: https://reviewboard.mozilla.org/r/54128/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54128/
Attachment #8754650 - Flags: review?(jmuizelaar)
From 7b58613f3d035795934ae3a8fc58017a8e35a6b8 Mon Sep 17 00:00:00 2001 webgl+native-gl. --- widget/windows/GfxInfo.cpp | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) Review commit: https://reviewboard.mozilla.org/r/54130/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54130/
From b02080fc6310b60e572c2391aecaa8395eb0fc65 Mon Sep 17 00:00:00 2001 --- gfx/layers/ipc/ISurfaceAllocator.cpp | 8 ++++++++ gfx/layers/ipc/ISurfaceAllocator.h | 3 +-- 2 files changed, 9 insertions(+), 2 deletions(-) Review commit: https://reviewboard.mozilla.org/r/54132/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54132/
From 7f233590e15bb0fcce396b4b8aadc82f9dcd981f Mon Sep 17 00:00:00 2001 --- dom/canvas/WebGLContext.cpp | 12 ++++++++++-- gfx/thebes/gfxPrefs.h | 1 + modules/libpref/init/all.js | 1 + 3 files changed, 12 insertions(+), 2 deletions(-) Review commit: https://reviewboard.mozilla.org/r/54134/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54134/
To be safe, we should keep using ANGLE for webgl1. WebGL 2 is where we actually want this anyway. We also need to require DXGL interop as a prereq for using WGL.
Attachment #8754651 - Flags: review?(jmuizelaar)
Attachment #8754653 - Flags: review?(jmuizelaar)
Attachment #8754652 - Flags: review?(jmuizelaar)
From d869ed023e051969103bf2164a1a2b15816d9dbc Mon Sep 17 00:00:00 2001 This will only run if they receive a WGL context, and we have a blocklist entry for this. Since there isn't a reason right now to want WGL without DXGL, we should tie these together. --- modules/libpref/init/all.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Review commit: https://reviewboard.mozilla.org/r/54716/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54716/
Attachment #8754651 - Attachment description: MozReview Request: r?jrmuizel - Add good driver versions to blocklist for → MozReview Request: r?jrmuizel - Add good driver versions to blocklist for
Attachment #8754652 - Attachment description: MozReview Request: r?jrmuizel - Avoid using gfxPrefs in a widely-included header. → MozReview Request: r?jrmuizel - Avoid using gfxPrefs in a widely-included header.
Attachment #8754653 - Attachment description: MozReview Request: r?jrmuizel - Add webgl.disable-wgl. (default: false) → MozReview Request: r?jrmuizel - Add webgl.disable-wgl. (default: false)
Attachment #8755653 - Flags: review?(jmuizelaar)
Attachment #8755654 - Flags: review?(jmuizelaar)
From 6af03664e00aad95467a1a749d4318cc0d38b572 Mon Sep 17 00:00:00 2001 --- dom/canvas/WebGLContext.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) Review commit: https://reviewboard.mozilla.org/r/54718/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/54718/
Comment on attachment 8754650 [details] Bug 1268638 - Bitrot. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54128/diff/1-2/
Comment on attachment 8754651 [details] Bug 1268638 - Redo backend selection dance for WebGL. - Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54130/diff/1-2/
Comment on attachment 8754652 [details] MozReview Request: r?jrmuizel - Avoid using gfxPrefs in a widely-included header. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54132/diff/1-2/
Comment on attachment 8754653 [details] MozReview Request: r?jrmuizel - Add webgl.disable-wgl. (default: false) Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54134/diff/1-2/
From a7f202b906767dfd18d346ffb670dbd43d7d970b Mon Sep 17 00:00:00 2001 --- dom/canvas/WebGL2Context.cpp | 9 ++++----- dom/canvas/WebGLContext.cpp | 36 +++++++++++++++++++++++------------- dom/canvas/WebGLContext.h | 17 +++++++++++++---- dom/canvas/WebGLContextValidate.cpp | 24 ++++++++++-------------- 4 files changed, 50 insertions(+), 36 deletions(-) Review commit: https://reviewboard.mozilla.org/r/55584/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55584/
Attachment #8757062 - Flags: review?(jmuizelaar)
Attachment #8757063 - Flags: review?(jmuizelaar)
From f8c8ab1c5c86621facbb57a471abd58b5003ba79 Mon Sep 17 00:00:00 2001 --- dom/canvas/WebGLContext.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) Review commit: https://reviewboard.mozilla.org/r/55586/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/55586/
Comment on attachment 8754650 [details] Bug 1268638 - Bitrot. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54128/diff/2-3/
Comment on attachment 8754651 [details] Bug 1268638 - Redo backend selection dance for WebGL. - Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54130/diff/2-3/
Comment on attachment 8754652 [details] MozReview Request: r?jrmuizel - Avoid using gfxPrefs in a widely-included header. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54132/diff/2-3/
Comment on attachment 8754653 [details] MozReview Request: r?jrmuizel - Add webgl.disable-wgl. (default: false) Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54134/diff/2-3/
Comment on attachment 8755653 [details] MozReview Request: r?jrmuizel - Enable DXGL by default. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54716/diff/1-2/
Comment on attachment 8755654 [details] MozReview Request: r?jrmuizel - WGL without DXGLInterop2 is perf caveat. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54718/diff/1-2/
Comment on attachment 8757062 [details] MozReview Request: r?jrmuizel - Use FailureReason struct. https://reviewboard.mozilla.org/r/55584/#review53198
Attachment #8757062 - Flags: review?(jmuizelaar) → review+
Assignee: nobody → jgilbert
Comment on attachment 8754651 [details] Bug 1268638 - Redo backend selection dance for WebGL. - https://reviewboard.mozilla.org/r/54130/#review55160 It looks like this patch should come before the previous one.
Attachment #8754651 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8754652 [details] MozReview Request: r?jrmuizel - Avoid using gfxPrefs in a widely-included header. https://reviewboard.mozilla.org/r/54132/#review55162
Attachment #8754652 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8755653 [details] MozReview Request: r?jrmuizel - Enable DXGL by default. https://reviewboard.mozilla.org/r/54716/#review55164 This should come last in the series and not in the middle.
Attachment #8755653 - Flags: review?(jmuizelaar) → review+
https://reviewboard.mozilla.org/r/54718/#review55166 What's the justification for allowing WGL without DXGLInterop2? It seems to me like ANGLE would be a better option in that case.
Attachment #8754653 - Flags: review?(jmuizelaar) → review-
Comment on attachment 8754653 [details] MozReview Request: r?jrmuizel - Add webgl.disable-wgl. (default: false) https://reviewboard.mozilla.org/r/54134/#review55168 Can you look at making this use gfxConfig instead of the pref directly?
https://reviewboard.mozilla.org/r/54128/#review55170 This seems to have a bunch of changes unrelated to the commit message.
Flags: needinfo?(jgilbert)
https://reviewboard.mozilla.org/r/54128/#review55170 It also adds support for multiple failure reasons.
(In reply to Jeff Muizelaar [:jrmuizel] from comment #24) > https://reviewboard.mozilla.org/r/54718/#review55166 > > What's the justification for allowing WGL without DXGLInterop2? It seems to > me like ANGLE would be a better option in that case. We're blocking ANGLE on WebGL 2 for now because it doesn't work yet, and WGL is disabled for WebGL 1, so these are currently mutually exclusive. We can do more sophisticated fallbacks as WebGL2+ANGLE comes online.
Flags: needinfo?(jgilbert) → needinfo?(jmuizelaar)
Blocks: webgl2-blockers
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5d1285d0c221 Add good driver versions to blocklist for webgl+native-gl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/8b0db9b0f87c Update WebGL creation to allow non-blacklisted native-gl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/002f87a61edf Avoid using gfxPrefs in a widfely-included header. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/69116551e4b9 Add webgl.disable-wgl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/8ed7ed6c57db Enable DXGL by default. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/0078605c6974 WGL without DXGLInterop2 is perf caveat. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/ad38e0439b62 Use FailureReason struct. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/1a2db6efc575 Disable native GL for webgl1, but allow in webgl2. - r=jrmuizel
Comment on attachment 8754650 [details] Bug 1268638 - Bitrot. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54128/diff/3-4/
Attachment #8754650 - Attachment description: MozReview Request: Bug 1268638 - r?jrmuizel - Update WebGL creation to allow → Bug 1268638 - Bitrot.
Attachment #8754651 - Attachment description: MozReview Request: r?jrmuizel - Add good driver versions to blocklist for → Bug 1268638 - Redo backend selection dance for WebGL. -
Comment on attachment 8754651 [details] Bug 1268638 - Redo backend selection dance for WebGL. - Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54130/diff/3-4/
Attachment #8754652 - Attachment is obsolete: true
Attachment #8754653 - Attachment is obsolete: true
Attachment #8755653 - Attachment is obsolete: true
Attachment #8755654 - Attachment is obsolete: true
Attachment #8755654 - Flags: review?(jmuizelaar)
Attachment #8757062 - Attachment is obsolete: true
Attachment #8757063 - Attachment is obsolete: true
Attachment #8757063 - Flags: review?(jmuizelaar)
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/34aa8f1f134f Add good driver versions to blocklist for webgl+native-gl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/8dd927ca1cd1 Update WebGL creation to allow non-blacklisted native-gl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/fdabb719c53a Avoid using gfxPrefs in a widfely-included header. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/c62305b23556 Add webgl.disable-wgl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/7fed3871832f Enable DXGL by default. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/419dea4c5e66 WGL without DXGLInterop2 is perf caveat. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/ca314c9d5249 Use FailureReason struct. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/55f389b590b7 Disable native GL for webgl1, but allow in webgl2. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/4171ff387666 Bitrot. https://hg.mozilla.org/integration/mozilla-inbound/rev/a89314380aea Redo backend selection dance for WebGL. - r=jrmuizel
I thought part of the rationale for using ANGLE was that we didn't trust drivers to deal with untrusted inputs because of the security risk. Is that not the case anymore? Is there something else that prevents untrusted input from Web content from going straight to the driver, or do we actually believe that these modern drivers are trustworthy from a security perspective? Or am I misunderstanding what these patches are doing?
(In reply to David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) from comment #35) > I thought part of the rationale for using ANGLE was that we didn't trust > drivers to deal with untrusted inputs because of the security risk. Is that > not the case anymore? Is there something else that prevents untrusted input > from Web content from going straight to the driver, or do we actually > believe that these modern drivers are trustworthy from a security > perspective? Or am I misunderstanding what these patches are doing? This is not the primary rationale for using ANGLE, and even so, we do all validation in Gecko, before passing validated calls off to the driver. Also these patches whitelist only very new drivers, and further, only WebGL 2 is using native GL for now. WebGL 1 still goes through ANGLE, and is likely to stay that way for most users.
Flags: needinfo?(jmuizelaar)
Flags: needinfo?(jgilbert)
Severity: enhancement → major
Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1071ec5ea9c9 Backed out changeset a89314380aea https://hg.mozilla.org/integration/mozilla-inbound/rev/06266813f509 Backed out changeset 4171ff387666 https://hg.mozilla.org/integration/mozilla-inbound/rev/316cb338b52b Backed out changeset 55f389b590b7 https://hg.mozilla.org/integration/mozilla-inbound/rev/68b5a49a3881 Backed out changeset ca314c9d5249 https://hg.mozilla.org/integration/mozilla-inbound/rev/c045de8b79b0 Backed out changeset 419dea4c5e66 https://hg.mozilla.org/integration/mozilla-inbound/rev/3a55d4cc067b Backed out changeset 7fed3871832f https://hg.mozilla.org/integration/mozilla-inbound/rev/62d2ea9a05ba Backed out changeset c62305b23556 https://hg.mozilla.org/integration/mozilla-inbound/rev/8fb0aa364772 Backed out changeset fdabb719c53a https://hg.mozilla.org/integration/mozilla-inbound/rev/1ff69fe79681 Backed out changeset 8dd927ca1cd1 https://hg.mozilla.org/integration/mozilla-inbound/rev/83d444698ab8 Backed out changeset 34aa8f1f134f for assertion failures in Assertion failure: !mLockedForGL, at SharedSurfaceD3D11Interop.cpp:302
Pushed by jgilbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7b0427fcc6aa Add good driver versions to blocklist for webgl+native-gl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/66dd1696e949 Update WebGL creation to allow non-blacklisted native-gl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/a683ecde58ba Avoid using gfxPrefs in a widfely-included header. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/a4545c904aec Add webgl.disable-wgl. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/2afbf4b9fee5 Enable DXGL by default. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/b99d14a16599 WGL without DXGLInterop2 is perf caveat. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/70f65e98cf98 Use FailureReason struct. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/2cefce6b93ff Disable native GL for webgl1, but allow in webgl2. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/4a4f2af4ddc9 Bitrot. https://hg.mozilla.org/integration/mozilla-inbound/rev/15224e6f9195 Redo backend selection dance for WebGL. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/2575bf65312d Empty functions defs can be in header. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/c9902da7753d Call ProducerRelease in ~GLScreenBuffer. - r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/f33f67f36e69 Cleanup better in failure cases. - r=jrmuizel
I'm assuming this patch is causing this to appear in about:support Graphics section: Decision Log ------------ OPENGL_COMPOSITING Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_OGL_ATI_DIS I am using the very latest AMD Crimson drivers. The OpenGL version in these drivers is 6.14.10.13440. Graphics -------- Features Compositing: Direct3D 11 Asynchronous Pan/Zoom: wheel input enabled; touch input enabled WebGL Renderer: Google Inc. -- ANGLE (AMD Radeon (TM) R9 390 Series Direct3D11 vs_5_0 ps_5_0) Hardware H264 Decoding: Yes; Using D3D11 API Direct2D: true DirectWrite: true (10.0.10586.0) GPU #1 Active: Yes Description: AMD Radeon (TM) R9 390 Series Vendor ID: 0x1002 Device ID: 0x67b1 Driver Version: 16.200.1035.0 Driver Date: 6-21-2016 Drivers: aticfx64 aticfx64 aticfx64 amdxc64 aticfx32 aticfx32 aticfx32 amdxc32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva atiumd6a atitmm64 Subsys ID: 00000000 RAM: 4095 Diagnostics AzureCanvasAccelerated: 0 AzureCanvasBackend: direct2d 1.1 AzureContentBackend: direct2d 1.1 AzureFallbackCanvasBackend: cairo Decision Log OPENGL_COMPOSITING: Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_OGL_ATI_DIS
(In reply to Gary [:streetwolf] from comment #40) > I'm assuming this patch is causing this to appear in about:support Graphics > section: > > Decision Log > ------------ > OPENGL_COMPOSITING Blocklisted; failure code > BLOCKLIST_FEATURE_FAILURE_OGL_ATI_DIS > > I am using the very latest AMD Crimson drivers. The OpenGL version in these > drivers is 6.14.10.13440. > > > Graphics > -------- > > Features > Compositing: Direct3D 11 > Asynchronous Pan/Zoom: wheel input enabled; touch input enabled > WebGL Renderer: Google Inc. -- ANGLE (AMD Radeon (TM) R9 390 Series > Direct3D11 vs_5_0 ps_5_0) > Hardware H264 Decoding: Yes; Using D3D11 API > Direct2D: true > DirectWrite: true (10.0.10586.0) > GPU #1 > Active: Yes > Description: AMD Radeon (TM) R9 390 Series > Vendor ID: 0x1002 > Device ID: 0x67b1 > Driver Version: 16.200.1035.0 > Driver Date: 6-21-2016 > Drivers: aticfx64 aticfx64 aticfx64 amdxc64 aticfx32 aticfx32 aticfx32 > amdxc32 atiumd64 atidxx64 atidxx64 atiumdag atidxx32 atidxx32 atiumdva > atiumd6a atitmm64 > Subsys ID: 00000000 > RAM: 4095 > > Diagnostics > AzureCanvasAccelerated: 0 > AzureCanvasBackend: direct2d 1.1 > AzureContentBackend: direct2d 1.1 > AzureFallbackCanvasBackend: cairo > > Decision Log > OPENGL_COMPOSITING: > Blocklisted; failure code BLOCKLIST_FEATURE_FAILURE_OGL_ATI_DIS This bug should be unrelated to this. This sounds related to bug 1002846, though that hasn't landed. I know there is motion around these bits right now though. bgirard may know more.
Flags: needinfo?(bgirard)
Thanks.
Best file another bug and CC me. Also include your full about:support (I'd like to see which preferences you have modified just in case). It would be handy to run a real bisection if you haven't done so already so we can tell what's querying the OGL_LAYERS blacklisting.
Flags: needinfo?(bgirard)
Blocks: 1285692
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: