examples/example_emscripten_wgpu: Initialize and change the size of the viewport and framebuffer according to the size of the canvas

[Traveller23]

First, use emscripten‘s built-in method emscripten_get_element_css_size to get the size of the canvas in order to initialize the viewport and framebuffer.
Then, use emscripten's built-in method emscripten_set_resize_callback to listen for changes of the browser's window size, and update the size of the viewport and framebuffer.

I've only implemented the GLFW case, I haven't used SDL. But the implementation should be very similar, and probably the only difference is "glfwSetWindowSize" on line 262.

Related discussion: #6640

[ocornut]

Thanks for the PR.
Ideally we would need this to be done/working on Chrome+Firefox and all on 3 examples supporting Emscripten: example_emscripten_wgpu, example_glfw_opengl3, example_sdl2_opengl3.

This already works with example_sdl2_opengl3 for me, without your PR.
So I wanted to dig how SDL did it...

https://github.com/libsdl-org/SDL/blob/d86d02bbbfee47936164dda28749ae65e0feb443/src/video/emscripten/SDL_emscriptenevents.c#L858
Emscripten_HandleResize() and Emscripten_HandleCanvasResize() functions.

I don't know how SDL knows which canvas to query but we need to investigate how SDL does it:
EDIT Ah, it does that:

    selector = SDL_GetHint(SDL_HINT_EMSCRIPTEN_CANVAS_SELECTOR);
    if (!selector)
        selector = "#canvas";
    wdata->canvas_id = SDL_strdup(selector);

So the proposed API would be to add to GLFW backend:

#ifdef __EMSCRIPTEN__
void ImGui_ImplGlfw_SetEmscriptenCanvasSelector(const char*);
#endif

To be called after ImGui_ImplGlfw_InitXXXX().

And main.cpp would still perform the initial query prior to glfwCreateWindow(), but rest can be handled by backend automatically.

[ocornut]

@Traveller23 Would you be willing to work on this suggested change?

[Traveller23]

I didn't realize you were waiting for me to change it, I apologize for the delay of so many days. 😂
I'll do it in the next couple days.

[Traveller23]

I apologize for my delay, having had almost no breaks at all lately. I'll probably be free by the end of the month.

[Traveller23]

@ocornut All done, please check it out.
I also tried to solve the problem when switching to full-screen, but didn't find a perfect solution. There is an imperfect way, I wrote it in the comments, but didn't actually use it.

[ypujante]

I wanted to let you know that the changes I implemented in emscripten recently fixes this issue when canvas_width and canvas_height happen to be the fullscreen sizes. See ChangeLog | 3.1.49

