[emscripten + glfw]: Added support for GLFW3 contrib port

[ypujante]

This is the PR implementing the changes as discussed in PR #7520. Due to the changes + merge conflicts, I decided to start clean and do a brand new PR.

As discussed, this PR adds support for the GLFW3 contrib port, but does NOT force the user to switch to it.

• I have not changed Makefile.emscripten in example_glfw_wgpu and example_glfw_opengl3 and as a result, building using these Makefiles, will build the same code as before
• I have changed CMakeLists.txt for example_glfw_wgpu to automatically detect the version of emscripten used and if it can use the new port, it uses it: this gives an example on how to build with the new port
• As discussed I also took the opportunity to fix Emscripten build runtime error: Unable to preventDefault inside passive event listener #7600 (using the canvas selector as opposed to the document addresses the issue)

The most notable changes visible when using the GLFW3 contrib port are (which can be tested with the live demos)

• the gamepad is now working properly
• copy to clipboard works (Menu: Tools / About Dear ImGui / Config/Build Configuration / Copy to clipboard)
• Hi DPI by default

Live demos:

• Build using -sUSE_GLFW=3: demo
• Build using --use-port=contrib.glfw3: demo

[ocornut]

Thank you for updating this.

EMSCRIPTEN_USE_GLFW3 seems a little confusingly named. Maybe I would use a more explicit name:
EMSCRIPTEN_USE_PORT_CONTRIB_GLFW3 vs EMSCRIPTEN_USE_EMBEDDED_GLFW3 ?

I'll read the rest when I have some time.

[ypujante]

I agree it is a bit confusing. I renamed it per your suggestion.

[ocornut]

• Why is emscripten_set_wheel_callback called in the ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback() function?
• I think the printf("") may be replaced by an assert() with comments.

[ypujante]

• Why is emscripten_set_wheel_callback called in the ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback() function?

You asked me to fix the #7600 issue for the embedded path and the fix is to call emscripten_set_wheel_callback with the selector. The prior code was calling emscripten_set_wheel_callback with a static value (for the document) and was being called in ImGui_ImplGlfw_Init. There is no access to the selector (in the embedded path) until you call ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback() while providing it as a parameter. So that is the reason why I had to move it here.

* I think the `printf("")` may be replaced by an assert() with comments.

Ok I will change it.

[ocornut]

Thanks! I didn't realize this was the fix for #7600. I'll try to test this now.

[ocornut]

I wonder if we should:

• rename ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback() to ImGui_ImplGlfw_InstallEmscriptenCallbacks()
• we can easily add an inline redirect under #ifndef IMGUI_DISABLE_OBSOLETE_FUNCTIONS block.
• KEEP calling ImGui_ImplGlfw_InstallEmscriptenCallbacks() even for your port. We disable the code inside imgui_impl_glfw.cpp but it seems sane/safe to request users to call this so we have a backup path if ever some function in your port requires extra code?

What is the "window" string passed to emscripten_glfw_make_canvas_resizable() ? is that part of the template?

Would it make some sense to ask both window and canvas names in ImGui_ImplGlfw_InstallEmscriptenCallbacks() and do that automatically?

[ypujante]

I made the change you requested printf -> assert.

I made it as a static_assert because the runtime assert was pretty horrible from a user perspective

At least with the static_assert the error message is at compilation time and add some detail:

../../backends/imgui_impl_glfw.cpp:867:19: error: static assertion failed: [ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback] When using --use-port=contrib.glfw3, you should include <GLFW/emscripten_glfw3.h> and call emscripten_glfw_make_canvas_resizable instead
  867 |     static_assert(false, "[ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback] When using --use-port=contrib.glfw3, you should include <GLFW/emscripten_glfw3.h> and call emscripten_glfw_make_canvas_resizable instead");
      |                   ^~~~~

In regards to your last questions:

