Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
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 theGLFW.onCanvasResizewhich was added toBrowser.resizeListenersThere 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.
Uh oh!
There was an error while loading. Please reload this page.
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.
onCanvasResizeshould 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:
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
resizeListenersare not notified of the window size change, soonCanvasResizeis 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?Uh oh!
There was an error while loading. Please reload this page.
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:
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...):emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, glfwGetCurrentContext(), false, onCanvasSizeChanged);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!
Uh oh!
There was an error while loading. Please reload this page.
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.glfw3which 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: