-
Notifications
You must be signed in to change notification settings - Fork 285
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
Fix usage of SDL_Init for emscripten #1322
Conversation
Aside: I was only able to start debugging when I added logging to SDL_SetError in my local emscripten ports build cache. I noticed that allegro doesn't provide a logging interface in its System driver, and only in one place does it ever call SDL_GetError (when making a window). Would it make sense to extend the system driver interface to include logging, and somehow wire SDL's logging infrastructure into allegro? Or is there already a way to get SDL logging from allegro that I'm overlooking? |
src/sdl/sdl_system.c
Outdated
unsigned int sdl_flags = SDL_INIT_EVERYTHING; | ||
#ifdef __EMSCRIPTEN__ | ||
// SDL currently does not support haptic feedback for emscripten. | ||
sdl_flags -= SDL_INIT_HAPTIC; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks very odd... I'd rather see this as sdl_flags &= ~SDL_INIT_HAPTIC;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to do this by trying to init with SDL_INIT_EVERYTHING
and falling back to SDL_INIT_EVERYTHING & ~SDL_INIT_HAPTIC
if it fails? The reason being that, if SDL ever does implement a haptic driver for Emscripten, it will work without having to change this code back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seemed to be some sound issues when I did the backoff approach. I'm not confident that SDL truly cleans up everything when it shutsdown its systems.
Thanks for investigating this. What do you mean by a logging interface? You want an Allegro function that one can call to add to a log? If SDL has something like |
Something like: void log_to_allegro(void *userdata, int category, SDL_LogPriority priority, const char *message) {
(void)userdata;
const char* category_string;
switch (category_string) {
case SDL_LOG_CATEGORY_APPLICATION:
category_string: "SDL_LOG_CATEGORY_APPLICATION",
break;
...
}
switch (priority) {
case SDL_LOG_PRIORITY_INFO:
ALLEGRO_INFO("%s: %s\n", category_string, message);
break;
...
}
}
SDL_LogSetOutputFunction(log_to_allegro, NULL) |
I don't have any particular ideas re: implementation, mainly just voicing an annoyance that whatever logging SDL is doing is not accessible to allegro / configurable. Sorry, I'm not being very constructive here :) There does seem to be a way to set a callback function for logging in SDL: https://github.com/EddieRingle/SDL/blob/master/src/SDL_log.c#L361 EDIT: ah, based on your example code you've found the same. |
This patch fixes audio for emscripten.
I printed errors as SDL_GetError was called, and saw this:
Debugging told me the audio system was being initialized, but then shut down. Turns out that if any subsytem requested by SDL_Init cannot start, SDL will shutdown everything. So the haptic subsystem failing was problematic.
It fails because SDL configured the haptic subsytem to not build for emscripten (guess it's not supported).
This behavior in SDL_Init is a recent change. About 6 months ago SDL changed the init function to shutdown things when any requested subsystem failed to start: libsdl-org/SDL@0e294e9 . Previously, it would just return -1, but allegro ignores the return code, so it didn't matter (not fully understanding why everything still worked even though every subsystem after the haptic would have not been initialized...shrug)
I confirmed that the audio demos now play sound in the browser when built locally, using the instructions found in the SDL readme.
Fixes #1321