Skip to content

Commit

Permalink
Update for bug 4923 - Calling SDL_GameControllerRumble() often takes …
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@slouken
slouken committed Feb 7, 2020
1 parent d88f3f5 commit 8125620
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 22 deletions.
92 changes: 77 additions & 15 deletions src/joystick/hidapi/SDL_hidapi_rumble.c
View file
Expand Up @@ -34,15 +34,16 @@
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;

} 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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/joystick/hidapi/SDL_hidapi_rumble.h
View file
Expand Up @@ -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);

Expand Down
41 changes: 34 additions & 7 deletions src/joystick/hidapi/SDL_hidapi_xboxone.c
View file
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 8125620

Please sign in to comment.