Summary: | WebGL power preference and discrete/internal gpu selection implemented incorrectly with ANGLE | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kimmo Kinnunen <kkinnunen> | ||||||||||||||||||||||
Component: | WebGL | Assignee: | 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
Kimmo Kinnunen
2021-01-21 23:59:38 PST
Created attachment 418112 [details]
Patch
Created attachment 418114 [details]
Patch
Created attachment 418123 [details]
Patch
Created attachment 418129 [details]
Patch
Created attachment 418130 [details]
Patch
Created attachment 418138 [details]
Patch
Created attachment 418149 [details]
Patch
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... Created attachment 418170 [details]
Patch
(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.. Created attachment 418171 [details]
Patch
Created attachment 418375 [details]
Patch
I did my best to verify this based on the logging, for the time being until the underlying things are fixed. Committed r271880: <https://trac.webkit.org/changeset/271880> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418375 [details]. This is actually: <rdar://problem/70979077>. 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). 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. 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. It shipped and then broke again with Metal and then fixed in bug 227408 and currently shipped with previews. |