Bug 227633

Summary: [macOS] Only use WebGL on Metal by default on safe versions of macOS
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebGLAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cdumez, cmarcelo, dino, ews-watchlist, kbr, kkinnunen, kpiddington, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Brent Fulgham 2021-07-02 11:51:17 PDT 
Some older supported macOS releases have a version of Metal that does not work properly for implementing WebGL features. We should stick to the OpenGL backend on those releases (by default) and only activate Metal WebGL on systems we know pass all tests.

Users can still activate WebGL on Metal if they wish, but it will not be the default behavior.

<rdar://problem/78527887>
Comment 1 Brent Fulgham 2021-07-02 11:59:00 PDT 
Created attachment 432809 [details]
Patch
Comment 2 Dean Jackson 2021-07-02 13:09:20 PDT 
Comment on attachment 432809 [details]
Patch

Won't this disable it on watchOS, tvOS and Catalyst?
Comment 3 Brent Fulgham 2021-07-02 14:13:40 PDT 
(In reply to Dean Jackson from comment #2)
> Comment on attachment 432809 [details]
> Patch
> 
> Won't this disable it on watchOS, tvOS and Catalyst?

#if PLATFORM(IOS_FAMILY) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 120000)
#define HAVE_WEBGL_COMPATIBLE_METAL 1
#endif

This should ENABLE it for watchOS, tvOS, Catalyst, and iPhone, and on any macOS at or above 120000.

I THINK!?!
Comment 4 Brent Fulgham 2021-07-02 14:21:47 PDT 
I asked Tim to double-check and he said it looked right.
Comment 5 EWS 2021-07-02 16:21:09 PDT 
Committed r279524 (239374@main): <https://commits.webkit.org/239374@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 432809 [details].
Comment 6 Kenneth Russell 2021-07-04 18:22:43 PDT 
Comment on attachment 432809 [details]
Patch

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

> Source/WTF/wtf/PlatformHave.h:1097
> +#if PLATFORM(IOS_FAMILY) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 120000)

Could you explain a bit more how these compile time guards work? I would assume that runtime checks would be needed in order to choose the Metal backend only when running on certain macOS versions.
Comment 7 Tim Horton 2021-07-04 18:28:25 PDT 
(In reply to Kenneth Russell from comment #6)
> Comment on attachment 432809 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432809&action=review
> 
> > Source/WTF/wtf/PlatformHave.h:1097
> > +#if PLATFORM(IOS_FAMILY) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 120000)
> 
> Could you explain a bit more how these compile time guards work? I would
> assume that runtime checks would be needed in order to choose the Metal
> backend only when running on certain macOS versions.

No need for a runtime check; we build separate binaries with the appropriate SDK for each OS, so a check like this is fine (and very common).
Comment 8 Tim Horton 2021-07-04 18:28:25 PDT 
(In reply to Kenneth Russell from comment #6)
> Comment on attachment 432809 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432809&action=review
> 
> > Source/WTF/wtf/PlatformHave.h:1097
> > +#if PLATFORM(IOS_FAMILY) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 120000)
> 
> Could you explain a bit more how these compile time guards work? I would
> assume that runtime checks would be needed in order to choose the Metal
> backend only when running on certain macOS versions.

No need for a runtime check; we build separate binaries with the appropriate SDK for each OS, so a check like this is fine (and very common).