Skip to content

libservo: Remove Opts::webrender_stats and -Z wr-stats - #39331

Merged
mrobinson merged 1 commit into
servo:mainfrom
mrobinson:remove-wr-stats-option
Sep 17, 2025
Merged

mrobinson merged 1 commit into
servo:mainfrom
mrobinson:remove-wr-stats-option

Conversation

@mrobinson

Copy link
Copy Markdown
Member

This option is supported via the WebView API now and you can enable it
in servoshell by pressing Ctrl + F12. The command-line argument and
Opts field are older and I believe are no longer necessary.

Testing: This just removes a redundant command-line option, so no tests.

@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 16, 2025
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Sep 16, 2025
@jdm
jdm added this pull request to the merge queue Sep 16, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Sep 16, 2025
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 16, 2025
@servo-highfive servo-highfive added S-needs-rebase There are merge conflict errors. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Sep 16, 2025
@mukilan

mukilan commented Sep 17, 2025

Copy link
Copy Markdown
Member

@mrobinson there are merge conflicts in this PR. Also, is the 'Ctrl + F12' option to toggle the webrender stats documented somewhere?

@mrobinson

Copy link
Copy Markdown
Member Author

@mrobinson there are merge conflicts in this PR. Also, is the 'Ctrl + F12' option to toggle the webrender stats documented somewhere?

There is currently no documentation for servoshell keybindings, so I do not think so.

@mukilan

mukilan commented Sep 17, 2025 •

Copy link
Copy Markdown
Member

There is currently no documentation for servoshell keybindings, so I do not think so.

The key bindings are documented in the book. For the wr-stats option, I guess it should also be mentioned in the debugging section, I can add this, but I don't know the context e.g when this should be used etc. I'll create a separate issue to discuss this.

Ctrl+F12 is a global shortcut on my KDE desktop. I'm not sure if other platforms also map Ctrl+F12, but are we sure a command line option to toggle these debugging features will not be useful? In any case, Ctrl+F12 doesn't seem to show any WebRender stats on my machine, even after disabling the global shortcut.

@mrobinson

Copy link
Copy Markdown
Member Author

Ctrl+F12 is a global shortcut on my KDE desktop. I'm not sure if other platforms also map Ctrl+F12, but are we sure a command line option to toggle these debugging features will not be useful? In any case, Ctrl+F12 doesn't seem to show any WebRender stats on my machine, even after disabling the global shortcut.

In order to see the output of this you need to enable the profiling feature and then scroll around for a while. After that, if you request a WebRender capture via the keyboard (Ctrl + F9), you should see the profiler-stats.txt file in the capture directory. I have not yet figured out how to make WebRender populate the file even though it creates it.

Since this is related to the capture functionality which is controlled by the keyboard, I'm not sure a command-line argument is the appropriate way to activate it. Perhaps we should change the shortcut key or even remove the functionality (unless we can figure out how to make it populate the file).

This option is supported via the WebView API now and you can enable it
in servoshell by pressing `Ctrl` + `F12`. The command-line argument and
`Opts` field are older and I believe are no longer necessary.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@mukilan

mukilan commented Sep 17, 2025

Copy link
Copy Markdown
Member

Perhaps we should change the shortcut key or even remove the functionality (unless we can figure out how to make it populate the file).

Sounds good. Another option would be to expose this as a button in the minibrowser's toolbar or in an about:debugging page.

@mrobinson
mrobinson force-pushed the remove-wr-stats-option branch from de7a5dd to f23e034 Compare September 17, 2025 10:54
@servo-highfive servo-highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-rebase There are merge conflict errors. labels Sep 17, 2025
@mrobinson
mrobinson added this pull request to the merge queue Sep 17, 2025
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Sep 17, 2025
Merged via the queue into servo:main with commit 5505d00 Sep 17, 2025
28 checks passed
@mrobinson
mrobinson deleted the remove-wr-stats-option branch September 17, 2025 13:40
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-awaiting-review There is new code that needs to be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants