-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Change emscripten GLFW to use contrib port (GLFW 3.4.0 / Bug fixes) #7520
Conversation
- greatly simplify the code in ImGui - works correctly (ex: gamepad)
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. Do you think you can help convincing them to switch? Suggestion, any way to detect the port and facilitate the discovery of that lib update? |
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 As I mentioned in the description above, the primary reasons to move forward with the contrib port are:
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 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:
I will obviously share it and make it public at some point in the future, because I think that is really neat... |
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 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. |
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. |
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! |
The copy in Tracy seems vanilla unmodified, it's just a copy of imgui repository (I just verified). 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. 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 :) |
The remaining tricky part is to probably find a way to standardize the
While the old |
Hi @ocornut You are raising some very valid points and let me try to address them:
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). |
- automatically detect if contrib port is available and use by default - can be turned off
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:
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. |
I opened a new PR for the work. See #7647 |
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
andexample_glfw_wgpu
(that you just renamed... and I also changed theCMakeLists.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 (likeemscripten_glfw_make_canvas_resizable
which 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)
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.