Skip to content

fix: set current size in Canvas::create based on inner_size attribute#3502

Closed
nicksenger wants to merge 1 commit into
rust-windowing:masterfrom
nicksenger:fix/create-canvas-size
Closed

fix: set current size in Canvas::create based on inner_size attribute#3502
nicksenger wants to merge 1 commit into
rust-windowing:masterfrom
nicksenger:fix/create-canvas-size

Conversation

@nicksenger
Copy link
Copy Markdown

This PR updates Canvas::create to set the current_size based on attr.inner_size if present. This allows the window size to be used immediately on web.

  • Tested on all platforms changed
  • [x ] Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • 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

@nicksenger nicksenger requested a review from daxpedda as a code owner February 19, 2024 19:02
@nicksenger nicksenger force-pushed the fix/create-canvas-size branch from de49da6 to 823baf7 Compare February 19, 2024 19:16
@daxpedda daxpedda added the DS - web Affects the Web backend (WebAssembly/WASM) label Feb 19, 2024
@daxpedda
Copy link
Copy Markdown
Member

daxpedda commented Feb 19, 2024

The reason why the size isn't immediately available is because there is currently no way to retrieve the actual size of the canvas in pixel synchronously.

For context:
We currently use ResizeObservers devicePixelContentBoxSize to get the actual size of the canvas in pixel. AFAIK this is the only Web API that can do this accurately.

Unfortunately this API is event-based, so we receive the size of the canvas in an event, so until we receive this event we don't know the actual size of the canvas.

We have decided that this is currently the best course of action unless we want to introduce a larger API surface which can tell the user if the size returned by Window::inner_size() is currently accurate or not, which we feel is a contra-productive direction to go in.

You can see the history and research on this here: #2859.
The solution to this problem we currently have in mind is here: #2863.


Your current implementation changes the size reported by Window::inner_size() at the start to an inaccurate one, which could lead to weird behavior as seen in this demo.

With that in mind I will close the PR.

Please let me know if you have any suggestions on how to tackle this problem other then #2863!

@daxpedda daxpedda closed this Feb 19, 2024
@nicksenger
Copy link
Copy Markdown
Author

Thanks a lot for the clarification! I figured this was just a weird corner-case, but seems like its been given a fair bit of thought already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DS - web Affects the Web backend (WebAssembly/WASM)

Development

Successfully merging this pull request may close these issues.

2 participants