Bug 220843 – WebGL power preference and discrete/internal gpu selection implemented incorrectly with ANGLE

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Kimmo Kinnunen]

GraphicsContextGLOpenGLCocoa is changing the GPU behind ANGLE's back. This invalidates ANGLE assumptions.
The context gpu is changed based on which window the context is being used on. This cannot work with ANGLE, as all the contexts share one underlying implementation.

[Kimmo Kinnunen]

Created attachment 418112 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418114 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418123 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418129 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418130 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418138 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418149 [details]
Patch

[Kenneth Russell]

Comment on attachment 418149 [details]
Patch

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

Looks like a good improvement overall. A couple of comments. Would like to see what else needs to change in order to pass the tests.

> Source/WebCore/platform/graphics/mac/GraphicsChecksMac.cpp:2
> + * Copyright (C) 2018 Apple Inc. All rights reserved.

2021? Here and throughout for the new files.

> Source/WebCore/platform/graphics/mac/ScopedHighPerformanceGPURequest.h:44
> +        SwitchingGPUClient::singletonIfExists()->releaseHighPerformanceGPU();

Won't this crash if the singleton doesn't exist?

> Source/WebCore/platform/graphics/mac/ScopedHighPerformanceGPURequest.h:49
> +            SwitchingGPUClient::singletonIfExists()->releaseHighPerformanceGPU();

Here too.

> Source/WebCore/platform/graphics/mac/ScopedHighPerformanceGPURequest.h:61
> +        return { 0 };

This form of calling the 1-arg constructor threw me for a bit of a loop...

[Kimmo Kinnunen]

Created attachment 418170 [details]
Patch

[Kimmo Kinnunen]

(In reply to Kenneth Russell from comment #8)
> Comment on attachment 418149 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=418149&action=review
> 
> Looks like a good improvement overall. A couple of comments. Would like to
> see what else needs to change in order to pass the tests.
> 
> > Source/WebCore/platform/graphics/mac/GraphicsChecksMac.cpp:2
> > + * Copyright (C) 2018 Apple Inc. All rights reserved.
> 
> 2021? Here and throughout for the new files.

The code is from 2018 file, though.. I'll change.

> > Source/WebCore/platform/graphics/mac/ScopedHighPerformanceGPURequest.h:44
> > +        SwitchingGPUClient::singletonIfExists()->releaseHighPerformanceGPU();
> 
> Won't this crash if the singleton doesn't exist?

If it doesn't exist, nothing gets acquired..

[Kimmo Kinnunen]

Created attachment 418171 [details]
Patch

[Kimmo Kinnunen]

Created attachment 418375 [details]
Patch

[Kimmo Kinnunen]

I did my best to verify this based on the logging, for the time being until the underlying things are fixed.

[EWS]

Committed r271880: <https://trac.webkit.org/changeset/271880>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418375 [details].

[Radar WebKit Bug Importer]

<rdar://problem/73613562>

[Brent Fulgham]

This is actually:
<rdar://problem/70979077>.

[Marcin Ignac]

Has this ever shipped? Both Safari Version 14.1.1 (16611.2.7.1.4) and Safari TP Release 128 (Safari 15.0, WebKit 16612.1.22.11.3) are unable to select high-performance gpu on Big Sur 11.4 (20F71).

[Kenneth Russell]

How are you determining whether the discrete GPU was selected? With ANGLE's Metal backend - now the default in WebKit / Safari - I don't think there will be a system-wide GPU switch any more when the discrete GPU is used for WebGL. However, testing indicates differently - I'll file a bug dependent on this one.

[Kenneth Russell]

Note, have filed Bug 228298 with a small test case - not sure whether things are working correctly in the Metal backend - let's follow up on that bug.

[Kimmo Kinnunen]

It shipped and then broke again with Metal and then fixed in bug 227408 and currently shipped with previews.