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

Allow canvas size to be controlled by page layout on web target #1661

Closed
alvinhochun opened this issue Aug 18, 2020 · 9 comments · Fixed by #2859
Closed

Allow canvas size to be controlled by page layout on web target #1661

alvinhochun opened this issue Aug 18, 2020 · 9 comments · Fixed by #2859
Labels
DS - web H - help wanted Someone please save us S - enhancement Wouldn't this be the coolest?

Comments

@alvinhochun
Copy link
Contributor

Currently, the web target uses a fixed window size and it sizes the canvas based on it. It would be nice to instead have a mode that allows the canvas to be laid out on the page with CSS layout like flexbox or CSS grid. The resizing of the element can be detected using ResizeObsever, which can also handle pixel-perfect canvas sizing (#1659 (comment)). In this mode, the winit window should not be resizable by user code.

@ryanisaacg
Copy link
Contributor

I think that's a great idea. I don't have time to implement it myself for now, but I would be happy to review a PR.

@alvinhochun
Copy link
Contributor Author

I would like to give this a go, but ResizeObserver is still a draft and web-sys` doesn't yet have the bindings (opened rustwasm/wasm-bindgen#2289) which means I will have to write the bindings myself. I'll see how far I can get.

@alvinhochun
Copy link
Contributor Author

A bit of reflection from my POC experiment:

  1. Adding our own ResizeObserver bindings before web-sys adds them is doable.
  2. Falling back to window resize event is good enough for most cases but we should document its limitations.
  3. Without device-pixel-content-box support, there really is no way to accurately get the canvas backbuffer to be pixel-perfect. All we can use is guesswork. The guesswork I used did seem to work fine though, with the limited number of browsers I've tested.
  4. Manual scale and size change tracking is needed to prevent superfluous resize events.
  5. The ResizeObserver approach requires using a container element for size reference. The container element also needs some specific CSS styles. Therefore, for this use case it seems better to have the user provide the container element and for winit to manage the canvas element, unlike how it currently works in winit.

@grovesNL
Copy link
Contributor

@alvinhochun for the second point in the list about the fallback path, does winit already add event listeners to window resize events for window resizing (i.e. not only switching between full-screen mode)?

@alvinhochun
Copy link
Contributor Author

Without checking the source code, I don't recall Winit currently utilizing the window resize event at all, not even for full-screen mode. When I did the POC experiment I added a resize event listener just for the fallback purpose.

@felipellrocha
Copy link

What's the status on this? I'm blocked from creating a fullscreen experience without this.

@alvinhochun
Copy link
Contributor Author

alvinhochun commented Dec 1, 2020

What's the status on this? I'm blocked from creating a fullscreen experience without this.

AFAIK nobody is working on it. I have the rough idea on how it can be done in practice and there is the proof-of-concept. I had wanted to write the final implementation, but I got caught by other personal matters so I don't have an estimate when I would be available to do it.

Another note is that my idea is pretty generic, as it aims to support any CSS page layout positioning, which makes the implementation quite complicated. If your aim is only to have the canvas to fill the page viewport, it is actually much easier to do as you just need to rely on the window resize event without the complexity of ResizeObserver. You can implement it yourself using something like a-b-street/abstreet#388.

@frewsxcv
Copy link
Contributor

There is an open pull request for this: #2074

@developomp
Copy link

Pog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DS - web H - help wanted Someone please save us S - enhancement Wouldn't this be the coolest?
6 participants