Closed Bug 1260944 Opened 8 years ago Closed 8 years ago

Fix ANGLE D3D WARP for WebGL

Categories

(Core :: Graphics: CanvasWebGL, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox47 --- verified
firefox48 --- verified

People

(Reporter: jgilbert, Assigned: jgilbert)

References

(Blocks 1 open bug)

Details

Attachments

(5 files)

The D3D WARP path in ANGLE isn't being hit anymore.
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/
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/
Attachment #8736531 - Flags: review?(jmuizelaar)
Attachment #8736532 - Flags: review?(jmuizelaar)
See Also: → 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)
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)
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/
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/
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/
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)
(In reply to Milan Sreckovic [:milan] from comment #17)
> Is bug 1188167 a duplicate of this one?

Yes, good catch.
Flags: needinfo?(jgilbert)
https://hg.mozilla.org/mozilla-central/rev/43e65d0c425f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
[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.
Reopen for possible uplift.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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?
Let's actually leave this for 48, given how close 47 uplift is.
Attachment #8736530 - Flags: approval-mozilla-aurora?
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
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)
(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 → ---
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?
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+
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)
(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)
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
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
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
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: