winit
winit copied to clipboard
Use `ResizeObserver` to control the size of the canvas
- [X] Tested on all platforms changed
Specifically on Firefox Developer Edition and Safari.
- [X] Compilation warnings were addressed
- [X]
cargo fmt
has been run on this branch - [X]
cargo doc
builds successfully - [X] Added an entry to
CHANGELOG.md
if knowledge of this change could be valuable to users - [X] Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
- [ ] Created or updated an example program if it would help users understand this functionality
- [ ] ~~Updated feature matrix, if new features were added or implemented~~
Resolves #1661
I've changed the web backend to set the window size entirely based on the canvas' CSS size using ResizeObserver
. It uses device-pixel-content-box
where available, but falls back on ResizeObserverEntry.contentRect
if it's not supported.
One issue is how to handle the window's initial size. ResizeObserver
does fire an event for the initial size when created, but it won't run until the task finishes, which means it isn't available for any init code before the event loop starts. I've worked around this by just getting the element's computed size and subtracting padding, and using that to estimate its size for the initial inner_size
value, but this doesn't work at all if the canvas is created by winit and then added to the DOM. If a canvas isn't in the DOM, its size is reported as 0, so if the canvas is created this way its initial size will always be 0.
At the moment this has to pull in a git version of web-sys
and enable cfg(web_sys_unstable_apis)
, so it can't really be shipped. The API does seem to be supported in pretty much all modern browsers, but it's marked unstable since it's a draft.
To-do
- [x] Wait for
web-sys
to release a new version - [X] Either wait for
ResizeObserver
to be stabilised or feature gate this somehow
Unresolved questions
- [ ] Should this be hidden behind a mode, instead of always enabled? Applications which already resize manually using
set_inner_size
should work functionally identically anyway; this should only really affect things which don't touch the size at all. - [ ] Should setting min/max size set the CSS min/max-width/height properties?
- [ ] Should outer size refer to the canvas' border box? Now that CSS can be freely applied to the canvas, it's possible for it to have padding or borders, in which case the element's outer size might not be the same as its content size.
Looks very cool! Let me know if you need help testing.
I tried this branch on my app and my app blows up in wgpu because I'm giving it a window with size of 0x0. Maybe we should we be respecting https://docs.rs/winit/latest/winit/window/struct.Window.html#method.set_min_inner_size or at least hardcode a minimum size of 1x1 ?
Would also be good to rebase, to make using the PR via [patch.crates-io]
easier.
You can reproduce the problem by cloning https://github.com/rukai/wgpu/tree/resize_observer
and then running ./run-wasm-example.sh hello-triangle webgl
I tried this branch on my app and my app blows up in wgpu because I'm giving it a window with size of 0x0. Maybe we should we be respecting https://docs.rs/winit/latest/winit/window/struct.Window.html#method.set_min_inner_size or at least hardcode a minimum size of 1x1 ?
That issue was occuring because the window's initial size was being measured in Window::new
, before the canvas was added to the DOM. At that point, the canvas's size is reported as 0x0. To work around that, I've changed it to measure the canvas's initial size on the first call to Window::inner_size
instead, if it isn't in the DOM during Window::new
.
Maybe we should we be respecting https://docs.rs/winit/latest/winit/window/struct.Window.html#method.set_min_inner_size or at least hardcode a minimum size of 1x1?
Getting resize events that tell you the window is 0x0
is currently expected behaviour, so user code generally has to check for it before passing the size on to wgpu anyway. This is, however, a common stumbling block, so maybe there's room for changing the API to guide users toward something that works by default.
Any status update on this PR? The bug it solves is likely to hit pretty much everyone using Rust with GPU on the web (through Bevy or something else), so even an imperfect implementation that relies on some unstable-but-widely-available feature is certainly much better than the status quo.
I tested again and it now works perfectly for my application!
Thanks so much for implementing this, I would love to see it merged under a feature flag before ResizeObserver is stabilized.
For anyone else wanting to make use of this, here is the html, css and rust I used to get value out of this. The result is a winit app that fills all the available horizontal space at a 4/3 aspect ratio.
<div id="render">
<canvas gets inserted here by rust code>
</div>
#render {
width: 100%;
aspect-ratio: 4 / 3;
}
let window = Window::new(&event_loop).unwrap();
let canvas = window.canvas();
canvas
.style()
.set_css_text("display: block; width: 100%; height: 100%");
document.get_element_by_id("render").unwrap()
.append_child(&web_sys::Element::from(canvas))
.unwrap();
Given that the spec for the API in question is still a "working draft", I'm a bit hesitant to enable it by default, despite its "wide availablity".
I've put the ResizeObserver
functionality behind a css-size
feature, so that this can be used without everyone having to enable cfg(web_sys_unstable_apis)
.
I've noticed a bit of an issue, though - if the scale factor is changed before the event loop starts, it'll panic, since it tries to handle the event synchronously when the event loop isn't running. It used to just not check for them at all before the event loop started, but that's not great either. How do other platforms handle this?
From looking at the table here: https://developer.mozilla.org/en-US/docs/Web/API/Resize_Observer_API#browser_compatibility It looks like all the unstable API's we need have been stabilized? Can wasm-bindgen be updated accordingly and this PR progressed?
Maybe wasm-bindgen has stronger requirements for it to be considered stable though...?
It was marked unstable because the spec is a 'Working Draft', which isn't technically a real spec yet. web-sys
already has plenty of Working Drafts not marked unstable, though (e.g. the File API), so I think it'd be fine to stabilize it.
https://github.com/rust-windowing/winit/pull/2209 has merged, so it would be good to rebase/merge to make it easy to test on the new example.
This is indispensable for any canvas application compiled to wasm, any progress?
Given that the spec for the API in question is still a "working draft", I'm a bit hesitant to enable it by default, despite its "wide availablity".
Pointed here from a conversation on Mastodon. I know effectively nothing about Rust but I am the editor of one or two web standards, so I figured I could help clarify this point at least.
A spec being a "Working Draft" doesn't mean much in terms of how stable, widespread, or standardized the specification has become. Ideally the spec authors want their specification to be granted "Candidate Recommendation" (CR) status by the W3C, but that typically isn't even considered until at least two independent browsers have been shipping compatible implementations of the feature for at least a little while. And even the spec moving from Draft to CR status is a formality that has no real bearing on how browsers, developers, or users approach the feature.
What this means for developers is that CanIUse.com is a far more practical reference than the spec documents for determining what is actually shipped and stable. In this specific case ResizeObserver has been ~universally available since Jan 2020, and is definitely not going to be removed from the web platform or see any breaking changes in the foreseeable future. Chrome usage stats show that over 20% of web pages that users load make use of ResizeObserver, and nobody is eager to break 20% of the web, regardless of whether the spec says "Draft" at the top or not.
Hope that helps clear up some of the (admittedly confusing) verbiage around web specs!
It appears that this code has been working for over a year and the ResizeObserver "draft" API has been unchanged for over three years. Caniuse has ResizeObserver working on everything except IE11.
I was hoping to release some wgpu sample code before Chrome's initial release of webgpu next week (?). --cfg=web_sys_unstable_apis
is not a problem for this application because the whole of webgpu is unstable.
This PR is somewhere between useful and essential for practical webgpu use. I set the version of winit in my sample code's Cargo.toml ( https://github.com/mcclure/rs-hello 2b3303b / https://data.runhello.com/j/wgpu/2/) to directly pull this branch from github and as-is the PR works great (although it's too bad because I think this means I'm basing my code on a version of winit older than 0.28.3?). I can't speak to your package's stability policy but is there a way some version of this could get into shipping winit versions? The feature is a very good one.
Thanks!
Thanks for the clarification, @toji. If anyone is willing to pick up work on this again (merge conflicts and whatnot), then I'd be happy to see us use ResizeObserver
by default.
I can't speak to your package's stability policy but is there a way some version of this could get into shipping winit versions?
If I'm not mistaken, some fixes do get back-ported, but I don't recall the criteria for it.
I'm happy to get this PR sorted out and rebased after https://github.com/rust-windowing/winit/pull/2789.
Awesome!
If I'm not mistaken, some fixes do get back-ported, but I don't recall the criteria for it.
Sorry, I meant "can it get in a future shipping version", not that I needed it compatible with any specific shipping version.
I think the big problem right now is how to deal with --cfg=web_sys_unstable_apis
, it's a pretty terrible user experience, though I wouldn't mind still going through with this as long as it's gated behind that instead of introducing a crate feature that requires it.
I made https://github.com/rustwasm/wasm-bindgen/pull/3459 to avoid this whole issue though. So we should probably wait for that.
Replaced by #2859.