Make timings graphs scalable to user's window #15766
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
rustbot
added
A-timings
|
As an alternative to filling the screen by defaut, we could set the scaler so that the graph takes e.g. 75% of screen width, might be a bit less "eye-popping". |
|
What browsers have you tested on? |
|
Just Chrome, but that's essentially what most browsers use these days anyway 😆 Also tested in Firefox now, looks fine. |
epage
reviewed
Jul 22, 2025
kpreid
reviewed
Jul 23, 2025
|
Updated the implementation based on Zulip discussion. The PR description has been updated with the description of the current approach. |
weihanglo
approved these changes
Jul 25, 2025
| const scaleElement = document.getElementById('scale'); | ||
| scaleElement.min = width_to_graph_scale(MIN_GRAPH_WIDTH); | ||
| scaleElement.max = width_to_graph_scale(MAX_GRAPH_WIDTH); | ||
| scaleElement.value = width_to_graph_scale(window.innerWidth * 0.75); |
There was a problem hiding this comment.
Where is the 0.75 from?
Regardless, feel good, and we can always tweak if people find a better default. Thank you for the PR!
There was a problem hiding this comment.
That is an arbitrary constant, to fit ~75% of the screen 😅
There was a problem hiding this comment.
I figured that. Anyway, I feel 75% is pretty balanced and eye-friendly.
rustbot
removed
the
S-waiting-on-review
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Jul 25, 2025
Update cargo 8 commits in 6833aa715d724437dc1247d0166afe314ab6854e..9b296973b425ffb159e12cf3cd56580fd5c85382 2025-07-13 02:25:52 +0000 to 2025-07-25 17:10:08 +0000 - Allow using Cargo-as-a-library with gix's reqwest backend (rust-lang/cargo#15653) - Make timings graphs scalable to user's window (rust-lang/cargo#15766) - refactor: rename arg `mode` to `intent` (rust-lang/cargo#15774) - fix: `no-proc-macro` is overridden by subsequent edges (rust-lang/cargo#15764) - Use `gix` for `cargo package` (rust-lang/cargo#15534) - cargo-credential-libsecret: give FFI correctly-sized object (rust-lang/cargo#15767) - Remove unnecessary target-c-int-width from target specs (rust-lang/cargo#15759) - Expose artifact dependency getters in cargo-as-a-library (rust-lang/cargo#15753)
bors
added a commit
to rust-lang/rust
that referenced
this pull request
Jul 25, 2025
Update cargo 8 commits in 6833aa715d724437dc1247d0166afe314ab6854e..9b296973b425ffb159e12cf3cd56580fd5c85382 2025-07-13 02:25:52 +0000 to 2025-07-25 17:10:08 +0000 - Allow using Cargo-as-a-library with gix's reqwest backend (rust-lang/cargo#15653) - Make timings graphs scalable to user's window (rust-lang/cargo#15766) - refactor: rename arg `mode` to `intent` (rust-lang/cargo#15774) - fix: `no-proc-macro` is overridden by subsequent edges (rust-lang/cargo#15764) - Use `gix` for `cargo package` (rust-lang/cargo#15534) - cargo-credential-libsecret: give FFI correctly-sized object (rust-lang/cargo#15767) - Remove unnecessary target-c-int-width from target specs (rust-lang/cargo#15759) - Expose artifact dependency getters in cargo-as-a-library (rust-lang/cargo#15753)
github-actions bot
pushed a commit
to rust-lang/rustc-dev-guide
that referenced
this pull request
Jul 28, 2025
Update cargo 8 commits in 6833aa715d724437dc1247d0166afe314ab6854e..9b296973b425ffb159e12cf3cd56580fd5c85382 2025-07-13 02:25:52 +0000 to 2025-07-25 17:10:08 +0000 - Allow using Cargo-as-a-library with gix's reqwest backend (rust-lang/cargo#15653) - Make timings graphs scalable to user's window (rust-lang/cargo#15766) - refactor: rename arg `mode` to `intent` (rust-lang/cargo#15774) - fix: `no-proc-macro` is overridden by subsequent edges (rust-lang/cargo#15764) - Use `gix` for `cargo package` (rust-lang/cargo#15534) - cargo-credential-libsecret: give FFI correctly-sized object (rust-lang/cargo#15767) - Remove unnecessary target-c-int-width from target specs (rust-lang/cargo#15759) - Expose artifact dependency getters in cargo-as-a-library (rust-lang/cargo#15753)
github-actions bot
pushed a commit
to rust-lang/rust-analyzer
that referenced
this pull request
Jul 28, 2025
Update cargo 8 commits in 6833aa715d724437dc1247d0166afe314ab6854e..9b296973b425ffb159e12cf3cd56580fd5c85382 2025-07-13 02:25:52 +0000 to 2025-07-25 17:10:08 +0000 - Allow using Cargo-as-a-library with gix's reqwest backend (rust-lang/cargo#15653) - Make timings graphs scalable to user's window (rust-lang/cargo#15766) - refactor: rename arg `mode` to `intent` (rust-lang/cargo#15774) - fix: `no-proc-macro` is overridden by subsequent edges (rust-lang/cargo#15764) - Use `gix` for `cargo package` (rust-lang/cargo#15534) - cargo-credential-libsecret: give FFI correctly-sized object (rust-lang/cargo#15767) - Remove unnecessary target-c-int-width from target specs (rust-lang/cargo#15759) - Expose artifact dependency getters in cargo-as-a-library (rust-lang/cargo#15753)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR try to resolve?
This PR changes the way the charts produced by
cargo build --timingsscale. It changes the scale slider so that its min/max values adapt to the duration of the build, to allow zooming in/out even for very short build durations. It also automatically initializes the scale value based on the client's window width.The number of pixels per second per scale value has been changed from 1 to 8, to avoid having too many scale values for the given duration of supported chart widths, which I have determined in this PR to be
[200, 4096]pixels.scalable-timings.mp4
How to test and review this PR?
Run
cargo build --timingse.g. on https://github.com/BurntSushi/ripgrep. Then open the resulting page in a browser, and try to enlarge/ensmall the window (possibly using mobile emulation), and see how the charts react to window size.Fixes: #15666