Bug 231012

Summary: WebGL low-power and high-performance contexts should use different ANGLE Metal EGLDisplays
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, dino, dustin.kerstein, ews-watchlist, geofflang, giuseppesalvo, gman, jonahr, kbr, kkinnunen, kondapallykalyan, kpiddington, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=227408
https://bugs.webkit.org/show_bug.cgi?id=228298
https://bugs.webkit.org/show_bug.cgi?id=231946
Bug Depends on: 231011    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Kimmo Kinnunen 2021-09-30 01:34:33 PDT
WebGL low-power and high-performance contexts should use different ANGLE EGLDisplays
Comment 1 Giuseppe 2021-10-05 04:03:01 PDT
Hello, any updates on the issue? Can I help in some way? I could create a patch if possible, only need a few inputs.
This issue is a bit critical for the project I'm currently working on, to be able to give a nice experience to the users using Safari.
Comment 2 Kenneth Russell 2021-10-05 12:36:05 PDT
There's currently a problem inside ANGLE where despite passing different attribute lists to multiple calls to eglGetPlatformDisplay, the same EGLDisplay handle is being returned.

This issue is currently being discussed upstream in the ANGLE repository:

Reintroduce GPU power preference selection code into Metal backend
https://bugs.chromium.org/p/angleproject/issues/detail?id=6143

We're hoping to decide on a fix expediently, and be able to either roll it into WebKit along with an ANGLE update, or cherry-pick it into WebKit and resolve conflicts later during the next ANGLE roll.
Comment 3 Giuseppe 2021-10-06 12:14:24 PDT
Thanks for the explanation Kenneth, I'll follow the ANGLE thread.
Comment 4 Radar WebKit Bug Importer 2021-10-07 01:35:16 PDT
<rdar://problem/83971417>
Comment 5 Kimmo Kinnunen 2021-10-19 05:31:03 PDT
Created attachment 441712 [details]
Patch
Comment 6 Kimmo Kinnunen 2021-10-19 22:30:36 PDT
Created attachment 441849 [details]
Patch
Comment 7 Kimmo Kinnunen 2021-10-20 05:53:10 PDT
Created attachment 441873 [details]
Patch
Comment 8 Kimmo Kinnunen 2021-10-20 10:12:21 PDT
Created attachment 441892 [details]
Patch
Comment 9 Kimmo Kinnunen 2021-10-20 10:14:06 PDT
Alexey, would the TestWebKitAPI related changes look ok?
Comment 10 Alexey Proskuryakov 2021-10-20 10:19:05 PDT
A few comments about TestWebKitAPI:
- API tests run in parallel. Is it OK to run this code in parallel with other tests?
- Why copyright 2019 for TestGraphicsContextGLOpenGLCocoa.mm.
- There is probably a reason, but it seems strange that RuntimeApplicationChecks.h is included WebCoreUtilities.h that doesn't do any runtime application checks.
Comment 11 Kimmo Kinnunen 2021-10-21 02:02:21 PDT
Created attachment 441998 [details]
Patch for landing
Comment 12 Kimmo Kinnunen 2021-10-22 00:06:36 PDT
(In reply to Alexey Proskuryakov from comment #10)
> A few comments about TestWebKitAPI:
> - API tests run in parallel. Is it OK to run this code in parallel with
> other tests?

Yes, works in parallel when the parallelism means that each TestWebKitAPI process executes only one test at a time, but multiple TestWebKitAPI processes can be running in parallel.

> - Why copyright 2019 for TestGraphicsContextGLOpenGLCocoa.mm.
Done

> - There is probably a reason, but it seems strange that
> RuntimeApplicationChecks.h is included WebCoreUtilities.h that doesn't do
> any runtime application checks.

It contains functionality that modifies the state checked by RuntimeApplicationChecks.
Comment 13 Kimmo Kinnunen 2021-10-22 00:07:38 PDT
This should be compatible but redundant when upstream fix https://chromium-review.googlesource.com/c/angle/angle/+/3231986 is merged back.
Comment 14 Kimmo Kinnunen 2021-10-22 00:08:25 PDT
.. and as such the code portions can be reverted and the test preserved after the merge.
Comment 15 EWS 2021-10-22 00:36:07 PDT
Committed r284669 (243389@main): <https://commits.webkit.org/243389@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441998 [details].
Comment 16 Giuseppe 2021-10-22 08:53:39 PDT
I see happily that the issue was fixed! Thanks for the work on it.
Any idea on when it might land on Safari? As it's a bit critical for WebGL apps.