• rename ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback() to ImGui_ImplGlfw_InstallEmscriptenCallbacks()
• we can easily add an inline redirect under #ifndef IMGUI_DISABLE_OBSOLETE_FUNCTIONS block.
• KEEP calling ImGui_ImplGlfw_InstallEmscriptenCallbacks() even for your port. We disable the code inside imgui_impl_glfw.cpp but it seems sane/safe to request users to call this so we have a backup path if ever some function in your port requires extra code?

I don't think I have an opinion one way or another. I don't envision the port to ever require something that would need to be added in ImGui_ImplGlfw_InstallEmscriptenCallbacks because it would mean that every other (non ImGui) projects my port is used would require this callbacks to be installed as well and I don't think that would be good for the end user.

But it doesn't cost much to do it the way you suggest and I am fine with it if that is the direction you want to go with.

What is the "window" string passed to emscripten_glfw_make_canvas_resizable() ? is that part of the template?

The documentation explains in detail the parameters provided to this function which can handle 3 different use cases. The "normal" use case for ImGui is to use the full window, and this corresponds to the css selector "window". As you can see in my WIP videos on a new project I am working on, (WIP1 and WIP2), in this instance I DON'T use "window" but the canvas selector itself, thus having ImGui run in a resizable frame in the browser.

Would it make some sense to ask both window and canvas names in ImGui_ImplGlfw_InstallEmscriptenCallbacks() and do that automatically?

I think the issue is that, as I mentioned, with the new API you don't necessarily use the full window, so I am not sure how the user could do that. And how would you pass the 3rd parameter (which is the handle to resize the canvas when you want one)?

[ypujante]

After spending more time thinking about your proposed changes (add ImGui_ImplGlfw_InstallEmscriptenCallbacks and deprecate ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback) I tried to implement it and I think I really like the suggestion.

It makes it easier for regular ImGui user. Although I was worried about the fact that the emscripten_glfw_make_canvas_resizable allow for 3 different use cases (that are not covered with this solution), the reality is probably 99.9% of ImGui users will not care about not being full window. For the remaining users who want a more advanced control, this solution will still work as a user can write something like this:

// main.cpp
#ifdef __EMSCRIPTEN__
#include <GLFW/emscripten_glfw3.h>
#endif
// ....
#ifdef __EMSCRIPTEN__
    ImGui_ImplGlfw_InstallEmscriptenCallbacks(window, "#canvas");  // this will make it full window
    emscripten_glfw_make_canvas_resizable(window, "#xxx", "#yyy"); // this will make it resizable via #xxx and handle #yyy
#endif

In other words, my code supports calling emscripten_glfw_make_canvas_resizable multiple times and does the right thing...

In conclusion, I would go for this solution which covers 99.9% of the use cases while still allowing to use the more advanced usage. It is less discoverable, but I am planning to write examples demonstrating it (including open sourcing the project I linked to in my previous message).

I marked ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback deprecated which then raises a nice warning during compilation:

main.cpp:108:5: warning: 'ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback' is deprecated: Use ImGui_ImplGlfw_InstallEmscriptenCallbacks instead [-Wdeprecated-declarations]
  108 |     ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback("#canvas");
      |     ^
../../backends/imgui_impl_glfw.h:35:3: note: 'ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback' has been explicitly marked deprecated here
   35 | [[deprecated("Use ImGui_ImplGlfw_InstallEmscriptenCallbacks instead")]]
      |   ^
1 warning generated.

[ypujante]

Hi @ocornut, I just found out an issue which is reproducible in BOTH the old/embedded path as well as the new one on macOS

• go to Demo/Inputs & Focus
• Hit Meta (Super)
• While holding Meta, Press V
• Release V
• Release Meta

"V" is still pressed

I am going to investigate of why this is happening and I will let you know what I find.

[ypujante]

As I pointed out in my comment:

• the issue exists with the built-in/embedded path
• I have issued an emscripten PR to use the workaround implemented for the new path

Since this problem existed before I don't see a reason why this should prevent this PR to be merged (unless there are other reasons to prevent the merge of course).