Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable GPU Process and UI side compositing on macOS #12161

Merged
merged 1 commit into from Apr 5, 2023

Conversation

rniwa
Copy link
Member

@rniwa rniwa commented Mar 30, 2023

56f0f42

Enable GPU Process and UI side compositing on macOS
https://bugs.webkit.org/show_bug.cgi?id=254725

Reviewed by Simon Fraser.

Enable GPU process for DOM rendering and UI side compositing by default on macOS.

Also add options to disable these two features to run-webkit-tests.

* Source/WTF/wtf/PlatformEnable.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::WebViewImpl):
* Tools/MiniBrowser/mac/SettingsController.m:
(-[SettingsController useUISideCompositing]):
* Tools/Scripts/webkitpy/api_tests/runner.py:
(Runner.command_for_port):
* Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:
(parse_args):
* Tools/Scripts/webkitpy/port/driver.py:
(Driver.cmd_line):
* Tools/WebKitTestRunner/Options.cpp:
(WTR::handleOptionNoRemoteLayerTree):
(WTR::OptionsHandler::OptionsHandler):
* Tools/WebKitTestRunner/TestOptions.cpp:
(WTR::TestOptions::defaults):
* Tools/WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::noUseRemoteLayerTree const):
* Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:
(WTR::PlatformWebView::PlatformWebView):
* Tools/WebKitTestRunner/mac/main.mm:
(setDefaultsToConsistentValuesForTesting):

Canonical link: https://commits.webkit.org/262629@main

862a82f

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ wpe-wk2
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitpy βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ›  jsc-armv7
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  watch-sim βœ… πŸ›  jsc-mips
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ§ͺ jsc-mips-tests

@rniwa rniwa requested a review from cdumez as a code owner March 30, 2023 07:05
@rniwa rniwa self-assigned this Mar 30, 2023
@rniwa rniwa added the Compositing Bugs related to layerization for transforms, video, scrolling etc label Mar 30, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Mar 30, 2023
@rniwa rniwa removed the merging-blocked Applied to prevent a change from being merged label Mar 30, 2023
@@ -342,6 +344,9 @@ def parse_args(args):
optparse.make_option(
"--use-gpu-process", action="store_true", default=False,
help=("Enable all GPU process related features, also set additional expectations and the result report flavor.")),
optparse.make_option(
"--no-use-gpu-process", action="store_true", default=False,
help=("Disable GPU process for DOM rendering.")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to PR #12222, we should consider arguments.NoAction here:

from webkitcorepy import arguments
...
optparse.make_option(
    '--use-gpu-process', '--no-use-gpu-process',
    dest='use_gpu_process', default=None,
    help='...',
    action=arguments.NoAction,
)

Here I think the case for arguments.NoAction is much stronger because it looks like we convert these to a feature flag. That would mean use_gpu_process =True -> UseGPUProcessForDOMRenderingEnabled and use_gpu_process =False -> UseGPUProcessForDOMRenderingEnabled=0 and use_gpu_process =None would pass nothing (which would have a GPU process enabled in WebKit but disabled in WebKitLegacy if I'm understanding the code correctly?). It does mean that something like --no-gpu-process and --dump-render-tree is redundant, but I think that's true in the proposed implementation anyways.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to work?

% ./Tools/Scripts/run-webkit-tests --debug --no-build --no-show-results fast/scrolling/scroll-animator-basic-events.htm
Traceback (most recent call last):
  File "/Volumes/Data/safari-2/OpenSource/./Tools/Scripts/run-webkit-tests", line 46, in <module>
    sys.exit(main(sys.argv[1:], sys.stdout, sys.stderr))
  File "/Volumes/Data/safari-2/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 57, in main
    options, args = parse_args(argv)
  File "/Volumes/Data/safari-2/OpenSource/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 123, in parse_args
    optparse.make_option("--remote-layer-tree", "--no-remote-layer-tree", dest="remote_layer_tree", action=arguments.NoAction, default=None,
  File "/AppleInternal/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/optparse.py", line 581, in __init__
    checker(self)
  File "/AppleInternal/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/optparse.py", line 636, in _check_action
    raise OptionError("invalid action: %r" % self.action, self)
optparse.OptionError: option --remote-layer-tree/--no-remote-layer-tree: invalid action: <class 'webkitcorepy.arguments.NoAction'>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem to work?

...

Forgot that run-webkit-tests uses the older optparse instead of argparse. So never mind, this suggestion won't work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it would be a nice cleanup to migrate that over but it's probably outside the scope of this PR.

@JonWBedard
Copy link
Member

The webkitpy and WebKitTestRunner bits look good, but I'm not the right person to review the core change here of enabling GPU Process and UI side compositing on macOS.

@rniwa rniwa added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 5, 2023
https://bugs.webkit.org/show_bug.cgi?id=254725

Reviewed by Simon Fraser.

Enable GPU process for DOM rendering and UI side compositing by default on macOS.

Also add options to disable these two features to run-webkit-tests.

* Source/WTF/wtf/PlatformEnable.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::WebViewImpl):
* Tools/MiniBrowser/mac/SettingsController.m:
(-[SettingsController useUISideCompositing]):
* Tools/Scripts/webkitpy/api_tests/runner.py:
(Runner.command_for_port):
* Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py:
(parse_args):
* Tools/Scripts/webkitpy/port/driver.py:
(Driver.cmd_line):
* Tools/WebKitTestRunner/Options.cpp:
(WTR::handleOptionNoRemoteLayerTree):
(WTR::OptionsHandler::OptionsHandler):
* Tools/WebKitTestRunner/TestOptions.cpp:
(WTR::TestOptions::defaults):
* Tools/WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::noUseRemoteLayerTree const):
* Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm:
(WTR::PlatformWebView::PlatformWebView):
* Tools/WebKitTestRunner/mac/main.mm:
(setDefaultsToConsistentValuesForTesting):

Canonical link: https://commits.webkit.org/262629@main
@webkit-commit-queue
Copy link
Collaborator

Committed 262629@main (56f0f42): https://commits.webkit.org/262629@main

Reviewed commits have been landed. Closing PR #12161 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Apr 5, 2023
@webkit-commit-queue webkit-commit-queue merged commit 56f0f42 into WebKit:main Apr 5, 2023
@rniwa rniwa deleted the fix254725 branch April 5, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Compositing Bugs related to layerization for transforms, video, scrolling etc
Projects
None yet
6 participants