floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
6.63k stars 472 forks source link

Use `emscripten_set_main_loop` in sokol app #915

Closed ambrusc closed 4 months ago

ambrusc commented 9 months ago

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 commented 9 months ago

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

https://github.com/floooh/sokol/issues/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 commented 9 months ago

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 commented 4 months ago

Closing this because I just merged https://github.com/floooh/sokol/pull/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!