Bug 205483

Summary: [WebGL] Enable ANGLE by default for Cocoa platforms
Product: WebKit Reporter: Dean Jackson <dino>
Component: New BugsAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, benjamin, cdumez, cmarcelo, dbates, eric.carlson, esprehn+autocc, ews-watchlist, glenn, graouts, gyuyoung.kim, jer.noble, justin_fan, kbr, kondapallykalyan, philipj, sergio, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=205627
https://bugs.webkit.org/show_bug.cgi?id=205662
https://bugs.webkit.org/show_bug.cgi?id=205665
https://bugs.webkit.org/show_bug.cgi?id=205664
https://bugs.webkit.org/show_bug.cgi?id=205663
https://bugs.webkit.org/show_bug.cgi?id=205666
https://bugs.webkit.org/show_bug.cgi?id=205667
https://bugs.webkit.org/show_bug.cgi?id=205668
https://bugs.webkit.org/show_bug.cgi?id=205707
https://bugs.webkit.org/show_bug.cgi?id=205756
https://bugs.webkit.org/show_bug.cgi?id=205739
https://bugs.webkit.org/show_bug.cgi?id=205738
https://bugs.webkit.org/show_bug.cgi?id=205737
https://bugs.webkit.org/show_bug.cgi?id=205736
https://bugs.webkit.org/show_bug.cgi?id=205735
https://bugs.webkit.org/show_bug.cgi?id=205734
https://bugs.webkit.org/show_bug.cgi?id=206318
Bug Depends on: 205707, 205843, 206318, 206625, 207080    
Bug Blocks: 126404, 198948, 205618, 205734, 207857, 210994    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
simon.fraser: review+
EWS test
none
EWS test 2
none
EWS test 3
none
EWS test 4
none
EWS test 5 none

