From: "Rémi Bernon" Subject: Re: [PATCH 4/4] hidclass.sys: Add input.inf that matches all HID devices. Message-Id: <205fb95b-1624-b12a-c2f8-a4d00f4c1c21@codeweavers.com> Date: Sat, 22 Jan 2022 07:36:37 +0100 In-Reply-To: <20220121231724.qxxu5rc4ljo2qhsk@failcraft> References: <20220121161044.911068-1-ahiler@codeweavers.com> <20220121161044.911068-4-ahiler@codeweavers.com> <7824dacf-59d7-f2fb-a935-570907ff381c@codeweavers.com> <20220121231724.qxxu5rc4ljo2qhsk@failcraft> On 1/22/22 00:17, Arkadiusz Hiler wrote: > On Sat, Jan 22, 2022 at 12:01:12AM +0100, Rémi Bernon wrote: >> On 1/21/22 17:10, Arkadiusz Hiler wrote: >>> This makes it so that all the devices on the HID bus now have Class, >>> ClassGUID, Driver and DriverDesc register properties populated. >>> >>> Without having at least Driver and Class set SDL2 refuses to use HID >>> devices directly. >>> >>> Signed-off-by: Arkadiusz Hiler >>> --- >>> dlls/hidclass.sys/Makefile.in | 2 ++ >>> dlls/hidclass.sys/hidclass.rc | 20 ++++++++++++++++++++ >>> dlls/hidclass.sys/input.inf | 13 +++++++++++++ >>> loader/wine.inf.in | 1 + >>> 4 files changed, 36 insertions(+) >>> create mode 100644 dlls/hidclass.sys/hidclass.rc >>> create mode 100644 dlls/hidclass.sys/input.inf >>> >>> diff --git a/dlls/hidclass.sys/Makefile.in b/dlls/hidclass.sys/Makefile.in >>> index 25a0396dabe..788828ad66a 100644 >>> --- a/dlls/hidclass.sys/Makefile.in >>> +++ b/dlls/hidclass.sys/Makefile.in >>> @@ -5,3 +5,5 @@ IMPORTS = hal ntoskrnl user32 hidparse >>> C_SRCS = \ >>> device.c \ >>> pnp.c >>> + >>> +RC_SRCS = hidclass.rc >>> diff --git a/dlls/hidclass.sys/hidclass.rc b/dlls/hidclass.sys/hidclass.rc >>> new file mode 100644 >>> index 00000000000..409bdc16b45 >>> --- /dev/null >>> +++ b/dlls/hidclass.sys/hidclass.rc >>> @@ -0,0 +1,20 @@ >>> +/* >>> + * Copyright 2022 Arkadiusz Hiler >>> + * >>> + * This library is free software; you can redistribute it and/or >>> + * modify it under the terms of the GNU Lesser General Public >>> + * License as published by the Free Software Foundation; either >>> + * version 2.1 of the License, or (at your option) any later version. >>> + * >>> + * This library is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >>> + * Lesser General Public License for more details. >>> + * >>> + * You should have received a copy of the GNU Lesser General Public >>> + * License along with this library; if not, write to the Free Software >>> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA >>> + */ >>> + >>> +/* @makedep: input.inf */ >>> +1 WINE_DATA_FILE input.inf >>> diff --git a/dlls/hidclass.sys/input.inf b/dlls/hidclass.sys/input.inf >>> new file mode 100644 >>> index 00000000000..d9222b13672 >>> --- /dev/null >>> +++ b/dlls/hidclass.sys/input.inf >>> @@ -0,0 +1,13 @@ >>> +[Version] >>> +Signature="$CHICAGO$" >>> +ClassGuid={745a17a0-74d3-11d0-b6fe-00a0c90f57da} >>> +Class=HIDClass >>> + >>> +[Manufacturer] >>> +Wine=mfg_section >>> + >>> +[mfg_section] >>> +Wine HID device=device_section,HID\ >>> + >>> +[device_section.Services] >>> +AddService = ,0x2 >>> diff --git a/loader/wine.inf.in b/loader/wine.inf.in >>> index c0251934dfc..afa344ffa80 100644 >>> --- a/loader/wine.inf.in >>> +++ b/loader/wine.inf.in >>> @@ -5717,6 +5717,7 @@ protocol,"@%11%\ws2_32.dll,-3" >>> services,"@%11%\ws2_32.dll,-4" >>> [InfFiles] >>> +input.inf,"@%12%\hidclass.sys,-1" >>> winebus.inf,"@%12%\winebus.sys,-1" >>> winehid.inf,"@%12%\winehid.sys,-1" >>> wineusb.inf,"@%12%\wineusb.sys,-1" >> >> Looking at the SDL hidapi code it looks like they have much more HID devices >> than just the DS4 and DS5. >> >> Isn't there a risk, if we satisfy all the requirements, that this would make >> SDL try using the devices through HID where we don't support the same >> reports, when it would currently just fallback to XInput / DInput? > > There's always a risk, but we were doing this for a long time as a hack > in Proton 6.3 and there were no complaints. FWIW SDL always handles IG_ > devices through xinput. > Yeah, though Proton has a device filtering which isn't upstream, where most devices are going through hidraw when possible, and when not through SDL. I think we would want some of it to be upstream, as the Sony controllers for instance, are better handled when used through hidraw, but I would like to keep things more flexible than Proton. I also don't know if anyone has tried the XBox wireless controllers ever, which are the one the SDL hidapi code seem to support and which I'm a bit skeptical whether they would work through hidraw. > Maybe we should not match devices that come with fake HID reports built > on top of SDL / evdev? We could solve that by having a specific > CompatibleID assigned for the real HID devices and then using that as a > matcher in the INF's model section. > Yeah that sounds like a good idea but although we could report some specific compatible id on the winebus.sys level for raw / pass-through devices, they will not be directly visible on the top HID devices. > Some context on why I want to do this: some games provide custom SDL > mappings or detect the controller type using SDLJoystickGUID. GUID's > last two bytes depend on the API SDL is using. Hades for example won't > display correct button glyphs for Sony controllers that are not handled > through HID. That's why the hacks were originally added to Proton. > Sure, thanks for the details! -- Rémi Bernon