From 81256207bdd497535fcff83a96ce3fa730b70c24 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Fri, 7 Feb 2020 11:02:34 -0800 Subject: [PATCH] Update for bug 4923 - Calling SDL_GameControllerRumble() often takes 8 ms meyraud705 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. --- src/joystick/hidapi/SDL_hidapi_rumble.c | 92 ++++++++++++++++++++---- src/joystick/hidapi/SDL_hidapi_rumble.h | 8 +++ src/joystick/hidapi/SDL_hidapi_xboxone.c | 41 +++++++++-- 3 files changed, 119 insertions(+), 22 deletions(-) diff --git a/src/joystick/hidapi/SDL_hidapi_rumble.c b/src/joystick/hidapi/SDL_hidapi_rumble.c index 860bd0649314a..a4f05e4a9be5f 100644 --- a/src/joystick/hidapi/SDL_hidapi_rumble.c +++ b/src/joystick/hidapi/SDL_hidapi_rumble.c @@ -34,7 +34,7 @@ typedef struct SDL_HIDAPI_RumbleRequest { SDL_HIDAPI_Device *device; - Uint8 data[USB_PACKET_LENGTH]; + Uint8 data[2*USB_PACKET_LENGTH]; /* need enough space for the biggest report: dualshock4 is 78 bytes */ int size; struct SDL_HIDAPI_RumbleRequest *prev; @@ -42,7 +42,8 @@ typedef struct SDL_HIDAPI_RumbleRequest typedef struct SDL_HIDAPI_RumbleContext { - volatile SDL_bool running; + SDL_atomic_t initialized; + SDL_atomic_t running; SDL_Thread *thread; SDL_mutex *lock; SDL_sem *request_sem; @@ -58,7 +59,7 @@ static int SDL_HIDAPI_RumbleThread(void *data) SDL_SetThreadPriority(SDL_THREAD_PRIORITY_HIGH); - while (ctx->running) { + while (SDL_AtomicGet(&ctx->running)) { SDL_HIDAPI_RumbleRequest *request = NULL; SDL_SemWait(ctx->request_sem); @@ -87,7 +88,7 @@ static int SDL_HIDAPI_RumbleThread(void *data) static void SDL_HIDAPI_StopRumbleThread(SDL_HIDAPI_RumbleContext *ctx) { - ctx->running = SDL_FALSE; + SDL_AtomicSet(&ctx->running, SDL_FALSE); if (ctx->thread) { int result; @@ -110,6 +111,8 @@ SDL_HIDAPI_StopRumbleThread(SDL_HIDAPI_RumbleContext *ctx) SDL_DestroyMutex(ctx->lock); ctx->lock = NULL; } + + SDL_AtomicSet(&ctx->initialized, SDL_FALSE); } static int @@ -127,7 +130,7 @@ SDL_HIDAPI_StartRumbleThread(SDL_HIDAPI_RumbleContext *ctx) return -1; } - ctx->running = SDL_TRUE; + SDL_AtomicSet(&ctx->running, SDL_TRUE); ctx->thread = SDL_CreateThreadInternal(SDL_HIDAPI_RumbleThread, "HIDAPI Rumble", 0, ctx); if (!ctx->thread) { SDL_HIDAPI_StopRumbleThread(ctx); @@ -136,23 +139,48 @@ SDL_HIDAPI_StartRumbleThread(SDL_HIDAPI_RumbleContext *ctx) return 0; } -int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size) +int SDL_HIDAPI_LockRumble(void) { SDL_HIDAPI_RumbleContext *ctx = &rumble_context; - SDL_HIDAPI_RumbleRequest *request; - - if (size > sizeof(request->data)) { - return SDL_SetError("Couldn't send rumble, size %d is greater than %d", size, (int)sizeof(request->data)); - } - if (!ctx->running) { + if (SDL_AtomicCAS(&ctx->initialized, SDL_FALSE, SDL_TRUE)) { if (SDL_HIDAPI_StartRumbleThread(ctx) < 0) { return -1; } } + return SDL_LockMutex(ctx->lock); +} + +SDL_bool SDL_HIDAPI_GetPendingRumbleLocked(SDL_HIDAPI_Device *device, Uint8 **data, int **size, int *maximum_size) +{ + SDL_HIDAPI_RumbleContext *ctx = &rumble_context; + SDL_HIDAPI_RumbleRequest *request; + + for (request = ctx->requests_tail; request; request = request->prev) { + if (request->device == device) { + *data = request->data; + *size = &request->size; + *maximum_size = sizeof(request->data); + return SDL_TRUE; + } + } + return SDL_FALSE; +} + +int SDL_HIDAPI_SendRumbleAndUnlock(SDL_HIDAPI_Device *device, const Uint8 *data, int size) +{ + SDL_HIDAPI_RumbleContext *ctx = &rumble_context; + SDL_HIDAPI_RumbleRequest *request; + + if (size > sizeof(request->data)) { + SDL_HIDAPI_UnlockRumble(); + return SDL_SetError("Couldn't send rumble, size %d is greater than %d", size, (int)sizeof(request->data)); + } + request = (SDL_HIDAPI_RumbleRequest *)SDL_calloc(1, sizeof(*request)); if (!request) { + SDL_HIDAPI_UnlockRumble(); return SDL_OutOfMemory(); } request->device = device; @@ -161,25 +189,59 @@ int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size SDL_AtomicIncRef(&device->rumble_pending); - SDL_LockMutex(ctx->lock); if (ctx->requests_head) { ctx->requests_head->prev = request; } else { ctx->requests_tail = request; } ctx->requests_head = request; - SDL_UnlockMutex(ctx->lock); + + /* Make sure we unlock before posting the semaphore so the rumble thread can run immediately */ + SDL_HIDAPI_UnlockRumble(); SDL_SemPost(ctx->request_sem); return size; } +void SDL_HIDAPI_UnlockRumble(void) +{ + SDL_HIDAPI_RumbleContext *ctx = &rumble_context; + + SDL_UnlockMutex(ctx->lock); +} + +int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size) +{ + Uint8 *pending_data; + int *pending_size; + int maximum_size; + + if (SDL_HIDAPI_LockRumble() < 0) { + return -1; + } + + /* check if there is a pending request for the device and update it */ + if (SDL_HIDAPI_GetPendingRumbleLocked(device, &pending_data, &pending_size, &maximum_size)) { + if (size > maximum_size) { + SDL_HIDAPI_UnlockRumble(); + return SDL_SetError("Couldn't send rumble, size %d is greater than %d", size, maximum_size); + } + + SDL_memcpy(pending_data, data, size); + *pending_size = size; + SDL_HIDAPI_UnlockRumble(); + return size; + } + + return SDL_HIDAPI_SendRumbleAndUnlock(device, data, size); +} + void SDL_HIDAPI_QuitRumble(void) { SDL_HIDAPI_RumbleContext *ctx = &rumble_context; - if (ctx->running) { + if (SDL_AtomicGet(&ctx->running)) { SDL_HIDAPI_StopRumbleThread(ctx); } } diff --git a/src/joystick/hidapi/SDL_hidapi_rumble.h b/src/joystick/hidapi/SDL_hidapi_rumble.h index dde3dbd10e667..9b14da0983557 100644 --- a/src/joystick/hidapi/SDL_hidapi_rumble.h +++ b/src/joystick/hidapi/SDL_hidapi_rumble.h @@ -23,6 +23,14 @@ #ifdef SDL_JOYSTICK_HIDAPI /* Handle rumble on a separate thread so it doesn't block the application */ + +/* Advanced API */ +int SDL_HIDAPI_LockRumble(void); +SDL_bool SDL_HIDAPI_GetPendingRumbleLocked(SDL_HIDAPI_Device *device, Uint8 **data, int **size, int *maximum_size); +int SDL_HIDAPI_SendRumbleAndUnlock(SDL_HIDAPI_Device *device, const Uint8 *data, int size); +void SDL_HIDAPI_UnlockRumble(void); + +/* Simple API, will replace any pending rumble with the new data */ int SDL_HIDAPI_SendRumble(SDL_HIDAPI_Device *device, const Uint8 *data, int size); void SDL_HIDAPI_QuitRumble(void); diff --git a/src/joystick/hidapi/SDL_hidapi_xboxone.c b/src/joystick/hidapi/SDL_hidapi_xboxone.c index d964351374fad..2e109c901fc96 100644 --- a/src/joystick/hidapi/SDL_hidapi_xboxone.c +++ b/src/joystick/hidapi/SDL_hidapi_xboxone.c @@ -118,6 +118,7 @@ typedef struct { Uint8 sequence; Uint8 last_state[USB_PACKET_LENGTH]; SDL_bool rumble_synchronized; + SDL_bool rumble_synchronization_complete; SDL_bool has_paddles; } SDL_DriverXboxOne_Context; @@ -194,7 +195,10 @@ SynchronizeRumbleSequence(SDL_HIDAPI_Device *device, SDL_DriverXboxOne_Context * SDL_memcpy(init_packet, xboxone_rumble_reset, sizeof(xboxone_rumble_reset)); for (i = 0; i < 255; ++i) { init_packet[2] = ((ctx->sequence + i) % 255); - if (SDL_HIDAPI_SendRumble(device, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) { + if (SDL_HIDAPI_LockRumble() < 0) { + return SDL_FALSE; + } + if (SDL_HIDAPI_SendRumbleAndUnlock(device, init_packet, sizeof(xboxone_rumble_reset)) != sizeof(xboxone_rumble_reset)) { SDL_SetError("Couldn't write Xbox One initialization packet"); return SDL_FALSE; } @@ -371,16 +375,39 @@ HIDAPI_DriverXboxOne_RumbleJoystick(SDL_HIDAPI_Device *device, SDL_Joystick *joy { SDL_DriverXboxOne_Context *ctx = (SDL_DriverXboxOne_Context *)device->context; Uint8 rumble_packet[] = { 0x09, 0x00, 0x00, 0x09, 0x00, 0x0F, 0x00, 0x00, 0x00, 0x00, 0xFF, 0x00, 0xFF }; + Uint8 *pending_rumble; + int *pending_size; + int maximum_size; SynchronizeRumbleSequence(device, ctx); - /* Magnitude is 1..100 so scale the 16-bit input here */ - rumble_packet[2] = ctx->sequence++; - rumble_packet[8] = low_frequency_rumble / 655; - rumble_packet[9] = high_frequency_rumble / 655; + if (SDL_HIDAPI_LockRumble() < 0) { + return -1; + } - if (SDL_HIDAPI_SendRumble(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) { - return SDL_SetError("Couldn't send rumble packet"); + if (!ctx->rumble_synchronization_complete) { + if (!SDL_HIDAPI_GetPendingRumbleLocked(device, &pending_rumble, &pending_size, &maximum_size)) { + /* Rumble synchronization has drained */ + ctx->rumble_synchronization_complete = SDL_TRUE; + } + } + + /* Try to overwrite any pending rumble with the new value */ + if (ctx->rumble_synchronization_complete && + SDL_HIDAPI_GetPendingRumbleLocked(device, &pending_rumble, &pending_size, &maximum_size)) { + /* Magnitude is 1..100 so scale the 16-bit input here */ + pending_rumble[8] = low_frequency_rumble / 655; + pending_rumble[9] = high_frequency_rumble / 655; + SDL_HIDAPI_UnlockRumble(); + } else { + /* Magnitude is 1..100 so scale the 16-bit input here */ + rumble_packet[2] = ctx->sequence++; + rumble_packet[8] = low_frequency_rumble / 655; + rumble_packet[9] = high_frequency_rumble / 655; + + if (SDL_HIDAPI_SendRumbleAndUnlock(device, rumble_packet, sizeof(rumble_packet)) != sizeof(rumble_packet)) { + return SDL_SetError("Couldn't send rumble packet"); + } } return 0; }