Skip to content
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

Add new (external) port for GLFW #21148

Closed
wants to merge 5 commits into from

Conversation

ypujante
Copy link
Contributor

Hi @sbc100 @juj.

After your suggestion, I spent quite some time working on a full port of GLFW. I must admit it was a pretty fun project.

I have implemented a huge amount of the GLFW api including support for multiple windows. I have a fairly comprehensive documentation including details on how GLFW interacts with the canvas, how to do fullscreen, HI DPI, joystick, etc...

There is also a fair amount of live examples, including the main demo

This PR is to add this external port as a port in emscripten so that users can simply use -sUSE_GLFW_PORT=3 and they are all set.

For the sake of this PR, I did the following:

  • added USE_GLFW_PORT (but I am open to any other name you would be more comfortable with)
  • added a test that demonstrate that the port downloads properly and that the includes are properly set
  • note that I had to change the order in tools/link.py because adding a js library in linked_setup was being ignored otherwise

Please let me know what you think.

Thank you!

@ypujante
Copy link
Contributor Author

I am not sure what is going on with test-browser-firefox: there are many failures and for example I can see this failure

FAIL [17.493s]: test_html5_gamepad (test_browser.browser)

If I run it locally:

EMTEST_BROWSER=/Applications/Firefox.app/Contents/MacOS/firefox ./test/runner browser.test_html5_gamepad
Test suites:
['test_browser']
Running test_browser: (1 tests)
(checking sanity from test runner)
shared:INFO: (Emscripten: Running sanity checks)
[Browser harness server on process 8375]
common:INFO: Launching browser: ['/Applications/Firefox.app/Contents/MacOS/firefox']

Running the browser tests. Make sure the browser allows popups from localhost.

test_html5_gamepad (test_browser.browser) ... []
127.0.0.1 - - [24/Jan/2024 08:10:06] code 404, message File not found
['-O2', '-g1', '--closure=1']
['-pthread', '-sPROXY_TO_PTHREAD']
ok
[Browser harness server terminated]

----------------------------------------------------------------------
Ran 1 test in 38.043s

OK

I ran a few other tests locally that fail in CI and they all pass...

I know I did make a change to tools/link.py which could have an impact, but somehow I don't think that it is the reason for these failures since it works locally with this change.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 24, 2024

The browser tests can be flaky so just re-running them will probable work in this case.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 24, 2024

I'm not sure we want to officially support another version of GLFW in emscripten proper.

Is there any reason this ports shouldn't replace the existing one? That would avoid increasing the testing and maintenance burden.

If we are going to have this port live outside of the emscripten repo (i'm not suggesting we should or shouldn't do this), is there any reason it shouldn't actually live upstream in https://github.com/glfw/glfw?

@ypujante
Copy link
Contributor Author

Thank you for getting back to me.

I'm not sure we want to officially support another version of GLFW in emscripten proper.

Is there any reason this ports shouldn't replace the existing one? That would avoid increasing the testing and maintenance burden.

The reason is that I do not know, and I think it would be hard to know, whether my implementation differs from the existing one. I have added much more functionality which is not supported in the existing one so that would not be a problem. But I also have different a implementation for keyboard events, joystick, etc... I have tried to be backward compatible but there is (I assume) a huge amount of code out there that depend on the existing implementation. I have tried to use more recent APIs so it might not work with older browsers. I do know for a fact for example that I intentionally did not implement the API related to drag and drop because of the limited support by browser and I did not want to implement the copy on drop functionality from the existing one. So that would definitely be a regression.

I personally would not feel comfortable simply replacing the existing one.

If we are going to have this port live outside of the emscripten repo (i'm not suggesting we should or shouldn't do this), is there any reason it shouldn't actually live upstream in https://github.com/glfw/glfw?

The reason is that it is not an "official" port as I did it on my own and I have no affiliation with glfw. Whether they would want to make it official or not, I don't know as I did not ask. I do not know how receptive the glfw team is in regards to this kind of contribution. I am sure they have different coding conventions and since my implementation is in C++ (vs C for everything else) it might be a showstopper. I can definitely follow that avenue though.

To be honest, in the end what I an after is the ease of using a port.

It is easy to explain: for example, if you want to use sdl2, then simply add -sUSE_SDL=2 and that's it. No "go there, clone this repo, add it to your cmake or use this lines in your Makefile".

I think the concept of "port" is awesome. I am just wondering if maybe it would make sense to open up the concept and allow for external ports that are not part of emscripten, yet they are as easy to use as the built-in ones. Imagine for a second that there was this syntax:

-sUSE_PORT=https://github.com/pongasoft/emsripten-glfw/glfw3-port.py

or in the case of @juj webgpu port:

-sUSE_PORT=https://github.com/juj/wasm_webgpu/webgpu-port.py

and instead of the port being under the ports directory, the build just fetches it and use this file.

This would allow to have ports like the build-in ones in terms of usability while not being local, so there is no burden on emscripten. Nothing to add, nothing to maintain.

I wouldn't mind taking a look at implementing such a behavior if this is something you think would be a worthwhile addition to emscripten

@sbc100
Copy link
Collaborator

sbc100 commented Jan 24, 2024

Regarding the ability to plug in third party ports, I have long thought that was a good idea and have been pushing that idea, but its not quite there yet.

The current effort is #13002. I called it "contrib" ports. In my version user could just drop myport.py into emscripten/ports/constib and they would start working. I suppose we could extent that to include ports pulled from URLs too.

@ypujante
Copy link
Contributor Author

Based on your feedback, I believe contrib and/or external url seems the right approach for my port. I am going to close this PR then.

If you are ok I will be spending a bit of time looking at your current effort and maybe submit a PR on this topic. Maybe that will help move the ball forward.

@ypujante ypujante closed this Jan 25, 2024
@ypujante ypujante deleted the emscripten-glfw branch March 23, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants