sinput: Add fully generic capabilities #13565

antheas · 2025-08-01T14:08:29Z

This PR refactors the generic capability loading of SInput to ensure that future SInput devices always use a correct mapping, with the best capability match that is available.

Description

Main Refactor Commit

Currently, the generic raspberry pi vid:pid pair must be used for
generic mask functionality. Moreover, the axes and buttons are hardcoded
in different places in the hid driver, making them difficult to use.

First, refactor SDL_gamepad.c to always use autodetection for unknown
controllers. Then, in the HID driver, generate valid masks based on a
given subtype that will always match the output SDL string and compare
them to the ones in the HID descriptor. If they are different, emit a
warning but use the ones from the sub_type anyway.

For already known controllers, add a path to skip the check and load
the masks from firmware, assuming what is there matches SDL. This is
done on faith, so it should be avoided for future controllers and hardcoded
alongside the mapping string.

Moving forward, this commit allows for the following procedure.
Assume company X releases controller Y. If the controller uses a
completely new layout, company asks for a sub_type. Company gets
sub_type and puts it into the controller firmware and releases the
controller.

In the period that the bundled Steam SDL does not have that sub_type,
it uses the full mapping which is correct for the controller and quite
useable, but misses a proper capability match. If it does have the sub_type,
capabilities are correct without any additions to SDL. And in the
scenario that a controller is too obscure to warrant a sub_type, 0 can
be used, and a mapping can be added to its vid:pid pair for proper
capabilities to avoid using sub_types.

Capabilities commit

The previous commit introduces the ability to add dynamic
capabilities for uknown controllers without hardcoding them to SDL.

This commit introduces 8 major types of controller capabilities that
are found in the market. Specifically, it adds a normal XInput
capability map. Then, it adds XInput + share, which covers most
handhelds without extra buttons, switch controllers, and new Xbox
controllers. Following, it adds combinations with 2 and 4 paddles,
which include e.g., devices such as the Legion Go S and Stadia
controllers with 2 paddles, and Legion Go/Xbox Elite with 4 paddles.

Finally, it adds the same paddle combos + clicks, where e.g.,
click + 2 paddles covers the Go S when emulating the touchpad and
click + 4 paddles covers the dualsense edge controller capabilities
fully.

Chain PR to #13524 , also fixes compilation when debug messages are on.

cc @mitchellcairns

antheas · 2025-08-01T15:37:40Z

A draft implementation that fully follows the spec can be found in this PR.
hhd-dev/hhd#202

It supports dynamically adjusting RGB, gyro, and number of paddles depending on user action.

Touchpad emulation still needs to be done, but for that we need to define a way to pass a touchpad size hint, so that 1) sensitivity can be correct by default, and 2) aspect ratio is correct.

antheas · 2025-08-01T15:38:19Z

All mappings were tested using testcontroller with an LD_PRELOAD for the built SDL.

src/joystick/hidapi/SDL_hidapi_sinput.c

mitchellcairns · 2025-08-02T07:39:49Z

src/joystick/SDL_gamepad.c

The intention here is to see how SInput is used for some time before dedicating slots to pre-defined types. I think that it's reasonable to start drafting some out and have proposals for these.

I need these for my use of the protocol. I could drop the XInput one but that's about it

With all buttons and analog axis exposed, what is missing for your current use case to get things working?

I need to be able to expose the correct amount of back buttons. I do not need all the buttons, there are enough controllers to do that with.

src/joystick/SDL_joystick_c.h

mitchellcairns · 2025-08-02T07:50:09Z

src/joystick/hidapi/SDL_hidapi_sinput.c

@@ -233,75 +233,176 @@ static inline float CalculateAccelScale(uint16_t g_range)
    return SDL_STANDARD_GRAVITY / (32768.0f / (float)g_range);
 }

-static void ProcessSDLFeaturesResponse(SDL_HIDAPI_Device *device, Uint8 *data)
+static bool ProcessSDLFeaturesResponse(SDL_HIDAPI_Device *device, Uint8 *data)


This is only invoked if we have a valid command packet input report, this should not need to return anything.

The packet has a protocol version. So it is not fair to assume that SDL will be able to always process it. It needs to be able to exit

How the features response is processed would, in the future, be determined by the protocol version, which I don't think is unreasonable to also have in this function, or broken out if/when that bridge needs to be crossed.

