-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix Glfw window resize #20831
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
Fix Glfw window resize #20831
Conversation
@@ -1206,6 +1206,9 @@ var LibraryGLFW = { | |||
window.addEventListener("keypress", GLFW.onKeyPress, true); | |||
window.addEventListener("keyup", GLFW.onKeyup, true); | |||
window.addEventListener("blur", GLFW.onBlur, true); | |||
window.addEventListener("resize", () => { | |||
GLFW.onCanvasResize(window.innerWidth, window.innerHeight); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't window.innerWidth, window.innerHeight be the size of the window rather than the size of the canvas?
Presumably calling Browser.updateResizeListeners()
would have the intended effect since that would then call the GLFW.onCanvasResize
which was added to Browser.resizeListeners
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling Browser.updateResizeListeners()
ends up being a noop as the canvas size is unchanged. Is the canvas size expected to remain unchanged if the window size changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not too familiar with those details but for sure there are cases (probably most cases) where you don't want the canvas taking up the whole window. I would imagine how the canvas size relates the windows size is determined by things like the CSS properties of the canvas object. @juj might have some more insight here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this is not correct. onCanvasResize
should be advertising the size of the canvas, not the whole size of the browser window.
There already exists the following code on line 1230/1233 below:
Browser.resizeListeners.push((width, height) => {
GLFW.onCanvasResize(width, height);
});
which should do the job? If it is not working then that sounds like some kind of bug in that code.
Note that there are two different sizes at play, because in WebGL the WebGL render target (front buffer) size is decoupled from the size of the visual rectangle that shows up on browser page (the CSS size).
The GLFW resize callback contract is interested in being notified about the WebGL render target size. That is, when a resize is notified, GLFW programs expect that the new render front buffer has the same size that is advertised in the resize callback. So passing CSS size in that callback would not be appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I resize my browser window, the resizeListeners
are not notified of the window size change, so onCanvasResize
is never called. This is true when using the default emscripten shell. This might be by design, but then how can I make the canvas size tied to that of the browser window?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pierricgimmig @juj from my experience dealing with this code for a while, this is what I have found:
Working in the context of a browser/canvas is very different from a desktop window. For glfw/emscripten, the canvas is the window and it is forcing the size of the canvas via css style (and as @juj mentioned I am working on this and for HiDPI there is no other choice really)
But the best way I have personally found to handle canvas resizing cleanly is to do this:
- in your html, you wrap your canvas in a
div
:
<div id="canvas-container">
<canvas class=emscripten id=canvas oncontextmenu=event.preventDefault() tabindex=-1></canvas>
</div>
then you can apply whatever css you want on the div
, for example for taking the full window I do this (but you can have margins and whatnot...):
#canvas-container {
position: absolute;
top: 0;
left: 0;
padding: 0;
border: 0 none;
margin: 0;
width: 100%;
height: 100%;
}
- in your code you register a window size listener on the container (you cannot listen for a resize on a canvas...)
emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, glfwGetCurrentContext(), false, onCanvasSizeChanged);
- in the callback you handle it this way:
static EM_BOOL onCanvasSizeChanged(int /* event_type */, const EmscriptenUiEvent* /* ui_event */, void* user_data)
{
double canvas_width, canvas_height;
emscripten_get_element_css_size("#canvas-container", &canvas_width, &canvas_height);
glfwSetWindowSize(glfwGetCurrentContext(), (int)canvas_width, (int)canvas_height);
return true;
}
Note how the size of the container is read and set as the canvas size (via glfwSetWindowSize
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, it all makes sense now :-) Thanks to all for the replies!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ypujante is this still the best way to do this? I've seen a few changes on the repo of the port, not sure how this is aligned with latest versions.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@albertrodriguezclo You can now use --use-port=contrib.glfw3
which implements a different and more modern version of the port. Check out the documentation for handling resizing with this port (examples are provided in this repo for every use-case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ypujante nvm, used your guide as an example: https://github.com/pongasoft/emscripten-glfw/blob/master/examples/example_resizable_container
This was my missing part:
#ifdef __EMSCRIPTEN__
// makes the canvas resizable to the size of its divs container
emscripten::glfw3::MakeCanvasResizable(m_window, "#canvas-container");
#endif
Window resize events are not piped to neither glfwSetFramebufferSizeCallback nor glfwSetWindowSizeCallback making the application unaware that the browser window was dynamically resized.
I noticed the behavior on Windows and MacOS with a small WebGpu application. The suggested code fixes the issue, but I'm not sure if it's the right solution. It seems
Browser.resizeListeners
are not called when resizing the browser window, perhaps there's a problem with this mechanism.Associated issue: #20830