Description Dean Jackson 2019-12-19 17:27:56 PST
[WebGL] Enable ANGLE by default for Cocoa platforms (except simulator)
Comment 1 Radar WebKit Bug Importer 2019-12-19 17:28:38 PST
<rdar://problem/58097701>
Comment 2 Dean Jackson 2019-12-19 17:29:54 PST
rdar://56925821
Comment 3 Dean Jackson 2019-12-19 17:44:28 PST
Created attachment 386169 [details]
Patch
Comment 4 Alexey Proskuryakov 2019-12-19 22:00:10 PST
Poor simulator! What is the plan for testing iOS port?
Comment 5 Dean Jackson 2019-12-20 09:39:47 PST
(In reply to Alexey Proskuryakov from comment #4)
> Poor simulator! What is the plan for testing iOS port?

Google are planning on allowing ANGLE to take a platform GL texture in place of an IOSurface.
Comment 6 Dean Jackson 2019-12-20 09:43:46 PST
Created attachment 386219 [details]
Patch
Comment 7 EWS Watchlist 2019-12-20 09:44:51 PST
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 8 Alexey Proskuryakov 2019-12-20 09:49:11 PST
Does the simulator not support something needed from IOSurface in this particular case?

See bug 203026, Turn on IOSurface support in the iOS Simulator.
Comment 9 Alexey Proskuryakov 2019-12-20 09:51:31 PST
Comment on attachment 386219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386219&action=review

> LayoutTests/webgl/2.0.0/conformance/renderbuffers/framebuffer-object-attachment-expected.txt:13
> +[ 10: FAIL ] gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36061

I haven't nearly reviewed much of the patch, but I scrolled around enough to find one regression. That would be a problem with zero regression policy.
Comment 10 Dean Jackson 2019-12-20 09:56:48 PST
Created attachment 386222 [details]
Patch
Comment 11 Dean Jackson 2019-12-20 10:17:41 PST
Comment on attachment 386219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386219&action=review

>> LayoutTests/webgl/2.0.0/conformance/renderbuffers/framebuffer-object-attachment-expected.txt:13
>> +[ 10: FAIL ] gl.checkFramebufferStatus(gl.FRAMEBUFFER) returned 36061
> 
> I haven't nearly reviewed much of the patch, but I scrolled around enough to find one regression. That would be a problem with zero regression policy.

There are some regressions, but there are many more progressions - so we've decided it is a net win.

(Also, in this case, it is a regression in WebGL 2 which is not enabled by default)
Comment 12 Kenneth Russell 2019-12-20 11:26:04 PST
(In reply to Alexey Proskuryakov from comment #8)
> Does the simulator not support something needed from IOSurface in this
> particular case?
> 
> See bug 203026, Turn on IOSurface support in the iOS Simulator.

Ah - I hadn't noticed that!

I'll try building for the iOS Simulator with the ANGLE/project.pbxproj build fix in this patch and see whether the IOSurface presentation path works.
Comment 13 Alexey Proskuryakov 2019-12-20 12:17:26 PST
Awesome!

There could be more things disabled in WebKit on Simulator that can/should be enabled now.
Comment 14 Justin Fan 2019-12-20 12:18:36 PST
Comment on attachment 386222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386222&action=review

> LayoutTests/fast/canvas/webgl/array-bounds-clamping-expected.txt:1
>   Checks that array access in a shader can not read out of bounds

Why was this test left in but other fast/canvas tests removed?
Comment 15 Dean Jackson 2019-12-20 15:55:19 PST
Created attachment 386272 [details]
Patch
Comment 16 Dean Jackson 2019-12-20 16:45:08 PST
Created attachment 386274 [details]
Patch
Comment 17 Dean Jackson 2019-12-20 16:56:50 PST
Comment on attachment 386222 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=386222&action=review

>> LayoutTests/fast/canvas/webgl/array-bounds-clamping-expected.txt:1
>>   Checks that array access in a shader can not read out of bounds
> 
> Why was this test left in but other fast/canvas tests removed?

This isn't in the WebGL conformance suite.
Comment 18 Dean Jackson 2019-12-20 16:59:48 PST
(In reply to Kenneth Russell from comment #12)
> (In reply to Alexey Proskuryakov from comment #8)
> > Does the simulator not support something needed from IOSurface in this
> > particular case?
> > 
> > See bug 203026, Turn on IOSurface support in the iOS Simulator.
> 
> Ah - I hadn't noticed that!
> 
> I'll try building for the iOS Simulator with the ANGLE/project.pbxproj build
> fix in this patch and see whether the IOSurface presentation path works.

Nope! It's not that the simulator doesn't have IOSurface, it's that

@interface EAGLContext(IOSurface)
- (BOOL)texImageIOSurface:(IOSurfaceRef)ioSurface target:(NSUInteger)target internalFormat:(NSUInteger)internalFormat width:(uint32_t)width height:(uint32_t)height format:(NSUInteger)format type:(NSUInteger)type plane:(uint32_t)plane NS_AVAILABLE_IOS(11_0);
@end

... is not implemented.
Comment 19 Kenneth Russell 2019-12-20 18:07:13 PST
(In reply to Dean Jackson from comment #18)
> (In reply to Kenneth Russell from comment #12)
> > (In reply to Alexey Proskuryakov from comment #8)
> > > Does the simulator not support something needed from IOSurface in this
> > > particular case?
> > > 
> > > See bug 203026, Turn on IOSurface support in the iOS Simulator.
> > 
> > Ah - I hadn't noticed that!
> > 
> > I'll try building for the iOS Simulator with the ANGLE/project.pbxproj build
> > fix in this patch and see whether the IOSurface presentation path works.
> 
> Nope! It's not that the simulator doesn't have IOSurface, it's that
> 
> @interface EAGLContext(IOSurface)
> - (BOOL)texImageIOSurface:(IOSurfaceRef)ioSurface target:(NSUInteger)target
> internalFormat:(NSUInteger)internalFormat width:(uint32_t)width
> height:(uint32_t)height format:(NSUInteger)format type:(NSUInteger)type
> plane:(uint32_t)plane NS_AVAILABLE_IOS(11_0);
> @end
> 
> ... is not implemented.

Thanks - I see that now, trying to enable that code path in ANGLE. I'll work on writing a (slow) emulation path for that in ANGLE and hopefully the WebKit-side code can work unmodified.
Comment 20 Dean Jackson 2019-12-21 19:03:14 PST
Weirdly, on this dual GPU device (MBP15), as soon as DumpRenderTree launches the machine swaps to the high-power GPU. This happens before the WebGL context is even created. Doesn't happen when running MiniBrowser with a WK1 window.
Comment 21 Dean Jackson 2019-12-21 19:05:06 PST
I'm trying to replicate the crash shown on the bots in WK1. It's not happening on this device when using the AMD chip in WK1, nor when using the Intel chip in WK2.
Comment 22 Dean Jackson 2019-12-21 19:08:50 PST
Actually, WebKitTestRunner triggers the high-power GPU too! MuxProbe says that the WindowServer and WebKitTestRunner/DumpRenderTree are the "bad" apps.

And it doesn't matter if it is a WebGL test or not.
Comment 23 Dean Jackson 2019-12-21 19:11:19 PST
https://bugs.webkit.org/show_bug.cgi?id=205546
Comment 24 Dean Jackson 2019-12-26 12:08:46 PST
Created attachment 386420 [details]
EWS test
Comment 25 Dean Jackson 2019-12-27 13:28:26 PST
Created attachment 386453 [details]
EWS test 2
Comment 26 Dean Jackson 2019-12-27 14:00:22 PST
Created attachment 386455 [details]
EWS test 3
Comment 27 Dean Jackson 2019-12-27 16:41:10 PST
Created attachment 386459 [details]
EWS test 4
Comment 28 Dean Jackson 2019-12-28 00:15:59 PST
Created attachment 386464 [details]
EWS test 5
Comment 29 Dean Jackson 2019-12-28 12:17:07 PST
Committed r253926: <https://trac.webkit.org/changeset/253926>
Comment 30 Alexey Proskuryakov 2019-12-30 17:35:09 PST
This patch adds many new expectations with "FAIL", but at least some of those are simply because those tests were marked as failing overall. Can you post a list of true known regressions, if there are any, and ideally file bugs for them?
Comment 31 Dean Jackson 2020-01-03 14:18:51 PST
Reverted in r254014
Comment 32 Kenneth Russell 2020-01-16 15:20:18 PST
Tracking down, and finding a fix or workaround for, the flakiness introduced with the ANGLE backend is our team's top priority.

At this point ANGLE has iOS Simulator support. Adjusting the summary.
Comment 33 Justin Fan 2020-01-21 14:45:16 PST
<rdar://problem/56925821>
Comment 34 Dean Jackson 2020-02-17 15:16:43 PST
Committed r256784: <https://trac.webkit.org/changeset/256784>