mitchellcairns · 2025-08-02T08:10:31Z

I have a few thoughts/notes for now:

I think that several aspects of this needs to be split into their own PR drafts.

Adding the sub-type mapping strings should be its own draft, so that we can evaluate that aspect on its own and keep discussion around that focused.
Refactoring should be a separate draft, after everything else is settled.

On the topic of sub-types...

I think that the sub-type mappings you suggest are within reason, but I would not expect this to be a situation where a company would request a sub-type. At the point a company is requesting a sub-type, they should be doing what they can do obtain a PID/VID pair.

I think it would be within reason to have the sub-type act as a means for a PID/VID to be split into 32 slots, allowing a company to essentially get more out of those 32 slots and have unique devices that accurately represent their own devices.

The generic slot right now is a fallback, to let developers get a handle on SInput without having to change anything in SDL. Once I can see how makers/hobbyists/companies do or don't use SInput, I will be able to make a better determination on how to allocate the other 31 slots for that specific PID. I think it's too early to make decisions on this.

antheas · 2025-08-02T08:18:33Z

If it is easier to merge I can spin a PR with e.g., the first 2 commits

antheas · 2025-08-02T08:21:48Z

#13592
#13593

antheas · 2025-08-02T08:32:13Z

I removed the refactor commit so this is a chain PR now to the two above. I will mark it ready once those two merge.

I cannot find a place to move the subtype to so I can rename it. @slouken you have any suggestions?

antheas · 2025-08-02T13:22:20Z

I renamed the variables after the convention used in other drivers so that they are sinput specific and added a custom controller type entry so that sinput can load dynamically with env vars in the future.

I also added my own vid:pid so that it can deviate in the future.

I tried to do a refactor so that the types would only apply to my vid:pid. However, it got quite complicated and ugly and the controller types are quite straightforward so they would be added anyway. I am not sure if there is a point in separating them.

antheas · 2025-08-02T13:38:15Z

I mirrored the logic between hidapi_sinput and gamepad so that they mirror each other and it is easier to add overrides if other layouts are required.

slouken · 2025-08-02T13:40:34Z

Just to be clear, official products are expected to have their own unique VID/PID. Any work based on subtype is intended for products in development and Steam will treat them all as having the same name. PRs making the subtype highly load bearing will not be accepted because of this.

antheas · 2025-08-02T14:22:12Z

Just to be clear, official products are expected to have their own unique VID/PID. Any work based on subtype is intended for products in development and Steam will treat them all as having the same name. PRs making the subtype highly load bearing will not be accepted because of this.

This PR is trying to solve for dynamic capability match for the same device under different conditions. Users need to e.g., be able to disable back buttons (because they want to remap them in their vendor software or they do not want them), touchpad game passthrough (for the same reason), and gyro support on demand (for e.g., power saving).

It is also trying to lower the effort required to upstream VID:PID pairs to essentially adding a db entry if the controller has a common layout.

I am not sure what you mean by the word official. A lot controller manufacturers either use the standard Xbox/Sony/Nintendo VID:PIDs or liberally re-use their approved/upstreamed VID:PID pairs across multiple product lines. This PR is inline with this assumption in that regard, both in the re-use part and in lowering the burden of upstreaming part to encourage using more unique ones.

The generic slot right now is a fallback, to let developers get a handle on SInput without having to change anything in SDL. Once I can see how makers/hobbyists/companies do or don't use SInput, I will be able to make a better determination on how to allocate the other 31 slots for that specific PID. I think it's too early to make decisions on this.

I missed the specific wording of this today. If you still feel strongly about this I can have a second attempt at separating the mappings to not affect your PID.

mitchellcairns · 2025-08-02T18:36:17Z

The way I am looking at the situation currently is that sub-type is functionally doing what Nintendo does with their PID sub-device allocations (Pro Controller, N64 controller, SNES controller all sharing a PID). It's a product ID to allocate sub-products under a single PID. Each sub-type should still have a defined mapping as SDL is expecting to understand what the physical layout is and have that hard-coded.

antheas · 2025-08-03T14:24:10Z

