SDL_DINPUT_HapticUpdateEffect sends unnecessary flags to IDirectInputEffect_SetParameters, which harms FFB quality in certain devices/usecases

[badfontkeming]

Context

This appears to affect both SDL2 and SDL3 (based on my reading of the code). This was discovered in the PCSX2 project, where a slice of Windows users have experienced a regression in force feedback (FFB) quality since the switch from DInput to SDL, while other users with different FFB wheels haven't noticed a thing.

Most racing titles with FFB, past or present, implement FFB primarily by updating the magnitude parameter of a single, always-running constant force. In a DirectInput implementation, that would look something like this:

// C++ psuedocode

// effect is some existing, already-running constant force effect
auto constForce = (DICONSTANTFORCE*)effect.lpvTypeSpecificParams;
constForce->lMagnitude = new_magnitude;
ref->SetParameters(effect, DIEP_TYPESPECIFICPARAMS); // Note that DIEP_TYPESPECIFICPARAMS is the only flag.

I did a lot of low-level digging into USB PID (Physical Interface Device, HID except for haptics) and DirectInput. I'll spare you the details, so here's all you need to know:

• There are two PID reports associated with a force: The "basic" parameters, and the "type-specific" parameters (for a constant force, it's just magnitude.)
• When DIEP_TYPESPECIFICPARAMS is the only flag set, only the "type-specific" report is sent by DirectInput drivers
• Setting any other flags causes the "basic" report to be sent as well.
  • This appears to aggravate specific wheels, causing what users describe as a "loss of detail" during rapid updates
• This was tested with an experimental test build that added a toggle to bypass SDL and call DirectInput's SetParameters directly with the DIEP_TYPESPECIFICPARAMS flag. Affected users reported that enabling this setting provided an immediate, noticeable improvement in FFB quality.
  • I validated with Wireshark that this bypass prevented the redundant report from being sent.

Relevant Code

Inside of SDL_DINPUT_HapticUpdateEffect, all of these flags are set when sending an update to DirectInput:

// src/haptic/windows/SDL_dinputhaptic.c

    /* Set the flags.  Might be worthwhile to diff temp with loaded effect and
     *  only change those parameters. */
    flags = DIEP_DIRECTION |
            DIEP_DURATION |
            DIEP_ENVELOPE |
            DIEP_STARTDELAY |
            DIEP_TRIGGERBUTTON |
            DIEP_TRIGGERREPEATINTERVAL | DIEP_TYPESPECIFICPARAMS;

The comment here already acknowledges that this is known to an extent, but I assume it hadn't yet been known to cause problems.

[slouken]

Thanks for the investigation. Since you have context and test environment, can you create a PR for this?

[badfontkeming]

Life is a bit hectic, but I can give it a go if I can find the time. I'm a bit rusty at C, so forgive me if it ends up needing iteration.

On that topic, how would you want me to handle the edge case where SDL_HapticUpdateEffect has been called without any changes? In this case, flags would be 0. Should I just exit early?

[slouken]

Life is a bit hectic, but I can give it a go if I can find the time. I'm a bit rusty at C, so forgive me if it ends up needing iteration.

That's fine, no worries.

On that topic, how would you want me to handle the edge case where SDL_HapticUpdateEffect has been called without any changes? In this case, flags would be 0. Should I just exit early?

Probably? That should get some testing in case just calling it with the same state is something that some drivers need or expect.

[badfontkeming]

Hi-

Want to apologize for the radio silence on this. Ran into some medical issues that kept me from setting aside the time to work on this. The medical situation is improving, but I'm very behind at my job now so I'm not sure when I'll have the time to send in a PR for this. If you want this done on a specific timetable you may be better off assigning this to someone else--let me know if so and I'll do what I can to help review/test their changes.

The actual changes shouldn't be too daunting, for what it's worth--I'm just swamped at the moment.

[slouken]

I'm glad the medical situation is improving! There's no rush, go ahead and contribute when you can.

Cheers,

[dwillbarron]

Different account, same guy.

I've risen from the grave and made a draft PR on this. Currently seeking testers to ensure that there aren't any unforeseen consequences from my patch (shouldn't be, but it's worth being superstitious about force feedback.)