Change emscripten GLFW to use contrib port (GLFW 3.4.0 / Bug fixes)

[ypujante]

Hi @ocornut

This PR is a follow up of the one I submitted then later closed several months ago (#7151).

Big picture what does this PR do?

This PR swaps the emscripten built-in/javascript implementation of GLFW for the contrib/c++ version.

Why?

The emscripten built-in implementation has been implemented many years ago and has not been kept up to date (I personally added support for 4K recently but that is pretty much it). There are many features missing. It is also buggy, as the Joystick/Gamepad code does not work at all with ImGui.

The contrib/c++ version (disclaimer: I am the author), is a modern implementation of GLFW 3.4.0 with the primary goal of being as close as possible to the desktop version (to the extent possible in a browser context obviously). This implementation fixes all the issues that are present in the javascript version (including joystick). This implementation has also a fairly extensive test/demo to make sure the features are working (you can check it out: Live Demo)

Note that the emscripten SLD2 implementation is an external port as well and this new implementation of GLFW is simply following a similar pattern.

Changes to ImGui

The changes in terms of building are trivial because it is now integrated with emscripten (since emscripten 3.1.55 released on 2024/03/01: I also did the work necessary in emscripten to enable contrib ports/--use-port syntax): instead of -sUSE_GLFW=3 you use --use-port=contrib.glfw3 (but it needs to be added to compilation as well)

I updated the 2 examples using emscripten/glfw: example_glfw_opengl3 and example_glfw_wgpu (that you just renamed... and I also changed the CMakeLists.txt there as well)

The bulk of the changes are in imgui_impl_glfw.cpp and in the end they greatly simplify this code because this new implementation simply conforms better to the API so no need to do something special in the case of emscripten. It also has some extensions (like emscripten_glfw_make_canvas_resizablewhich implements all the necessary logic to make the canvas properly resizable (full window or other...))

Note that this new implementation obviously support 4K/Hi DPI and is the default (like SDL2 BTW).

Is there any drawback?

The only drawback I can think of at the moment is that it requires to use emscripten 3.1.55+ and because of the changes applied in imgui_impl_glfw.cpp to remove the old code dealing with the other implementation, if somebody were to use -sUSE_GLFW=3 it will actually not compile anymore. The fix is simple (use --use-port=contrib.glfw3) but it might surprise users and I don't know how you feel about this.

Live Demo for this PR (example_glfw_wgpu)

• This is the live demo - imgui-master-b720c0f for the code before the changes
• This is the live demo - ypujante-master-fd249f9 for the code with the changes

In particular if you have access to a gamepad, you should try it and see that the gamepad does not really work in the first demo and it works perfectly in the second one. Also if you are on 4k monitor you should see that the second demo is 4K.

[ocornut]

Hello,

Thanks for the PR.

What's not simple about this is that we'd requires every project using imgui_impl_glfw to switch to the contrib port, and there's a bit of a chicken and egg problem. I shouldn't force dear imgui users if I don't have clear confidence this is going to be working for them.

We should do a little investigation on project using imgui + GLFW backend on the web, e.g.
https://github.com/wolfpld/tracy

Do you think you can help convincing them to switch?
I believe if Tracy switched and confirmed "this works well" I would be more inclined to accept the change.

Suggestion, any way to detect the port and facilitate the discovery of that lib update?
e.g. call glfwGetError() on init on emscripten, and add a comment/message reminding user they should use the port.
The local changelog inside backend code should also be updated accordingly (I will update the global changelog when merging)

[ypujante]

Thank you for your response.

I totally understand your chicken and egg problem.

I do not know anything about the Tracy project that you mention, but from my cursory glance at their project they indeed use ImGui/GLFW. Because they have a copy of imgui_impl_glfw.cpp I cannot just tell them to switch to using the contrib port. I would have to also give them the patched version (the one in this PR) and convince them somehow to use it. Which seems like a tall order for me not even knowing this project in the first place.

As I mentioned in the description above, the primary reasons to move forward with the contrib port are:

• joystick is working (which wouldn't be an issue for anybody not caring about joystick support like Tracy...)
• features from GLFW 3.4.0 (like all cursors)
• big cleanup of the ImGui code

I suppose, given your comment "any way to detect the port and facilitate the discovery of that lib update?", maybe the right approach would be to detect the usage of the built-in port and act accordingly, so it is backward compatible. That means that there is no cleanup of the ImGui code since all the #ifdef would still have to be there.

I will investigate this approach, as I feel like this is a more reasonable way forward.

All that being said, it is ok to reject the PR if you feel like this is not something you want to move forward with for ImGui. Just let me know.

As a side note and for your information, I have already written a ImGui emscripten port file, so that you can simply use ImGui without downloading it at all (the entire project is 2 files: main.cpp and imgui.py, the port file), like this:

emcc --use-port=imgui.py:backend=glfw:renderer=wgpu:branch=docking main.cpp -o /tmp/test/index.html

I will obviously share it and make it public at some point in the future, because I think that is really neat...

[ypujante]

Hi @ocornut

I just wanted to provide you with an update. I have submitted a PR to emscripten to add a compile time option in order to detect if -sUSE_GLFW=3 or --use-port=contrib.glfw3 is used.

I have already worked on the changes in ImGui to account for this new compile time option and make it so that users don't have to be forced into using one or the other which I think is the right approach in the end.

That being said, the emscripten PR needs to be merged and then released so that is going to take a while and I do not have any control over the timing. When this happens I will update this PR with the new changes for you to review.

[Jeremy-Boyle]

Thank you for this !

I'm using this currently its a lot better than the current implementation!

Was driving me crazy, I was considering switching backends, thank you so much !

Going to maintain my own private port until this is merged into the docking branch and main.

[ypujante]

Hi @Jeremy-Boyle. I am very glad to hear that it is working for you. Do you mind posting the question you asked on the emscripten PR on the official project (pongasoft/emscripten-glfw) instead. Thank you!

[ocornut]

I do not know anything about the Tracy project that you mention, but from my cursory glance at their project they indeed use ImGui/GLFW. Because they have a copy of imgui_impl_glfw.cpp

The copy in Tracy seems vanilla unmodified, it's just a copy of imgui repository (I just verified).
I realize it may seem daunting but Tracy maintainer is quite reactive and often interacted with dear imgui and pushing for improvements. So my guess is that he may be supportive of helping moving the needle forward here.

Again the chicken-and-egg problem is that regardless of knowing that your version is better on some aspect, it's difficult for me to judge if this is a one-man-project that's going to live for 6 months and disappears or if it is the future of using GLFW on Emscripten. Hopefully the later! But difficult for me to push my users toward a new thing I don't know much about.
It's also impossible to prove that your version has no other issues, so it's a bit of a leap of faith.

I imagine your end game is probably that your version becomes the official port, then this wouldn't be an issue.

I guess the new route you outlined is the pragmatic one. I know it's not ideal and frankly I would be more than happy to remove those ifdef and hacks, but I guess in the meanwhile the best intermediate step is to support both and wait for your version to take over :)
As mentioned in one of the comment, I wonder why you aren't just adding an extra define in your version of the GLFW? That seems simpler?

[ocornut]

The remaining tricky part is to probably find a way to standardize the ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback()/ImGui_ImplGlfw_EmscriptenCanvasSetup(). I'm not too bothered about breaking this API, that would be fine. But I'd be preferable to have one API rather than two.

• Why passing a GLFWWindow* as we got one in the Init() call?
• What's the "window" literal in your new function?
• I'm quite vary of multiple bools in API (in my experience this almost always led to trouble) they might be better as a structure o flags... but...
• Also wary of any claim of hi-dpi support because that can mean twelve different things. If this only materialize as a call to glfwSetWindowAttrib() it could be moved to app/main.cpp ?
• Ditto for the call emscripten_glfw_make_canvas_resizable() wouldn't that be better done in app/main.cpp?

While the old ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback() seemed obligatory in our context, it seems like nothing in ImGui_ImplGlfw_EmscriptenCanvasSetup() is obligatory as they are wrappers for simple thing? Let me know if you think I am wrong.

[ypujante]

Hi @ocornut

You are raising some very valid points and let me try to address them:

• in regards to Tracy, I still think it is a bit of a stretch because in order for this project to use my port, they would have to use the patched version of ImGui. So this is the true chicken and egg problem. But I will try to reach out. I just want to point out that comments like @Jeremy-Boyle is showing the advantage of the new port.
• yes this is a 1 man project and yes I can die tomorrow. The project is 100% open source and I believe fairly well written/understandable (like ImGui code BTW) and far smaller in scope than ImGui. It is also a very different kind of project: it is implementing an API that changes rather slowly. So besides fixing issues if there are any, and upgrading to the latest version when there is one, it is really not a huge amount of work unlike ImGui that is constantly evolving.
• I also agree that there might be different bugs. But let's be honest, the emscripten built-in GLFW3 implementation is buggy and has not been maintained or enhanced in years (except by me to add support for 4k). When I was working on the port, I actually used ImGui as my "not built by me" test bed to test that my code was working properly because I knew that ImGui was using GLFW the way it is intended to be used (unlike raylib for example). And I did find issues with the joystick implementation thanks to that :)
• I agree 100% with the pragmatic approach. It was an oversight on my part. ImGui should not impose which implementation of GLFW is used. I just need to find the right way to determine at compilation time which implementation is being used (working on the PR on the emscripten side right now)
• The remaining tricky part you are talking about is actually not tricky and you are 100% right in your conclusion: the introduction of the API ImGui_ImplGlfw_EmscriptenCanvasSetup was just me trying to make it easier to transition. In my latest changes to this PR (that I have not pushed yet because I am waiting on the final solution with emscripten), I have removed it entirely. The API emscripten_glfw_make_canvas_resizable can simply be called from main.cpp. Note that, in the end emscripten_glfw_make_canvas_resizable (when provided with the "window" argument) does something very similar to ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback. So using this API is still ok.
• The "window" argument in the API is defined in the documentation. It's because I actually support 3 different mode out-of the box, all variants of handling resizing which is necessary but not trivial to implement/get right:
  • full browser window (live demo)
  • resizable not full window (live demo)
  • resizable with a handle (live demo)

As a final note, I was really happy to see @Jeremy-Boyle comment as it shows that people have bumped into the same issues that made me do this work in the first place. That being said, I am just going to repeat it, it is OK to not integrate it in ImGui directly. It is also OK if you would rather wait to see if there is traction with my project before you make a decision. There are other ways to make it available (the ImGui emscripten port I mentioned in my prior comment can patch the ImGui code after downloading it for example which is what I am doing for another project I am working on).

[ypujante]

Hi @ocornut

The emscripten PR has been merged to main. I do not know how long it will take to be released but I am assuming it will make it in the next release (3.1.59). So I marked this PR as draft in the meantime.

I just pushed the latest changes if you want to take a look at it. What I did is the following:

• imgui_impl_glfw.cpp is now deciding which path to use based on the define EMSCRIPTEN_USE_PORT_CONTRIB_GLFW3 being set. This define will be automatically set when using --use-port=contrib.glfw3 with emscripten 3.1.59+. That being said, it can be manually set for prior versions (see below)
• I modified example_glfw_wgpu/main.cpp and example_glfw_opengl3/main.cpp to use the same define in order to do the proper call (emscripten_glfw_make_canvas_resizable vs ImGui_ImplGlfw_InstallEmscriptenCanvasResizeCallback) which is what you were worrying in your last message
• I modified the newly created example_glfw_wgpu/CMakeLists.txt to have logic to automatically detect if --use-port=contrib.glfw3 is available and it it is, use it. There is a way to override this behavior. Note that this will work even with emscripten prior to 3.1.59 because I add the define EMSCRIPTEN_USE_PORT_CONTRIB_GLFW3 manually. So I guess technically we do not have to wait until 3.1.59 is released, but it is your call...
• I left both Makefile.emscripten unchanged so they will continue using -sUSE_GLFW3 (mostly because the CMakeLists.txt logic would be pretty hard to implement in Makefile... and I am not an expert in Makefile...)

I don't know if this is what you had in mind, but I believe it should cover all the use cases we discussed. Of course I am open to any changes/concerns you might have.