Add web fullscreen support#1142
Conversation
Supporting fullscreen requires both the business logic (how to determine which canvas should be fullscreen) and the backend support (abstracting over the web methods that handle fullscreen.) This fulfills the latter.
|
I don't think this should block this PR or feature but it's maybe worth noting that the fullscreen API works on iOS Safari on iPads only. The iPhone has no fullscreen API. |
Pending on merge of koute/stdweb#370
| let height = window.inner_height().expect("Failed to get height").as_f64().expect("Failed to get height as f64"); | ||
|
|
||
| LogicalSize { width, height } | ||
| } |
There was a problem hiding this comment.
This should probably be a backend static method: web_sys::window_size().
| }); | ||
|
|
||
| let runner = self.runner.clone(); | ||
| let event_canvas = canvas.clone(); |
There was a problem hiding this comment.
Could we avoid cloning the whole canvas if we create the intended_size here and move it into the closure? We could keep there the size before entering fullscreen by reading the size of the raw canvas.
There was a problem hiding this comment.
We do need to keep track of intended_size as it changes, because it can be changed by the user at any point.
There was a problem hiding this comment.
I was thinking of ignoring user resizes when fullscreen is enabled. Does that break something? Or would it be unintuitive?
There was a problem hiding this comment.
That works for when fullscreen is enabled but the problem comes in when you have something like:
create window 1 at size of 800x600
set window 1's size to 640x480
fullscreen window 1
un-fullscreen window 1
You probably want it to return to 640x480, which means the intended size can change during the lifetime of the program.
There was a problem hiding this comment.
Couldn't we clone raw instead and use it to read the intended_size (with the width and height methods), store it while fullscreen mode is enabled, and restore the correct size when disabled? We could wrap the raw canvas in a new type if necessary. It's just that implementing Clone for the current Canvas type feels a bit hacky because of all the event listeners.
There was a problem hiding this comment.
Great point, I used that design and was able to remove the Clone impl from Canvas.
| *wants_fullscreen.borrow_mut() = false; | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
I understand this is an API limitation, but it will be a bit confusing if users are buffering events and processing them in batches. If a fullscreen request happens as a result of this event processing, it will not be applied until the user interacts again, right? This is a bit unfortunate.
There was a problem hiding this comment.
Yeah it is an unfortunate fact of the web fullscreen APIs. I'm not sure of a better way to work around it.
hecrj
left a comment
hecrj
left a comment
There was a problem hiding this comment.
Looks great! Nice work! 🥂
Just noticed a tiny thing that I think we should fix before merging.
| intended_size | ||
| }; | ||
| raw.set_width(new_size.width as u32); | ||
| raw.set_width(new_size.height as u32); |
There was a problem hiding this comment.
Whoops, the second one should be set_height I think.
3 participants
cargo fmthas been run on this branchCHANGELOG.mdif knowledge of this change could be valuable to usersPR progress:
This makes progress on #1072
Currently there is an issue that the event is not processed within a user handler, so the browser rejects the attempt to fullscreen.