Bug 220843

Summary: WebGL power preference and discrete/internal gpu selection implemented incorrectly with ANGLE
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, bfulgham, cdumez, changseok, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, kbr, kondapallykalyan, kpiddington, marcin.ignac, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Mac (Intel)   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=227408
Bug Depends on: 220845, 220852, 220989    
Bug Blocks: 220844, 220846, 228298    
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Kimmo Kinnunen
Reported 2021-01-21 23:59:38 PST
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.
Attachments
Patch (58.31 KB, patch)
2021-01-22 00:20 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (58.63 KB, patch)
2021-01-22 00:27 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Patch (59.76 KB, patch)
2021-01-22 02:33 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Patch (58.74 KB, patch)
2021-01-22 02:59 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (57.62 KB, patch)
2021-01-22 03:05 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (57.84 KB, patch)
2021-01-22 04:35 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Patch (57.95 KB, patch)
2021-01-22 08:12 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Patch (58.08 KB, patch)
2021-01-22 12:33 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Patch (58.08 KB, patch)
2021-01-22 12:52 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Patch (58.17 KB, patch)
2021-01-26 00:02 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.
Kimmo Kinnunen
Comment 1 2021-01-22 00:20:10 PST
Created attachment 418112 [details] Patch
Kimmo Kinnunen
Comment 2 2021-01-22 00:27:16 PST
Created attachment 418114 [details] Patch
Kimmo Kinnunen
Comment 3 2021-01-22 02:33:13 PST
Created attachment 418123 [details] Patch
Kimmo Kinnunen
Comment 4 2021-01-22 02:59:28 PST
Created attachment 418129 [details] Patch
Kimmo Kinnunen
Comment 5 2021-01-22 03:05:59 PST
Created attachment 418130 [details] Patch
Kimmo Kinnunen
Comment 6 2021-01-22 04:35:30 PST
Created attachment 418138 [details] Patch
Kimmo Kinnunen
Comment 7 2021-01-22 08:12:53 PST
Created attachment 418149 [details] Patch
Kenneth Russell
Comment 8 2021-01-22 11:28:55 PST
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
Comment 9 2021-01-22 12:33:21 PST
Created attachment 418170 [details] Patch
Kimmo Kinnunen
Comment 10 2021-01-22 12:51:23 PST
(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
Comment 11 2021-01-22 12:52:51 PST
Created attachment 418171 [details] Patch
Kimmo Kinnunen
Comment 12 2021-01-26 00:02:38 PST
Created attachment 418375 [details] Patch
Kimmo Kinnunen
Comment 13 2021-01-26 05:25:40 PST
I did my best to verify this based on the logging, for the time being until the underlying things are fixed.
EWS
Comment 14 2021-01-26 05:44:05 PST
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
Comment 15 2021-01-26 05:45:16 PST
<rdar://problem/73613562>
Brent Fulgham
Comment 16 2021-01-29 10:26:45 PST
This is actually: <rdar://problem/70979077>.
Marcin Ignac
Comment 17 2021-07-24 15:51:05 PDT
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
Comment 18 2021-07-26 13:47:27 PDT
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
Comment 19 2021-07-26 13:54:52 PDT
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
Comment 20 2021-08-02 01:02:42 PDT
It shipped and then broke again with Metal and then fixed in bug 227408 and currently shipped with previews.
Note You need to log in before you can comment on or make changes to this bug.