The way I am looking at the situation currently is that sub-type is functionally doing what Nintendo does with their PID sub-device allocations (Pro Controller, N64 controller, SNES controller all sharing a PID). It's a product ID to allocate sub-products under a single PID. Each sub-type should still have a defined mapping as SDL is expecting to understand what the physical layout is and have that hard-coded.

I refactored the implementation to do that. Moving forward, different PID:VID pairs can arbitrarily select how to allocate their values, and by default your generic pair will always use full mapping, and an adjustable face style. 0x16d0:0x145b uses the mappings I suggested.

mitchellcairns · 2025-08-04T21:12:02Z

src/joystick/SDL_gamepad.c

-        Uint8 sub_type  = guid.data[15] & 0x1F;
+        ESinputControllerType controller_type;
+        ESinputFaceStyle face_style;
+        HIDAPI_DriverSInput_GetControllerType(vendor, product, version, guid.data[15], &controller_type, &face_style);


I think that this could be more effective as a GetMappingString, and utilize a capability bitmask that can be populated into the 'version' field by the SInput HIDAPI Init function based on the reported dynamic capabilities.

To clarify, the version field should not fill out this data on the device side, we are filling out the version field within SDL only.

Example for implementation of dynamic mapping string generation: https://pastebin.com/J5GFbEbt

mitchellcairns · 2025-08-04T21:13:19Z

