You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@slouken Any chance to look at this yet? It's been a couple weeks so I'm worried the patch might not apply cleanly again (though it looks like the joystick API hasn't been refactored again this week, so it's probably still good).
On 2019-06-12 03:37:59 +0000, Sam Lantinga wrote:
We're going to look at this after we ship SDL 2.0.10.
Thanks!
On 2019-06-12 04:03:35 +0000, Jimb Esser wrote:
Cool! Give me a ~2 week heads up and I'll merge the latest SDL over again and make sure you've still got a patch that applies well.
On 2019-06-12 14:47:41 +0000, Sam Lantinga wrote:
Now's a good time. We're about to lock code for 2.0.10, and we'll let it soak for a bit for release.
On 2019-06-12 22:36:58 +0000, Jimb Esser wrote:
Created attachment 3824
Patch (2019-06-12)
On 2019-06-12 22:38:32 +0000, Jimb Esser wrote:
Upstream merge blessedly had no conflicts, so patch was probably still good anyway, but I've made a new patch based off the latest in Mercurial to be safe, so should be good go.
On 2020-03-10 02:48:25 +0000, wrote:
SDL 2.0.12 is out soon. What is the current status on this? Does it support more than 4 XInput or should I stick to 2.0.10+patch ?
On 2020-03-10 03:39:28 +0000, Jimb Esser wrote:
Still no support for more than 4 XInput devices, so this patch is still the way to go if you require that. That being said, Steam Remote Play does not support RawInput, and with the recent introduction of Steam Remote Play Together, you can no longer detect a Steam Remote Play session at app startup / SDL initialization time (which the code in this patch handled previously) to fall back to something else, so if you also want to support Steam Remote Play Together, DirectInput is currently the only way to do so, so you need to give your users an option to switch to that instead of RawInput (but then, of course, they lose simultaneous triggers and rumble).
On 2020-03-11 02:16:08 +0000, wrote:
Does patch work with 2.0.12?
I think Parsec is better than Steam Remote Play. Although it's more effort to integrate, if you do it has its own input API so no player limit.
On 2020-03-11 02:38:49 +0000, Jimb Esser wrote:
This isn't the place to discuss the merits of Steam Remote Play, however, if you patch the SDL2.DLL in your Steam folder with this patch, it has absolutely no controller number limits =).
I haven't tested the patch on 2.0.12 myself.
On 2020-03-11 05:25:02 +0000, Sam Lantinga wrote:
Now that 2.0.12 is out, we'd like to look at this patch again. Jimb, can you update it so it applies cleanly and works with the latest code?
On 2020-03-11 17:58:23 +0000, Jimb Esser wrote:
I'm currently swamped on another (non-local-multiplayer) project at the moment, so hard to find time for this now, but should be able to take a look at it this weekend.
The SDL HIDAPI input code and HID 360 driver changed quite a bit, with a lot more coupling of high level logic with low level stuff, so the patch is not as elegant as it was previously, and required quite a bit of work to get working again, but, I think I've got it all working.
Caveats/comments from the commit:
New HID driver: xbox360w - no idea what that is, hopefully urelated
SDL_hidapijoystick.c had been refactored to couple data handling logic with device opening logic and device lists caused some problems, yields slightly uglier integration than previously when the 360 HID device driver was just handling the data.
SDL_hidapijoystick.c now often pulls the device off of the joystick_hwdata structure for some rumble logic, but it appears that code path is never reached, so probably not a problem.
Looks like joystick_hwdata was refactored to not include a mutex in other drivers, maintainers may want to do the same refactor here if that's useful for some reason.
Something changed in how devices get names, so getting generic names.
Had to fix a (new?) bug where removing an XInput controller caused existing controllers (that moved to a new XInput index) to get identified as 0x045e/0x02fd ("it's probably Bluetooth" in code), rendering the existing HIDAPI_IsDevicePresent and new RAWINPUT_IsDevicePresent unreliable.
On 2020-03-15 21:24:31 +0000, Jimb Esser wrote:
And, just to mention again (think I mentioned in the general discussion), this patch is basically two independent changes (each of which is not particularly useful without the other, but could be used independently):
Fix the correlation in SDL_hidapi_xbox360.c to be reliable, better catch guide presses, triggers, etc (not particularly useful on its own for Steam because that code path is disabled by default because that hidapi does not work in the background).
Add new high-level Windows joystick driver that uses RAWINPUT to get the same state packets HIDAPI gets, but in a reliable manner, and use the existing HIDAPI Xbox 360 driver to process them (not particularly useful on its own since it provides only very slightly better support (adds Xbox One in background) than DirectInput)
Together, of course, means we get all controllers supported in foreground or background, with XInput correlation reliably getting us accurate triggers, rumble, and guide button presses.
There is also the (still disabled, still requires the SDK to build) Windows.Gaming.Input code path, that should work (no recent testing, but was integrate to use the same, more reliable, correlation code last year, and I didn't have to touch anything around that to merge with the upstream master this weekend). If we generally get this working, would be worth getting that code path working without needing the Windows SDK installed, and that'd extend support for getting good triggers/rumble on more devices (just not Xbox One controllers when in the background).
And, as mentioned earlier, since Steam Remote Play Together isn't detectable in the same way Steam Remote Play was, defaulting to RAWINPUT on (SDL_HINT_JOYSTICK_RAWINPUT) is probably no longer a good default for Steam apps (great for Steam itself, great for non-Steam apps, and great for users not doing Remote Play Together, so nice to have as an option), at least until Steam adds support to their virtual controller for RAWINPUT games (of which there are plenty that don't use SDL, of course).
My base has diverged a bit, but it should apply pretty trivially.
On 2020-03-16 21:50:48 +0000, Sam Lantinga wrote:
Thanks! One more file...
./src/joystick/hidapi/SDL_hidapi_steam.c:975:49: error: too few arguments to function call, expected 3, have 2
return HIDAPI_JoystickConnected(device, NULL);
~~~~~~~~~~~~~~~~~~~~~~~~ ^
./src/joystick/hidapi/SDL_hidapijoystick_c.h:125:1: note: 'HIDAPI_JoystickConnected' declared here
extern SDL_bool HIDAPI_JoystickConnected(SDL_HIDAPI_Device *device, SDL_JoystickID *pJoystickID, SDL_bool is_external);
^
./src/joystick/hidapi/SDL_hidapi_steam.c:1133:69: error: too few arguments to function call, expected 3, have 2
HIDAPI_JoystickDisconnected(device, device->joysticks[0]);
~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
./src/joystick/hidapi/SDL_hidapijoystick_c.h:126:1: note: 'HIDAPI_JoystickDisconnected' declared here
extern void HIDAPI_JoystickDisconnected(SDL_HIDAPI_Device *device, SDL_JoystickID joystickID, SDL_bool is_external);
^
./src/joystick/hidapi/SDL_hidapi_steam.c:1170:1: warning: missing field 'PostUpdate' initializer
[-Wmissing-field-initializers]
};
^
On 2020-03-16 23:01:37 +0000, Jimb Esser wrote:
Ah, that one wasn't in the VS project, so I missed it with Find in Files ^_^. Earlier one I have no excuse for.
Maybe this should be talked about elsewhere (I've mentioned some of this in the Steamworks forums in the past, but not to this amount of detail), but anyway, here's what I see with Steam and RawInput today:
I did some testing on Steam with RawInput, and it seems a little better than the last time I tried, seems to at least not immediately break local Raw Input controllers anymore, but as soon as I turn off my Steam Link, I stop seeing WM_INPUT_DEVICE_CHANGE messages showing up even for local controllers, so no controllers can be added or removed. Also, Steam appears to not be generating/intercepting RawInput events - if I rebind buttons in Steam's controller config and force SteamInput on, it only affects the XInput and DInput controllers seen by the app, and if I play over SteamLink I see no RawInput controllers for the remote players. At a low level, I'm seeing 4 different devices showing up in the app when I turn on one Xbox 360 controller:
28de/11ff with a device name starting with \.\pipe\HID#VID_045E&PID_028E&IG_00 showing up in GetRawInputDeviceList()
045e/02a1 with a device name starting with \?\HID#VID_045E&PID_028E&IG_00 showing up in WM_INPUT and WM_INPUT_DEVICE_CHANGE events
28de/11ff in DirectInput
An XInput device (which SDL guesses is 28de/11ff from GetRawInputDeviceList())
The only major problem with the current code (if it wasn't disabling itself whenever it sees a Valve controller at startup) in an app running under Steam is if Steam Input is forced on, we see two controllers reaching SDL per device - one that's 0x045e/0x02a1 on \?\HID (the physical, presumably non-intercepted device that shows up in RawInput only) and a 0x28de/0x11ff device showing up in DirectInput/XInput, which is not reconciling (RAWINPUT_IsDevicePresent() doesn't think it's the same, or even a supported, device). If Steam Input was generating RawInput events for this controller, we could probably safely put in the same short-circuit I see in SDL_IsXInputDevice() looking for 0x28de/0x11ff, and just completely ignore these controllers in DirectInput/XInput if RawInput is enabled.
Generally, there exists a "polling"/"buffered" API to RawInput as well (polling GetRawInputBuffer() instead of WM_INPUT messages), and perhaps that is emulated by Steam, but that API has problems for general use (no background events for Xbox One controllers, IIRC), and is not used in this patch.
Reasonable tests are:
Use anything patched with this code, comment out "any_unsupported = SDL_TRUE;" in SDL_rawinputjoystick.c (or add the first controller/Remote Play user after launching the app), and you'll get two controllers showing up for a local Xbox controller (one which does not get remapped buttons from Steam Input), or just one (no RawInput controllers) for a remote user.
I've got a super simple fork of a RawInput test project forked that does some OutputDebugString calls when it gets devices added, but doesn't visually deal with multiple controllers: https://github.com/Jimbly/rawinput/tree/master/RawInputMessaged Can run that through Steam and see that it doesn't get remote play users or button remapping.
I've got a pretty robust SDL GameController test app here: https://github.com/Jimbly/SDL/blob/master/test/testgamecontroller2.cpp which when combined with the other changes in that fork (SDL_JOYSTICK_ANNOTATE_NAMES, specifically), let's you see exactly which joystick API is providing which device which I've been launching through Steam to test things. If you want to run that fork, load up VisualC/SDL.sln and build the DebugLIB configuration of testgamecontroller2 project and that's probably easiest (or copy it into another branch, but it's Win32-only console app, and C++, so not intended for general SDL use).
In the next Steam client beta update the raw controllers will be blocked if they are being handled by Steam, so you won't get duplicated controllers in that case.
Thanks!
The text was updated successfully, but these errors were encountered:
This bug report was migrated from our old Bugzilla tracker.
These attachments are available in the static archive:
Patch (2019-03-21) (sdl-rawinput.patch, text/plain, 2019-03-21 23:04:58 +0000, 94191 bytes)Patch (2019-06-12) (rawinput_flat3.patch, text/plain, 2019-06-12 22:36:58 +0000, 94198 bytes)Reported in version: HG 2.1
Reported for operating system, platform: Windows 10, x86
Comments on the original bug report:
On 2019-01-29 16:13:34 +0000, Jimb Esser wrote:
On 2019-03-21 23:04:58 +0000, Jimb Esser wrote:
On 2019-04-08 16:07:58 +0000, Jimb Esser wrote:
On 2019-06-12 03:37:59 +0000, Sam Lantinga wrote:
On 2019-06-12 04:03:35 +0000, Jimb Esser wrote:
On 2019-06-12 14:47:41 +0000, Sam Lantinga wrote:
On 2019-06-12 22:36:58 +0000, Jimb Esser wrote:
On 2019-06-12 22:38:32 +0000, Jimb Esser wrote:
On 2020-03-10 02:48:25 +0000, wrote:
On 2020-03-10 03:39:28 +0000, Jimb Esser wrote:
On 2020-03-11 02:16:08 +0000, wrote:
On 2020-03-11 02:38:49 +0000, Jimb Esser wrote:
On 2020-03-11 05:25:02 +0000, Sam Lantinga wrote:
On 2020-03-11 17:58:23 +0000, Jimb Esser wrote:
On 2020-03-12 16:23:38 +0000, Sam Lantinga wrote:
On 2020-03-15 21:03:57 +0000, Jimb Esser wrote:
On 2020-03-15 21:07:19 +0000, Jimb Esser wrote:
On 2020-03-15 21:24:31 +0000, Jimb Esser wrote:
On 2020-03-16 19:27:35 +0000, Sam Lantinga wrote:
On 2020-03-16 20:20:37 +0000, Sam Lantinga wrote:
On 2020-03-16 21:37:14 +0000, Jimb Esser wrote:
On 2020-03-16 21:50:48 +0000, Sam Lantinga wrote:
On 2020-03-16 23:01:37 +0000, Jimb Esser wrote:
On 2020-03-16 23:02:00 +0000, Jimb Esser wrote:
On 2020-03-16 23:05:57 +0000, Jimb Esser wrote:
On 2020-03-20 23:27:15 +0000, Jimb Esser wrote:
On 2020-03-21 02:50:00 +0000, Sam Lantinga wrote:
On 2020-03-25 16:07:58 +0000, Sam Lantinga wrote:
The text was updated successfully, but these errors were encountered: