Fix ANGLE D3D WARP for WebGL

VERIFIED FIXED in Firefox 47

Status

()

Core
Canvas: WebGL
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

(Blocks: 1 bug)

unspecified
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 verified, firefox48 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

(Assignee)

Description

2 years ago
The D3D WARP path in ANGLE isn't being hit anymore.
(Assignee)

Comment 1

2 years ago
Created attachment 8736530 [details]
MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel

Review commit: https://reviewboard.mozilla.org/r/43351/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43351/
Attachment #8736530 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

2 years ago
Created attachment 8736531 [details]
MozReview Request: WARP should never be broken. - r?jrmuizel

From f870a1ba53c21c878496ef703e1f7fa19b3f852f Mon Sep 17 00:00:00 2001
---
 gfx/gl/GLLibraryEGL.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Review commit: https://reviewboard.mozilla.org/r/43353/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43353/
(Assignee)

Comment 3

2 years ago
Created attachment 8736532 [details]
MozReview Request: Add mochitest to monitor backends. - r?jrmuizel

From fe95d5cc569ced06440d862816cd5d6b48842fbe Mon Sep 17 00:00:00 2001
---
 dom/canvas/test/webgl-mochitest.ini                |   1 +
 dom/canvas/test/webgl-mochitest/test_backends.html | 165 +++++++++++++++++++++
 2 files changed, 166 insertions(+)
 create mode 100644 dom/canvas/test/webgl-mochitest/test_backends.html

Review commit: https://reviewboard.mozilla.org/r/43355/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43355/
(Assignee)

Updated

2 years ago
Attachment #8736531 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
Attachment #8736532 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
See Also: → bug 1260574
Comment on attachment 8736531 [details]
MozReview Request: WARP should never be broken. - r?jrmuizel

https://reviewboard.mozilla.org/r/43353/#review39989
Attachment #8736531 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8736530 [details]
MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel

https://reviewboard.mozilla.org/r/43351/#review39987
Attachment #8736530 - Flags: review?(jmuizelaar) → review+
Attachment #8736532 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8736532 [details]
MozReview Request: Add mochitest to monitor backends. - r?jrmuizel

https://reviewboard.mozilla.org/r/43355/#review39991
Comment on attachment 8736531 [details]
MozReview Request: WARP should never be broken. - r?jrmuizel

This might actually not be safe. I believe we may have seen crashes creating WARP d3d11 devices with the intel drivers and DisplayLink.
Attachment #8736531 - Flags: review+ → review?(jmuizelaar)
(Assignee)

Comment 8

2 years ago
Created attachment 8737012 [details]
MozReview Request: Make force-warp a Live pref, not Once. - r?jrmuizel

From 3e1a05138b685d3a7f07eac2cc57d8f3d0b9588e Mon Sep 17 00:00:00 2001
---
 gfx/thebes/gfxPrefs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Review commit: https://reviewboard.mozilla.org/r/43643/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43643/
Attachment #8736530 - Attachment description: MozReview Request: Bug 1260944 - Fix ANGLE D3D WARP for WebGL. r?jrmuizel → MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel
Attachment #8736531 - Attachment description: MozReview Request: WARP should never be broken. → MozReview Request: WARP should never be broken. - r?jrmuizel
Attachment #8736532 - Attachment description: MozReview Request: Add mochitest to monitor backends. → MozReview Request: Add mochitest to monitor backends. - r?jrmuizel
Attachment #8737012 - Flags: review?(jmuizelaar)
Attachment #8737013 - Flags: review?(jmuizelaar)
(Assignee)

Comment 9

2 years ago
Created attachment 8737013 [details]
MozReview Request: Handle that ANGLE is a singleton. - r?jrmuizel

From 426ba4ee64c5829fd119f7f2a0eb16bd2f1350b0 Mon Sep 17 00:00:00 2001
This means we can't change which ANGLE backend we get at runtime.
Changing the ANGLE backend requires a browser restart.
---
 dom/canvas/test/webgl-mochitest/test_backends.html | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Review commit: https://reviewboard.mozilla.org/r/43645/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43645/
(Assignee)

Comment 10

2 years ago
Comment on attachment 8736530 [details]
MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43351/diff/1-2/
(Assignee)

Comment 11

2 years ago
Comment on attachment 8736531 [details]
MozReview Request: WARP should never be broken. - r?jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43353/diff/1-2/
(Assignee)

Comment 12

2 years ago
Comment on attachment 8736532 [details]
MozReview Request: Add mochitest to monitor backends. - r?jrmuizel

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43355/diff/1-2/
Attachment #8736531 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8736531 [details]
MozReview Request: WARP should never be broken. - r?jrmuizel

https://reviewboard.mozilla.org/r/43353/#review40479

I think I was misremembering this. It should be fine.

::: gfx/gl/GLLibraryEGL.cpp:387
(Diff revision 2)
>  
>      EGLDisplay chosenDisplay = nullptr;
>  
>      if (IsExtensionSupported(ANGLE_platform_angle_d3d)) {
>          bool accelAngleSupport = IsAccelAngleSupported(gfxInfo);
> -        bool warpAngleSupport = gfxPlatform::CanUseDirect3D11ANGLE();
> +        bool warpAngleSupport = true; // WARP should never be broken.

Just remove this variable completely
Comment on attachment 8737012 [details]
MozReview Request: Make force-warp a Live pref, not Once. - r?jrmuizel

https://reviewboard.mozilla.org/r/43643/#review40481
Attachment #8737012 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8737013 [details]
MozReview Request: Handle that ANGLE is a singleton. - r?jrmuizel

https://reviewboard.mozilla.org/r/43645/#review40483
Attachment #8737013 - Flags: review?(jmuizelaar) → review+
Is bug 1188167 a duplicate of this one?
Flags: needinfo?(jgilbert)
(Assignee)

Comment 18

2 years ago
(In reply to Milan Sreckovic [:milan] from comment #17)
> Is bug 1188167 a duplicate of this one?

Yes, good catch.
Flags: needinfo?(jgilbert)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1188167

Comment 20

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/43e65d0c425f
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Blocks: 1257692
(Assignee)

Comment 21

2 years ago
[Tracking Requested - why for this release]:
It should slightly help our WebGL activation rates on Windows 10.
Also it adds tests to help ensure things continue to work.
status-firefox47: --- → affected
tracking-firefox47: --- → ?
(Assignee)

Comment 22

2 years ago
Reopen for possible uplift.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 23

2 years ago
Comment on attachment 8736530 [details]
MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel

Approval Request Comment
[Feature/regressing bug #]: ANGLE update late 2013
[User impact if declined]: Lower WebGL activation rates on Windows.
[Describe test coverage new/current, TreeHerder]: Must be manually tested.
[Risks and why]: Low: There's not too much that can go wrong here.
[String/UUID change made/needed]: none
Attachment #8736530 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 24

2 years ago
Let's actually leave this for 48, given how close 47 uplift is.
status-firefox47: affected → wontfix
(Assignee)

Updated

2 years ago
Attachment #8736530 - Flags: approval-mozilla-aurora?
(Assignee)

Updated

2 years ago
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
tracking-firefox47: ? → ---
This hasn't caused any problems on nightly, and now on Aurora, right?  Would you consider uplifting to Beta if we get an approval?
Flags: needinfo?(jgilbert)
(Assignee)

Comment 26

2 years ago
(In reply to Milan Sreckovic [:milan] from comment #25)
> This hasn't caused any problems on nightly, and now on Aurora, right?  Would
> you consider uplifting to Beta if we get an approval?

Yes.
Status: RESOLVED → REOPENED
Flags: needinfo?(jgilbert)
Resolution: FIXED → ---
(Assignee)

Comment 27

2 years ago
Comment on attachment 8736530 [details]
MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel

Approval Request Comment
[Feature/regressing bug #]: ANGLE update late 2013
[User impact if declined]: Lower WebGL activation rates on Windows.
[Describe test coverage new/current, TreeHerder]: Must be manually tested.
[Risks and why]: Low: There's not too much that can go wrong here.
[String/UUID change made/needed]: none
Attachment #8736530 - Flags: approval-mozilla-beta?

Updated

2 years ago
status-firefox47: wontfix → affected
Comment on attachment 8736530 [details]
MozReview Request: Bug 1260944 - Fix WARP. - r?jrmuizel

Milan suggested we uplift this, Beta47+
Attachment #8736530 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 29

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/7754cd79897d
status-firefox47: affected → fixed
Hey Jeff, could you please expand a bit on how can this fix be manually tested? If manual testing is required to confirm the patch, perhaps our team should take a look at it.
Flags: needinfo?(jgilbert)
(Assignee)

Comment 31

2 years ago
(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #30)
> Hey Jeff, could you please expand a bit on how can this fix be manually
> tested? If manual testing is required to confirm the patch, perhaps our team
> should take a look at it.

Manual testing for this is:
1. In about:support, "WebGL Renderer" should NOT be "Google Inc. -- ANGLE (Microsoft Basic Render Driver Direct3D11 vs_5_0 ps_5_0)"
2. In about:config, set webgl.angle.force-warp to true
3. Restart the browser
4. In about:support, "WebGL Renderer" should be "Google Inc. -- ANGLE (Microsoft Basic Render Driver Direct3D11 vs_5_0 ps_5_0)"
5. Reset the webgl.angle.force-warp pref and restart the browser before doing other tests.
Flags: needinfo?(jgilbert)
(Assignee)

Updated

2 years ago
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Flags: needinfo?(andrei.vaida)
(In reply to Jeff Gilbert [:jgilbert] from comment #31)
> Manual testing for this is:
> 1. In about:support, "WebGL Renderer" should NOT be "Google Inc. -- ANGLE
> (Microsoft Basic Render Driver Direct3D11 vs_5_0 ps_5_0)"
> 2. In about:config, set webgl.angle.force-warp to true
> 3. Restart the browser
> 4. In about:support, "WebGL Renderer" should be "Google Inc. -- ANGLE
> (Microsoft Basic Render Driver Direct3D11 vs_5_0 ps_5_0)"
> 5. Reset the webgl.angle.force-warp pref and restart the browser before
> doing other tests.

Thanks, we'll make sure this gets looked at.
Flags: needinfo?(andrei.vaida) → qe-verify+
Depends on: 1273252

Comment 33

2 years ago
Verified fixed using Firefox 47 beta 7 (Build ID: 20160518173344) and latest Aurora 48.0a2, under Windows 10 64-bit - the expected results mentioned in comment 31 were encountered on both builds; also conducted a spot-check focusing some websites with WebGL content and no issues were found.
Status: RESOLVED → VERIFIED
status-firefox47: fixed → verified
status-firefox48: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.