We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4923 - Calling SDL_GameControllerRumble() often takes 8 ms
Summary: Calling SDL_GameControllerRumble() often takes 8 ms
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: haptic (show other bugs)
Version: 2.0.10
Hardware: All All
: P2 major
Assignee: Sam Lantinga
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-02 16:29 UTC by RustyM
Modified: 2020-02-07 19:46 UTC (History)
1 user (show)

See Also:


Attachments
fix DS4 bluetooth, use atomic, one request per device (2.57 KB, patch)
2020-02-06 12:30 UTC, Mathieu Eyraud
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description RustyM 2020-01-02 16:29:05 UTC
I love the new rumble APIs, as it allows haptic feedback on many controller types on all the desktop platforms. However, I’ve found it to be very slow. Often taking over 8 milliseconds per call. 

I timed the calls with std::chrono::high_resolution_clock::time_point
The lengths of the calls can very, but it tends to hover around 8 milliseconds when many calls are made back to back. Here’s an example of normal usage:

2.646768 ms
8.459243 ms
4.641060 ms
1.497755 ms
7.205246 ms
1.215720 ms
0.780426 ms
4.126962 ms
1.253109 ms
3.574739 ms
3.146366 ms
6.619936 ms
8.498833 ms
8.491646 ms

I’ve seen this on Windows 10 and macOS 10.12 using PS4 or Switch Pro controllers. (I’m using the older haptic API with Xbox controllers.) SDL_JoystickRumble() and SDL_GameControllerRumble() are equally slow. And calling SDL_GameControllerRumble() to multiple controllers on the same frame really adds up. I first found the issue in SDL 2.0.9 and updating to 2.0.10 or the latest development version didn’t fix the slowness.

I tried bypassing the slowness by putting SDL_GameControllerRumble() calls on a different thread. This made the call faster, but I still experienced this same slowdown in my program. I suspect this is because of some mutex or locking calls inside the new rumble API? Not really sure though, because I don’t have a lot of experience with multithreaded code.
Comment 1 Mathieu Eyraud 2020-01-04 10:14:08 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);
}
Comment 2 Mathieu Eyraud 2020-01-04 10:32:08 UTC
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.
Comment 3 Sam Lantinga 2020-02-05 00:06:59 UTC
I think this is fixed in the recent commits. Can you try the latest snapshot?
http://www.libsdl.org/tmp/SDL-2.0.zip
Comment 4 RustyM 2020-02-05 17:01:13 UTC
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.
Comment 5 Sam Lantinga 2020-02-05 18:55:20 UTC
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.
Comment 6 RustyM 2020-02-05 22:06:19 UTC
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.
Comment 7 Sam Lantinga 2020-02-06 02:51:03 UTC
Great!

Yup, the Switch code hasn't been converted over yet because it's more complicated, but that's on my TODO list.
Comment 8 Mathieu Eyraud 2020-02-06 12:30:43 UTC
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.
Comment 9 Sam Lantinga 2020-02-07 17:37:00 UTC
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.
Comment 10 Sam Lantinga 2020-02-07 19:03:21 UTC
This is implemented in https://hg.libsdl.org/SDL/rev/1d8cb0c5236b

Thanks!
Comment 11 Sam Lantinga 2020-02-07 19:46:50 UTC
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!