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

[sokol_app] add an option html5_use_emsc_set_main_loop #997

Merged
merged 1 commit into from
Mar 2, 2024

Conversation

Dvad
Copy link

@Dvad Dvad commented Feb 22, 2024

Implementation of the idea suggested in #843 (comment)

emscripten_set_main_loop is a higher level function compared to emscripten_request_animation_frame_loop and seems to play nicer with various emcc options (EXIT_RUNTIME or PROXY_TO_PTHREAD). The downside is that we have to compute the timestamp on our side and hence it might be less accurate than the one the browser returns with [requestAnimationFrame](https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame).

I tested both implementation on a test app. Not sure if we need more testing and how to test.

In particular for my use case, only the version SOKOL_EMSC_USE_SET_MAIN_LOOP works when PROXY_TO_PTHREAD is enabled. But A few more changes are needed for this case in sokol_app which will be for another PR.

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

floooh commented Feb 23, 2024

See my comment #843 (comment) about preprocessor define vs runtime flag.

@floooh
Copy link
Owner

floooh commented Feb 23, 2024

...also check the build error (I just added that warning recently):

/home/runner/work/sokol/sokol/tests/compile/../../sokol_app.h:5837:47: error: a function declaration without a prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
 5837 | _SOKOL_PRIVATE void _sapp_emsc_frame_main_loop() {
      |                                               ^

(the function must have an explicit void param: void _sapp_emsc_frame_main_loop(void))

@Dvad
Copy link
Author

Dvad commented Feb 23, 2024

SOKOL_EMSC_USE_SET_MAIN_LOOP -> _sapp.desc.html5_use_emsc_set_main_loop

@Dvad Dvad changed the title [sokol_app] add SOKOL_EMSC_USE_SET_MAIN_LOOP #define [sokol_app] add an option html5_use_emsc_set_main_loop Feb 23, 2024
@floooh
Copy link
Owner

floooh commented Mar 2, 2024

Giving the PR a whirl now...

@floooh floooh merged commit 36790cc into floooh:master Mar 2, 2024
23 checks passed
@floooh
Copy link
Owner

floooh commented Mar 2, 2024

Ok merged. I also added another config option .html5_emsc_set_main_loop_simulate_infinite_loop to be used as the simulate_infinite_loop param to emscripten_set_main_loop since this was also mentioned in #843.

...and I changed the texcube-sapp sample to use the set-main-loop method.

Also updated the changelog.

Many thanks for the PR!

@Dvad Dvad deleted the sokol_emsc_set_main_loop branch March 3, 2024 01:54
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