The glfwSetWindowSize function no longer switches to fullscreen when the width/height provided as parameters match the screen size. This behavior now matches the behavior of SDL and glut. In order to switch to fullscreen, the client code should invoke Module.requestFullscreen(...) from a user triggered event otherwise the browser raises an error. (#20600)

[ypujante]

@Traveller23 @ocornut I wanted to let you know that my changes to add Hi DPI support to emscripten/glfw have been merged and will be released when emscriten 3.1.51 is completed. From the Changelog | 3.1.51

Added Hi DPI support to GLFW. When enabled, GLFW automatically accounts for the devicePixelRatio browser property and changes the size of the canvas accordingly (including dynamically if the canvas is moved from a 4k screen to a 2k screen and vice-versa). glfwGetFramebufferSize now properly returns the canvas size in pixels, while glfwGetWindowSize returns the canvas size is screen size. By default, this feature is disabled. You can enable it before creating a window by calling glfwWindowHint(GLFW_SCALE_TO_MONITOR, GLFW_TRUE). You can also dynamically change it after the window has been created by calling glfwSetWindowAttrib(window, GLFW_SCALE_TO_MONITOR, GLFW_TRUE). (#20584)

If you need my help adding this support to ImGui, please let me know. I can open a PR if you want me to. Note that I used example_emscripten_wgpu for some of my testing and it works fine.

[ocornut]

Hello @ypujante, I am not sure I follow.
Are you suggesting that upcoming Emscripten change would make this PR unnecessary?
Or should this PR be amended further to add/fix support for Fullscreen? In which case your amend would be welcome.

[ypujante]

@ocornut I am sorry I wasn't clear enough

1. the comments left in the code of this PR about "Permissions check failed" are now irrelevant since my changes to emscripten 3.1.49 and can/should be removed from the code. As per the Changelog:

The `glfwSetWindowSize` function no longer switches to fullscreen when the
  width/height provided as parameters match the screen size. This behavior
  now matches the behavior of SDL and glut. In order to switch to fullscreen,
  the client code should invoke `Module.requestFullscreen(...)` from a user 
  triggered event otherwise the browser raises an error. (#20600)

2. In my personal experience, I was able to handle fullscreen by doing something like this:

      if(ImGui::Button("Fullscreen"))
      {
        EM_ASM(
          const fs = document.getElementById("fullscreen");
          if(fs)
            fs.click();
          );
      }

<input id="fullscreen" type="button" value="Fullscreen" onclick="Module.requestFullscreen(false, true);" style="display: none;">

3. Finally, there are upcoming (imminent) changes to emscripten (3.1.51) to add HiDPI support to GLFW which has nothing to do with Fullscreen. I can submit a PR to add that support to ImGui once this PR is merged (since it touches the same files). It makes ImGui in the browser renders as nicely as on the desktop when the screen is 4k.

[Traveller23]

@ypujante

Great! for your first message, I just need to remove those two comments without tweaking any code.
(I tested it on emsdk 3.1.50, and it really doesn't report an error anymore when I press F11 to switch to full screen.)

For the second message, I really don't have experience with Hi DPI and really need some help from you.

[ypujante]

@Traveller23 No Problem. I am glad I can help.

For Hi DPI, no worries it is completely unrelated to your changes and I apologize for having brought it up. I will do a small PR once yours is merged (and emscripten 3.1.51 is official).

In regards to your changes for this PR, this is the right approach: use the canvas dimension to create the window of the proper size, then listens to window resize then use emscripten_get_element_css_size in the callback to call glfwSetWindowSize with these values (and these are the right values to read for when Hi DPI is involved) 👍

For fullscreen, you can check my previous comment on how I ended up making it work from ImGui (the trick is to generate a click event so that you don't trigger the permission error)

[ypujante]

(I tested it on emsdk 3.1.50, and it really doesn't report an error anymore when I press F11 to switch to full screen.)

Really glad to hear. That was the whole point of my PR on the emscripten project :)

[Traveller23]

@Traveller23 No Problem. I am glad I can help.

For Hi DPI, no worries it is completely unrelated to your changes and I apologize for having brought it up. I will do a small PR once yours is merged (and emscripten 3.1.51 is official).

In regards to your changes for this PR, this is the right approach: use the canvas dimension to create the window of the proper size, then listens to window resize then use emscripten_get_element_css_size in the callback to call glfwSetWindowSize with these values (and these are the right values to read for when Hi DPI is involved) 👍

For fullscreen, you can check my previous comment on how I ended up making it work from ImGui (the trick is to generate a click event so that you don't trigger the permission error)

I see your trick, but I don't know the difference between pressing F11 and calling Module.requestFullscreen() in the input event.
Maybe the former just sets the resolution to match the fullscreen resolution and is not really fullscreen (so it doesn't trigger the emscripten_set_fullscreenchange_callback())?

[ypujante]

Pressing F11 does not invoke Module.requestFullscreen() and I don't know what that does exactly. Probably your callback gets invoked and does the right thing. If you have tested it (on different browsers) and it works and does what is expected then that is great news.

If you want to offer a way for the user to go fullscreen that is in the ImGui application (for example a button, or a menu entry), then my trick works.

I have this example (my code from when I was working on Hi DPI, not your PR obviously, and I was not using emscripten_set_fullscreenchange_callback) which shows that if you click "Fullscreen" it goes full screen. But if you select "View/Enter Fullscreen" on Firefox for macOS it goes fullscreen but keeps the browser elements (toolbar, shortcut bar). Going Fullscreen with Chrome behaves differently as there is no browser elements remaining. Pressing the Fullscreen button goes fullscreen with no browser elements in both cases.

But maybe your implementation addresses this issue.

[Traveller23]

Pressing F11 does not invoke Module.requestFullscreen() and I don't know what that does exactly. Probably your callback gets invoked and does the right thing. If you have tested it (on different browsers) and it works and does what is expected then that is great news.

If you want to offer a way for the user to go fullscreen that is in the ImGui application (for example a button, or a menu entry), then my trick works.

I have this example (my code from when I was working on Hi DPI, not your PR obviously, and I was not using emscripten_set_fullscreenchange_callback) which shows that if you click "Fullscreen" it goes full screen. But if you select "View/Enter Fullscreen" on Firefox for macOS it goes fullscreen but keeps the browser elements (toolbar, shortcut bar). Going Fullscreen with Chrome behaves differently as there is no browser elements remaining. Pressing the Fullscreen button goes fullscreen with no browser elements in both cases.

But maybe your implementation addresses this issue.

Thank you, I'll remove those two comments first, and then test the fullscreen effect in Firefox on Windows over the weekend (I only have windows computers).

[Traveller23]

Hmm... In Firefox 120.0.1 navigator.gpu is undefined, so the test doesn't work at all...

[Traveller23]

Without adding additional buttons, I think the current code is the best we can do. What do you think? @ypujante @ocornut

[ypujante]

I suppose so.

[Traveller23]

But ocornut does want it:

Then your implementation does what he wants to do (I was not aware of this comment, but it does make sense). The reality is what you implemented is exactly what needs to happen for emscripten. emscripten is more complicated than "desktop" because of the way that it ties to a canvas.

I am not sure that shoving the logic in ImGui_ImplGlfw_SetEmscriptenCanvasSelector so that the opengl3 example is "cleaner", is the right approach because this method is used by everybody. And if somebody does not want a full window then this call will force it to full window...

You are right, not everyone needs a full window canvas.

[Traveller23]

The core problem is that glfwCreateWindow must not specify an arbitrary size, the size must be obtained via the emscripten method.

But if the point of the change is to support resolution change, could the event be automatically made to trigger?

Updated. Please check it out.

A negative aspect of this implementation is that the size of the GLFW window will be changed one more time when the page is initialized.

[ypujante]

Wouldn't it make more sense to rename ImGui_ImplGlfw_SetEmscriptenCanvasSelector to ImGui_ImplGlfw_EmscriptenInit?

And maybe provide a flag as an argument, that way you can still use a fixed size canvas (by passing false)?

void ImGui_ImplGlfw_EmscriptenInit(const char *canvas_selector, bool use_canvas_size_as_window_size = true)
{
    IM_ASSERT(canvas_selector != nullptr);
    ImGui_ImplGlfw_Data* bd = ImGui_ImplGlfw_GetBackendData();
    IM_ASSERT(bd != nullptr && "Did you call ImGui_ImplGlfw_InitForXXX()?");
    bd->CanvasSelector = canvas_selector;

    emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, bd, false, ImGui_ImplGlfw_OnCanvasSizeChange);
    emscripten_set_fullscreenchange_callback(EMSCRIPTEN_EVENT_TARGET_DOCUMENT, bd, false, ImGui_ImplGlfw_OnFullscreenChange);

    // Change the size of the GLFW window according to the size of the canvas
    if(use_canvas_size_as_window_size)
      ImGui_ImplGlfw_OnCanvasSizeChange(EMSCRIPTEN_EVENT_RESIZE, {}, bd);
}

[ocornut]

A negative aspect of this implementation is that the size of the GLFW window will be changed one more time when the page is initialized.

It seems good to me at least from API point of view.
What's the problem with that? It is any visible for the user?

Wouldn't it make more sense to rename ImGui_ImplGlfw_SetEmscriptenCanvasSelector to ImGui_ImplGlfw_EmscriptenInit?

One tricky aspect is that we already have ImGui_ImplGlfw_InitForXXXX() functions relating to rendering backends.
So it would likely need to be an orthogonal pre-init or post-init function.

I'm really overwhelmed with tasks and don't know those tech very well, if you want to help further I'd need people to be more thorough with 1) the overall design work. 2) conveying the "whys" to me so I'm easily convinced a solution is the right one.

[Traveller23]

Wouldn't it make more sense to rename ImGui_ImplGlfw_SetEmscriptenCanvasSelector to ImGui_ImplGlfw_EmscriptenInit?

And maybe provide a flag as an argument, that way you can still use a fixed size canvas (by passing false)?

void ImGui_ImplGlfw_EmscriptenInit(const char *canvas_selector, bool use_canvas_size_as_window_size = true)
{
    IM_ASSERT(canvas_selector != nullptr);
    ImGui_ImplGlfw_Data* bd = ImGui_ImplGlfw_GetBackendData();
    IM_ASSERT(bd != nullptr && "Did you call ImGui_ImplGlfw_InitForXXX()?");
    bd->CanvasSelector = canvas_selector;

    emscripten_set_resize_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, bd, false, ImGui_ImplGlfw_OnCanvasSizeChange);
    emscripten_set_fullscreenchange_callback(EMSCRIPTEN_EVENT_TARGET_DOCUMENT, bd, false, ImGui_ImplGlfw_OnFullscreenChange);

    // Change the size of the GLFW window according to the size of the canvas
    if(use_canvas_size_as_window_size)
      ImGui_ImplGlfw_OnCanvasSizeChange(EMSCRIPTEN_EVENT_RESIZE, {}, bd);
}

I've considered this approach as well, but it only makes sense at initialization time. After that the GLFW window size would still be changed whenever the canvas size is changed.
Unless we set a parameter, when it is true, the size of the GLFW window will not be changed at any time.
I'll ask @ocornut if this is acceptable.

[ypujante]

Sorry to have bothered in this effort. I won't post anymore.

[Traveller23]

It seems good to me at least from API point of view. What's the problem with that? It is any visible for the user?

I tested it on Chrome v120 on Windows, there is no flickering visible to the naked eye.
Theoretically this only slows down the initialization slightly.

[Traveller23]

Sorry to have bothered in this effort. I won't post anymore.

I really appreciate your advice and hope you continue to give it. 👍

[ocornut]

Theoretically this only slows down the initialization slightly.

Not meaningfully at all. I'm happy to merge as-is now.

Sorry to have bothered in this effort. I won't post anymore.
I really appreciate your advice and hope you continue to give it. 👍

Me too. I didn't intend that and apologies if it was misunderstood.

People seems to miss-comprehend that my attention is stretched between HUNDREDS of simultaneous issues. And yet, I keep finding issues, incorrect reasoning or lack of explanations in a vast majority of pull requests and suggestion. The best way anyone can help a project is to give it increased attention and care for details. I tried to convey it in https://github.com/ocornut/imgui/blob/0d582dabf34e9e31f072b1ee5c353c18351b4424/docs/CONTRIBUTING.md#how-to-open-a-pull-request :

"Please understand that by submitting a PR you are also submitting a request for the maintainer to review your code and then take over its maintenance [forever]. PR should be crafted both in the interest of the end-users and also to ease the maintainer into understanding and accepting it."

I don't know much about Emscripten myself! But I know enough to know that in order to trust a PR the people involved with it need to explain it to me and gives me confidence this is the best possible solution at a given time, meaning they explored possibility space and the trade-off of each solutions. Sometimes the difference between a passerby spending 5 minutes on a topic vs 20 minutes on same topic is that the later is more likely to give us the right solution that fits imgui and current constraint, and easier for me to understand and accept.

It's mostly a result of me being overwhelmed that I have a tendency to ask more thoughful work of other people, sort of a survival technique.

[ocornut]

"Is this code really necessary for the opengl3 example? Isn't this code used for webgpu?"

Curious about the answer for this?

Could you also recap:

• why is the new index.html needed
• how does it differ from the one in example_emscripten_wgpu/web/index.html ? do both need to be synched going forward?
• a general recap about the presence of examples/libs/emscripten/shell_minimal.html vs example_emscripten_wgpu/web/index.html vs example_glfw_opengl3/web/index.html

I'm ok merging PR as is now if you are confident we can followup with a cleanup/clarification of those.
EDIT In fact, I would prefer to merge this now, so we clear one bite-sized work from the queue, but I'd appreciate the followup in figuring out how to clean those.

[Traveller23]

• why is the new index.html needed

We can't use the index.html that emscripten generates by default, otherwise the size of the canvas would be fixed and there would be elements on the page that we don't want to see.

• how does it differ from the one in example_emscripten_wgpu/web/index.html ? do both need to be synched going forward?

They are identical except for the different titles of the pages.

$ diff example_emscripten_wgpu/web/index.html example_glfw_opengl3/web/index.html
6c6
<     <title>Dear ImGui Emscripten+WebGPU example</title>
---
>     <title>Dear ImGui Emscripten+GLFW+OpenGL3 example</title>

• a general recap about the presence of examples/libs/emscripten/shell_minimal.html vs example_emscripten_wgpu/web/index.html vs example_glfw_opengl3/web/index.html

I didn't know “examples/libs/emscripten/shell_minimal.html” existed before, I'll try to see if I can use it directly as example_glfw_opengl3‘s html.
Give me a few minutes.

[Traveller23]

@ocornut "examples/example_glfw_opengl3/web/index.html" has been deleted, I modified the make file a little bit, and everything works fine.

[ocornut]

Thanks a lot. Then I'm confused as to why example_emscripten_wgpu used its own. I recall at some point we commented out some section because it caused problem with GLFW.
Now the example_emscripten_wgpu copy adds an async() block. Presumably they could all use the same now? With async in the shared template?

[Traveller23]

Now the example_emscripten_wgpu copy adds an async() block. Presumably they could all use the same now? With async in the shared template?

I'm not quite sure of the significance of the changes in "example_emscripten_wgpu/web/index.html".
But tried example_emscripten_wgpu compiled with "examples/libs/emscripten/shell_minimal.html" and it results in an error.

[Traveller23]

@ypujante Do you know the reason?

[ypujante]

In regards to @ocornut comments about PR, etc... these are fair points. I will try to do my best to be more explicit. FTR I have been working on emscripten/glfw3 for a few months now so I am fairly familiar with the issues discussed in this PR.

1. In regards to the naming, I feel like ImGui_ImplGlfw_SetEmscriptenCanvasSelector is not the right name because it does multiple things:

• it adds the canvas selector to bd (so that it can be retrieved by the callbacks)
• it sets 2 callbacks (one for size change and one for fullscreen)
• it invokes the callback to override whatever size is provided when creating the window
  => this call is only necessary if the user wants a resizable canvas and if the user is fine with a fixed size canvas, then this call does not have to be done. Maybe ImGui_ImplGlfw_EmscriptenEnableResizableCanvas would describe better what this call actually does. But if you are fine with the name/what it does, I am ok with it. ImGui will still work if I don't call it and handle it myself in main

2. In regards to my comment about the webgpu initialization, I don't have much context and it is hard from the PR since the entire code is not included. What I do know is webgpu and opengl are different things. That is why I was wondering if the section in the file shell.html was a copy/paste from example_emscripten_wgpu or if it was a legitimate use.

3. In regards to your question "Do you know the reason?", then I believe it is tied to my point 2. example_emscripten_wgpu uses webgpu and index.html for example_emscripten_wgpu initializes it. If you use shell_minimal.html then this initialization is now missing and it will fail.

[Traveller23]

3. In regards to your question "Do you know the reason?", then I believe it is tied to my point 2. example_emscripten_wgpu uses webgpu and index.html for example_emscripten_wgpu initializes it. If you use shell_minimal.html then this initialization is now missing and it will fail.

So emscripten doesn't implicitly initialize webgpu, we have to write our own code to do that, right?
And then because of the above, we also have to use special code in example_emscripten_wgpu instead of a generic shell_minimal.html, right?

[ypujante]

The emscripten/glfw implementation is doing what glfw3 specifies:

• if you define the window hint glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API) (which example_emscripten_wgpu defines), then no context is created for you and you must create the context yourself.
• if you don't define this hint, it defaults to GLFW_OPENGL_API and as a result a context will be created for you, which in the case of emscripten calls canvas.getContext('webgl', ...) or canvas.getContext('webgl2', ...) depending on if you compile with -sMIN_WEBGL_VERSION=2

From what I can see in main.cpp for example_glfw_opengl3 it does not use the GLFW_NO_API hint and as a result the context should be created for you and as a result it does not use webgpu so it seems that my earlier comment about the webgpu code in shell.html for this example seems to be justified.

Because example_emscripten_wgpu is defining GLFW_NO_API and as the name of the example implies, it uses webgpu, it is the responsibility of the code to instantiate webgpu hence the section in shell.html which initializes it. The function wgpuInstanceCreateSurface which ends up being called is the one creating the context canvas.getContext('webgpu', ...)

[Traveller23]

The emscripten/glfw implementation is doing what glfw3 specifies:

• if you define the window hint glfwWindowHint(GLFW_CLIENT_API, GLFW_NO_API) (which example_emscripten_wgpu defines), then no context is created for you and you must create the context yourself.
• if you don't define this hint, it defaults to GLFW_OPENGL_API and as a result a context will be created for you, which in the case of emscripten calls canvas.getContext('webgl', ...) or canvas.getContext('webgl2', ...) depending on if you compile with -sMIN_WEBGL_VERSION=2

From what I can see in main.cpp for example_glfw_opengl3 it does not use the GLFW_NO_API hint and as a result the context should be created for you and as a result it does not use webgpu so it seems that my earlier comment about the webgpu code in shell.html for this example seems to be justified.

Because example_emscripten_wgpu is defining GLFW_NO_API and as the name of the example implies, it uses webgpu, it is the responsibility of the code to instantiate webgpu hence the section in shell.html which initializes it. The function wgpuInstanceCreateSurface which ends up being called is the one creating the context canvas.getContext('webgpu', ...)

I got it, thanks.
It does seem that we can't use shell_minimal.html in example_emscripten_wgpu.

[ocornut]

In regards to the naming, I feel like ImGui_ImplGlfw_SetEmscriptenCanvasSelector is not the right name because it does multiple things:

I agree it could be renamed (and noticed you suggested that earlier and I skimmed through it, sorry).

this call is only necessary if the user wants a resizable canvas and if the user is fine with a fixed size canvas, then this call does not have to be done.

Is the call harmful is the user decide to have a fixed size canvas?

How about:

IMGUI_IMPL_API bool     ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback(....);

Not overly-worried with occasionally breaking Emscripten-specific API in the future if we need it too, as it's quite moving technology.

However I'm a little bit lost with the remaining .html stuff.
The main intent was to make our demos look closer to a fullscreen-ish application, which IMHO looks much more attractive (vs Emscripten default template which quite terrible in this regard).

[Traveller23]

@ocornut I have no further questions and I think you can proceed with the merger.

After that a new PR can be opened to deal with the naming and other issues we are discussing now.

[ocornut]

Thank you!
For reduce back and forth and I have merged the squashed PR as 22a7d24 + added amends 3cb8054 (coding style, function name, unnecessary include leftovers, misc comments).

[ypujante]

The main intent was to make our demos look closer to a fullscreen-ish application, which IMHO looks much more attractive (vs Emscripten default template which quite terrible in this regard).

100% agree on that! 👍