src/joystick/SDL_gamepad.c

        switch (face_style) {
-        default:
+        case k_eSInputFaceStyle_abxy:


There is a type that this is derived from already (SDL_GamepadFaceStyle) that could be used instead.

It's in the C file I'll have to move it

…llers Add a controller type for sinput to allow loading the driver dynamically via an environment variable override. Then, cleanup the mapping code in SDL_gamepad.c and SDL_hidapi_sinput.c by placing the vid:pid and subtype logic in HIDAPI_DriverSInput_GetControllerType. This function can arbitrarily decide the layout and face style of a controller based on its vid, pid, version, and subtype. This allows for up to 65336 different combinations of controller types/face styles per vid:pid. Then, add 8 controller layouts and face styles for the vid:pid pair 0x16d0:0x145b that will be used to virtually test the sinput driver. To defer making a decision, for other vid:pid pairs, FullMapping will always be used, except for the generic sinput pid, which will load different face styles based on the upper three bits of subtype.

This commit includes additions relating to SInput generic device reporting capabilities in a bit more detail, to automatically choose the best input map possible for the given device. Thanks to Antheas Kapenekakis ([email protected]) for contributing the neat compression algorithm, this is pulled from the PR Draft here: libsdl-org#13565 Co-authored-by: Antheas Kapenekakis <[email protected]>

antheas · 2025-08-05T09:10:06Z

Closed in favor of #13667

slouken · 2025-08-05T18:20:08Z

@antheas, thank you for your contribution!

I have some feedback, both generally, and specific to this PR.

First, if a feature is newly contributed to a project, and the author of that feature is actively working on it, it's polite to approach them first and discuss your ideas and collaborate with them on it. What I see here is a massive refactor and large functional addition without that initial discussion and collaboration. I appreciate your enthusiasm, but please be kind when doing this kind of thing.

Second, it's much easier to review and accept smaller changes than one large comprehensive one. Especially if some of those changes are likely to be accepted and others need more iteration.

Now, specific feedback on this PR:

I don't think that 8 major types of controller capabilities is the right approach here. There is some debate on what set of capabilities are the right ones to represent, so it's probably better to allocate them on an as-needed basis. In addition, @mitchellcairns is proposing a more detailed capability system that might cover all uses cases and should be considered.
You should not add to EControllerType. I haven't looked closely at why you've done that, but that enum is shared with Valve code and can't be modified in this project.

Both you and @mitchellcairns are trying to solve this problem. I am asking you both to cooperate to break the changes down into smaller commits that can be reviewed and committed. Since he is the original author, I expect to get his sign-off on any accepted PRs.

Thanks!

antheas · 2025-08-05T18:36:32Z

Hi @slouken

Thanks for the feedback. I agree with everything you said.

I should note that I refactored this PR yesterday while chatting with @mitchellcairns before he made #13667 . So it currently implements the same thing, in the same way.

It does that in a way I feel is cleaner, which is where the frustration comes from. I would rather he had began with my code instead of re-implementing it from scratch. Because I feel that certain aspects of this PR are better (such as the vid/pid handling being centralized in the header), and it is very discouraging to have to port them to the other PR with an additional time sink to me, as Michael could have collaborated with me on this one instead of rewriting it.

I don't think that 8 major types of controller capabilities is the right approach here. There is some debate on what set of capabilities are the right ones to represent, so it's probably better to allocate them on an as-needed basis. In addition, @mitchellcairns is proposing a more detailed capability system that might cover all uses cases and should be considered.

The text of this PR is outdated. It is a essentially a carbon copy #13667 but done 4 hours before.

You should not add to EControllerType. I haven't looked closely at why you've done that, but that enum is shared with Valve code and can't be modified in this project.

This could be spun to a separate PR as it is mostly a separate feature. The reasoning behind this is to allow providing an environment hint to load SInput for a specific VID/PID. If there is a different way to do this I could propose that instead.

This commit includes additions relating to SInput generic device reporting capabilities in a bit more detail, to automatically choose the best input map possible for the given device. Thanks to Antheas Kapenekakis ([email protected]) for contributing the neat compression algorithm, this is pulled from the PR Draft here: libsdl-org#13565 Co-authored-by: Antheas Kapenekakis <[email protected]>

antheas force-pushed the sinput-capabilities branch from 893d1b0 to 026b277 Compare August 1, 2025 15:39

slouken reviewed Aug 1, 2025

View reviewed changes

src/joystick/hidapi/SDL_hidapi_sinput.c Outdated Show resolved Hide resolved

antheas force-pushed the sinput-capabilities branch 4 times, most recently from 8dae7b9 to 7ec8e4a Compare August 1, 2025 17:38

mitchellcairns reviewed Aug 2, 2025

View reviewed changes

src/joystick/SDL_joystick_c.h Outdated Show resolved Hide resolved

mitchellcairns reviewed Aug 2, 2025

View reviewed changes

antheas force-pushed the sinput-capabilities branch from 7ec8e4a to d0e3af5 Compare August 2, 2025 08:30

antheas mentioned this pull request Aug 2, 2025

sinput: refactor to make unknown controllers fully dynamic #13593

Closed

antheas force-pushed the sinput-capabilities branch from d0e3af5 to 021f2c9 Compare August 2, 2025 13:19

antheas force-pushed the sinput-capabilities branch 3 times, most recently from 4fe152a to 705e0bb Compare August 2, 2025 13:36

antheas force-pushed the sinput-capabilities branch from 705e0bb to 05fc27a Compare August 2, 2025 15:11

antheas force-pushed the sinput-capabilities branch 2 times, most recently from c1a55e5 to ee121f4 Compare August 3, 2025 14:22

antheas force-pushed the sinput-capabilities branch 5 times, most recently from 43f3da0 to 217f89f Compare August 4, 2025 18:29

mitchellcairns reviewed Aug 4, 2025

View reviewed changes

antheas force-pushed the sinput-capabilities branch from 217f89f to 3738bf9 Compare August 4, 2025 23:07

mitchellcairns mentioned this pull request Aug 5, 2025

SInput: Version as a capabilities vehicle #13667

Draft

antheas closed this Aug 5, 2025

sinput: Add fully generic capabilities #13565

sinput: Add fully generic capabilities #13565

Conversation

antheas commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Main Refactor Commit

Capabilities commit

Uh oh!

antheas commented Aug 1, 2025

Uh oh!

antheas commented Aug 1, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchellcairns commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antheas commented Aug 2, 2025

Uh oh!

antheas commented Aug 2, 2025

Uh oh!

antheas commented Aug 2, 2025

Uh oh!

antheas commented Aug 2, 2025

Uh oh!

antheas commented Aug 2, 2025

Uh oh!

slouken commented Aug 2, 2025

Uh oh!

antheas commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mitchellcairns commented Aug 2, 2025

Uh oh!

antheas commented Aug 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antheas commented Aug 5, 2025

Uh oh!

slouken commented Aug 5, 2025

Uh oh!

antheas commented Aug 5, 2025

Uh oh!

Uh oh!

antheas commented Aug 1, 2025 •

edited

Loading

mitchellcairns commented Aug 2, 2025 •

edited

Loading

antheas commented Aug 2, 2025 •

edited

Loading