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

Use emscripten_set_main_loop in sokol app #915

Closed
wants to merge 1 commit into from

Conversation

ambrusc
Copy link

@ambrusc ambrusc commented Oct 7, 2023

Problem: It appears that Emscripten's async APIs don't get a chance to call user callbacks in Sokol App. For instance, when using emscripten_async_wget_data, Firefox shows the request completed with 200 OK, and the data available, but the user callbacks are never called.

Root Cause: From docs about emscripten_request_animation_frame_loop, it seems to be intended as a low level API, and is likely not handling callbacks. Switching to emscripten_set_main_loop fires the callbacks, and still uses requestAnimationFrame under the hood.

Proposal: Use emscripten_set_main_loop instead of emscripten_request_animation_frame_loop in Sokol App.

NOTE: I haven't dug too deeply into what else emscripten_set_main_loop does, or if it duplicates functionality elsewhere in Sokol App. Maybe we just need to hook up the async API callbacks explicitly somewhere else?

I don't mean to open a glut of pull requests, but I thought that maybe others could benefit from some of these findings. If I've misunderstood something or these PRs are not helpful, please just let me know and I'll close them! :-)

@floooh
Copy link
Owner

floooh commented Oct 7, 2023

Hmm, there was a recent discussion about emscripten_set_main_loop() vs emscripten_request_animation_frame_loop() here:

#843

...the callback / event loop problem is new to me though. If the completion callbacks in emscripten_async_wget_data() are not called that sounds more like a bug in the Emscripten runtime to me.

My 'counter proposal' would be to implement the set-main-loop method as an alternative, activated by a preprocessor define similar to SOKOL_NO_ENTRY or SOKOL_WIN32_FORCE_MAIN.

(my main gripe is that the frame timestamp needs to be aquired separately instead of being provided by the request-animation-loop callback, this might theoretically introduce more jitter).

@ambrusc
Copy link
Author

ambrusc commented Oct 21, 2023

Off topic, but I just wanted to thank you for creating this awesome set of libraries. The more I use them the more I really really like them. I'm also really enjoying the initializer/desc-style.

I didn't want to make too much more traffic on these PRs as you have a much better understanding of how things work under the hood. You make a good point about timestamp jitter for instance. Blargh. (Why can't platform-APIs be consistent?)

@floooh floooh mentioned this pull request Feb 22, 2024
@floooh
Copy link
Owner

floooh commented Mar 2, 2024

Closing this because I just merged #997 in favour of this PR (also see changelog entry: https://github.com/floooh/sokol/blob/master/CHANGELOG.md#02-mar-2024).

Still, many thanks for the PR of course!

@floooh floooh closed this Mar 2, 2024
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