| Summary: | Calling SDL_GameControllerRumble() often takes 8 ms | ||
|---|---|---|---|
| Product: | SDL | Reporter: | RustyM <rustym> |
| Component: | haptic | Assignee: | Sam Lantinga <slouken> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | major | ||
| Priority: | P2 | CC: | meyraud705 |
| Version: | 2.0.10 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | fix DS4 bluetooth, use atomic, one request per device | ||
|
Description
RustyM
2020-01-02 16:29:05 UTC
I can confirm that SDL_JoystickRumble take several milliseconds to return on joysticks that use hidapi. I suspect this issue is caused because hid_write is blocking: https://hg.libsdl.org/SDL/file/d984274996dd/src/hidapi/windows/hid.c#l717 > /* Wait here until the write is done. This makes > hid_write() synchronous. */ As a workaround you can call SDL_JoystickRumble in another thread: SDL_Joystick* joy; SDL_mutex* m; Uint16 left, right; void Rumble(Uint16 l, Uint16 r) { SDL_LockMutex(m); left = l; right = r; SDL_UnlockMutex(m); } int RumbleThread(void*) { SDL_LockMutex(m); Uint16 l = left; Uint16 r = right; SDL_UnlockMutex(m); SDL_Joystick* joy SDL_JoystickRumble(haptic_joy, l, r, 1000); } Writing code in here is not easy as you cannot write tabulation. Previous example is incomplete: rumble thread obviously need a loop.
SDL_Joystick* joy;
SDL_mutex* m;
Uint16 left, right;
bool run_rumble_thread = true;
void Rumble(Uint16 l, Uint16 r)
{
SDL_LockMutex(m);
left = l;
right = r;
SDL_UnlockMutex(m);
}
int RumbleThread(void*) {
while (1) {
SDL_LockMutex(m);
if (!run_rumble_thread) {return 0;}
Uint16 l = left;
Uint16 r = right;
SDL_UnlockMutex(m);
SDL_JoystickRumble(joy, l, r, 1000);
}
return 0;
}
then you create the thread:
SDL_CreateThread(RumbleThread, "rumble", NULL);
And you can call Rumble from your main thread, it will return immediately.
I think this is fixed in the recent commits. Can you try the latest snapshot? http://www.libsdl.org/tmp/SDL-2.0.zip Awesome Sam! I was able to test this on Windows. When calling rumble on the main thread: - Calls a second or so apart: just a slow as before (~7ms). - Multiple calls a second: the first was slow, but the following calls were fast. - Calling every frame: every call was slow. When calling rumble on a separate thread: The very first call after launching the program was slow. Every subsequent was fast. Calls a second apart, multiple times a second, and every frame were all fast. Even calling every frame exhibited no slowdown! Before this would be quiet slow. So, if I call rumble on a separate thread it seems solved I guess? I wasn’t able to run the test on Mac because the snapshot crashes on SDL_Init if a controller is plugged in or crashes on SDL_PollEvent if a controller is plugged in later. I saw a new "hidapi.framework". I tried including it in my Xcode project, but it didn't fix the crashing. Perhaps I’m not adding it correctly. Can you debug and find out what's taking so long? The new code should send the rumble request directly to a separate thread and return immediately. Sure! I was testing with a Switch Pro controller. It looks like most of the SDL hidapi’s have been updated to call SDL_HIDAPI_SendRumble() in their “RumbleJoystick()” functions. However in SDL_hidapi_switch.c the HIDAPI_DriverSwitch_RumbleJoystick() calls WriteRumble(ctx). I think this is a synchronous call and is why I’m still seeing the slowness. When testing with a PS4 controller the function returns immediately for me. Great! Yup, the Switch code hasn't been converted over yet because it's more complicated, but that's on my TODO list. Created attachment 4192 [details]
fix DS4 bluetooth, use atomic, one request per device
The fix works but:
Dualshock4 on bluetooth need 78 bytes for the rumble data while SDL_HIDAPI_RumbleRequest can only hold 64 bytes.
'volatile' is not meant for thread synchronization.
The list of rumble request could grow infinitely if user call SDL_JoystickRumble too much. The documentation says "Each call to this function cancels any previous rumble effect", so overwriting pending request seem like a good idea.
I attached a patch that implement all my comment.
Thanks for the patch. The Xbox One controller has a sequence number in the rumble data that must be correct, so I'll adjust the code for that. This is implemented in https://hg.libsdl.org/SDL/rev/1d8cb0c5236b Thanks! Okay, Nintendo Switch controllers now use the asynchronous I/O path, so this should be functional for all controllers. https://hg.libsdl.org/SDL/rev/5de509fd3a96 Thanks! |