Hi, The next stepping stone in my project to improve out-of-the-box gamecontroller support.
Background: On other platforms (Linux, MacOSX), SDL2 uses a GUID for gamecontrollers combined with a database [1] to provide the default button/axis mappings. This hasn't worked on OpenBSD (or apparently any *BSD) and the SDL2 code resorts to simply converting the first 16 chars of the device name to the GUID, which doesn't help with making use of the gamecontrollerdb [2]. Therefore, the way we have been handling this so far is that the env var SDL_GAMECONTROLLERCONFIG was available to hold a mapping string [3]. This poses a significant barrier, as users have to learn the syntax of the mapping strings and find one fitting their controller by trial and error mostly. This diff fixes the issue. To accomplish that, a couple of tasks are added: - Calculate and store the GUID (Credit goes to brynet@ for finding the GUID calculation and writing the initial proof-of-concept code). - Remove the workaround code in SDL_gamecontroller.c that would either read the mapping from SDL_GAMECONTROLLERCONFIG if present, or otherwise default to the (commonly used) XBox 360 controller layout. - Enable the mappings in SDL_gamecontrollerdb.h for OpenBSD. - Assign buttons correctly - not in the order that they appear in the HID report, but based on their numbering in the HID report descriptor. - Invert Y/Ry axes for the newer XInput-type gamecontrollers. This has been a particularly annoying problem, as USB standards recommend the positive directions to be left to right and far to near, but these controllers just use near-to-far as the positive direction. [4] The problem with the last bullet point is that the Y/Ry axis inversion needs to be applied *only* to the XInput devices, and *not* to the older DInput devices. I can't check for XBox360 and other interface subclasses (as used by the kernel in uhidev.c), as USB_GET_INTERFACE_DESC is not exposed in the uhid(4) driver (see uhid_do_ioctl() in uhid.c). The solution I came up with is admittedly a (minor) hack. It turns out that the old DInput devices report their axis values as an unsigned 8bit integer, while the newer ones use a signed 16bit integer. With my collection of gamecontrollers, checking if hitem.logical_maximum is greater than 255 allows to reliably pick the correct handling of the Y/Ry axes. Other caveats: - With the newly enabled GUID detection, SDL2 checks for a matching controller in gamecontrollerdb.txt in the application directory. I found that with one game (Cryptark), this contained bogus mappings for the XBox One controller (see the post on tech@), so some buttons and the Dpad didn't work until I removed that file. - For some reason, the GUID for DInput devices matches the MACOSX entries in SDL_gamecontrollerdb.h, but the XInput devices match the LINUX entries. It's gonna take me a bit of time to get to the bottom of this; in the meantime just allowing both sets of GUID mappings works for the devices here on my testing - The BSD_JoystickGetDeviceGUID() is not ideal at this point, as it takes the GUID stored as a string in char **joyguids and converts it into the raw numerical. I haven't figured out how to do the GUID calculation to avoid this additional transformation step yet. In the meantime, I don't notice performance issues and I don't think it's worth holding up the GUID mapping support over this. Testing: I tested this with sdl2-jstest (from sdl-jstest package) and few games (Owlboy, Cryptark) with these controllers that I have: XBox 360 controller (XInput), XBox One controller (XInput) (see separate mail on tech@), Logitech F310 (both DInput and XInput), Logitech Dual Action (DInput)). This maps everything correctly without additional configuration (as long as faulty gamecontrollerdb.txt in the game directory is removed). Testing with a variety of controllers would be useful to see if this really works across most of them as intended. If you do that, please let me know the mapping/functioning before and after the diff is applied. [1] https://hg.libsdl.org/SDL/file/2370522dac4c/src/joystick/SDL_gamecontrollerdb.h [2] https://hg.libsdl.org/SDL/file/2370522dac4c/src/joystick/bsd/SDL_bsdjoystick.c#l700 [3] https://marc.info/?l=openbsd-ports-cvs&m=152080802204959&w=2 [4] https://www.usb.org/sites/default/files/documents/hid1_11.pdf (page 20) Index: Makefile =================================================================== RCS file: /cvs/ports/devel/sdl2/Makefile,v retrieving revision 1.32 diff -u -p -r1.32 Makefile --- Makefile 6 Jan 2021 22:32:08 -0000 1.32 +++ Makefile 9 Jan 2021 16:09:20 -0000 @@ -5,6 +5,7 @@ COMMENT= cross-platform multimedia libra V= 2.0.14 DISTNAME= SDL2-${V} PKGNAME= sdl2-${V} +REVISION= 0 CATEGORIES= devel MASTER_SITES= https://www.libsdl.org/release/ Index: patches/patch-src_joystick_SDL_gamecontroller_c =================================================================== RCS file: patches/patch-src_joystick_SDL_gamecontroller_c diff -N patches/patch-src_joystick_SDL_gamecontroller_c --- patches/patch-src_joystick_SDL_gamecontroller_c 6 Jan 2021 22:32:08 -0000 1.7 +++ /dev/null 1 Jan 1970 00:00:00 -0000 @@ -1,50 +0,0 @@ -$OpenBSD: patch-src_joystick_SDL_gamecontroller_c,v 1.7 2021/01/06 22:32:08 thfr Exp $ - -enable GameController API the Linux fallback way (by posing as Xbox360 -controller) -also disable checking string "Xbox 360 Wireless Receiver", so for now -everything will be Xbox360 controller (works with generic joysticks) -map to SDL_GAMECONTROLLERCONFIG envvar if available -Use layout for XBox360 controller to maximize compatibility because -many controllers use this mapping - -Index: src/joystick/SDL_gamecontroller.c ---- src/joystick/SDL_gamecontroller.c.orig -+++ src/joystick/SDL_gamecontroller.c -@@ -968,7 +968,7 @@ static char *SDL_PrivateGetControllerGUIDFromMappingSt - SDL_memcpy(&pchGUID[8], &pchGUID[0], 4); - SDL_memcpy(&pchGUID[0], "03000000", 8); - } --#elif __MACOSX__ -+#elif defined(__MACOSX__) || defined(__OpenBSD__) - if (SDL_strlen(pchGUID) == 32 && - SDL_memcmp(&pchGUID[4], "000000000000", 12) == 0 && - SDL_memcmp(&pchGUID[20], "000000000000", 12) == 0) { -@@ -1131,17 +1131,21 @@ static ControllerMapping_t *SDL_PrivateGetControllerMa - ControllerMapping_t *mapping; - - mapping = SDL_PrivateGetControllerMappingForGUID(guid, SDL_FALSE); --#ifdef __LINUX__ -+#if defined(__LINUX__) || defined(__OpenBSD__) - if (!mapping && name) { -- if (SDL_strstr(name, "Xbox 360 Wireless Receiver")) { -- /* The Linux driver xpad.c maps the wireless dpad to buttons */ -- SDL_bool existing; -+ /* The Linux driver xpad.c maps the wireless dpad to buttons */ -+ SDL_bool existing; -+ char guid_str[1024]; -+ SDL_JoystickGetGUIDString(guid, guid_str, sizeof(guid_str)); -+ if (SDL_GetHint(SDL_HINT_GAMECONTROLLERCONFIG) == NULL) { - mapping = SDL_PrivateAddMappingForGUID(guid, --"none,X360 Wireless Controller,a:b0,b:b1,back:b6,dpdown:b14,dpleft:b11,dpright:b12,dpup:b13,guide:b8,leftshoulder:b4,leftstick:b9,lefttrigger:a2,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b10,righttrigger:a5,rightx:a3,righty:a4,start:b7,x:b2,y:b3", -+"none,XBox360 Controller,a:b7,b:b8,back:b1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b4,leftstick:b2,lefttrigger:a2,leftx:a0,lefty:a1~,rightshoulder:b5,rightstick:b3,righttrigger:a5,rightx:a3,righty:a4~,start:b0,x:b9,y:b10", - &existing, SDL_CONTROLLER_MAPPING_PRIORITY_DEFAULT); -+ } else { -+ mapping = SDL_PrivateAddMappingForGUID(guid, SDL_GetHint(SDL_HINT_GAMECONTROLLERCONFIG), &existing, SDL_CONTROLLER_MAPPING_PRIORITY_DEFAULT); - } - } --#endif /* __LINUX__ */ -+#endif /* __LINUX__ || __OpenBSD__ */ - - if (!mapping && name && !SDL_IsJoystickWGI(guid)) { - if (SDL_strstr(name, "Xbox") || SDL_strstr(name, "X-Box") || SDL_strstr(name, "XBOX")) { Index: patches/patch-src_joystick_SDL_gamecontrollerdb_h =================================================================== RCS file: patches/patch-src_joystick_SDL_gamecontrollerdb_h diff -N patches/patch-src_joystick_SDL_gamecontrollerdb_h --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-src_joystick_SDL_gamecontrollerdb_h 9 Jan 2021 16:09:20 -0000 @@ -0,0 +1,26 @@ +$OpenBSD$ + +enable controller detection by GUID on OpenBSD +use both LINUX and MACOSX guids to match both XInput and DInput devices + +Index: src/joystick/SDL_gamecontrollerdb.h +--- src/joystick/SDL_gamecontrollerdb.h.orig ++++ src/joystick/SDL_gamecontrollerdb.h +@@ -338,7 +338,7 @@ static const char *s_ControllerMappings [] = + "030000004f04000003d0000000000000,run'n'drive,a:b1,b:b2,back:b8,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b7,leftshoulder:a3,leftstick:b10,lefttrigger:b4,leftx:a0,lefty:a1,rightshoulder:a4,rightstick:b11,righttrigger:b5,rightx:a2,righty:a5,start:b9,x:b0,y:b3,", + "03000000101c0000171c000000000000,uRage Gamepad,a:b2,b:b1,back:b8,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b4,leftstick:b10,lefttrigger:b6,leftx:a0,lefty:a1,rightshoulder:b5,rightstick:b11,righttrigger:b7,rightx:a2,righty:a3,start:b9,x:b3,y:b0,", + #endif +-#if defined(__MACOSX__) ++#if defined(__MACOSX__) || defined(__OpenBSD__) + "03000000c82d00000090000001000000,8BitDo FC30 Pro,a:b0,b:b1,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:a4,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:a5,rightx:a2,righty:a3,start:b11,x:b3,y:b4,hint:SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", + "03000000c82d00000090000001000000,8BitDo FC30 Pro,a:b1,b:b0,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:a4,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:a5,rightx:a2,righty:a3,start:b11,x:b4,y:b3,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", + "03000000c82d00001038000000010000,8BitDo FC30 Pro,a:b0,b:b1,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:a5,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:a4,rightx:a2,righty:a3,start:b11,x:b3,y:b4,hint:SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", +@@ -479,7 +479,7 @@ static const char *s_ControllerMappings [] = + "03000000830500006020000000010000,iBuffalo SNES Controller,a:b1,b:b0,back:b6,dpdown:+a1,dpleft:-a0,dpright:+a0,dpup:-a1,leftshoulder:b4,rightshoulder:b5,start:b7,x:b3,y:b2,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", + "03000000830500006020000000000000,iBuffalo USB 2-axis 8-button Gamepad,a:b1,b:b0,back:b6,leftshoulder:b4,leftx:a0,lefty:a1,rightshoulder:b5,start:b7,x:b3,y:b2,", + #endif +-#if defined(__LINUX__) ++#if defined(__LINUX__) || defined(__OpenBSD__) + "03000000c82d00000090000011010000,8BitDo FC30 Pro,a:b0,b:b1,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:a4,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:a5,rightx:a2,righty:a3,start:b11,x:b3,y:b4,hint:SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", + "03000000c82d00000090000011010000,8BitDo FC30 Pro,a:b1,b:b0,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:a4,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:a5,rightx:a2,righty:a3,start:b11,x:b4,y:b3,hint:!SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", + "05000000c82d00001038000000010000,8BitDo FC30 Pro,a:b0,b:b1,back:b10,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,leftshoulder:b6,leftstick:b13,lefttrigger:a5,leftx:a0,lefty:a1,rightshoulder:b7,rightstick:b14,righttrigger:a4,rightx:a2,righty:a3,start:b11,x:b3,y:b4,hint:SDL_GAMECONTROLLER_USE_BUTTON_LABELS:=1,", Index: patches/patch-src_joystick_bsd_SDL_bsdjoystick_c =================================================================== RCS file: patches/patch-src_joystick_bsd_SDL_bsdjoystick_c diff -N patches/patch-src_joystick_bsd_SDL_bsdjoystick_c --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ patches/patch-src_joystick_bsd_SDL_bsdjoystick_c 9 Jan 2021 16:09:20 -0000 @@ -0,0 +1,128 @@ +$OpenBSD$ + +assign buttons correctly +get GUID using USB_GET_DEVICEINFO +detect newer (XInput-style) gamecontroller if hitem.logical_maximum is +> 255; if so invert y axes + +Index: src/joystick/bsd/SDL_bsdjoystick.c +--- src/joystick/bsd/SDL_bsdjoystick.c.orig ++++ src/joystick/bsd/SDL_bsdjoystick.c +@@ -83,6 +83,9 @@ + + #ifdef __OpenBSD__ + ++#define DEV_USB 3 /* needed to get GUID from USB_GET_DEVICEINFO */ ++#define GUID_LEN 32 /* GUID string has length 32 */ ++ + #define HUG_DPAD_UP 0x90 + #define HUG_DPAD_DOWN 0x91 + #define HUG_DPAD_RIGHT 0x92 +@@ -192,6 +195,9 @@ struct joystick_hwdata + + static char *joynames[MAX_JOYS]; + static char *joydevnames[MAX_JOYS]; ++#ifdef __OpenBSD__ ++static char joyguids[MAX_JOYS][GUID_LEN]; ++#endif + + static int report_alloc(struct report *, struct report_desc *, int); + static void report_free(struct report *); +@@ -217,11 +223,17 @@ BSD_JoystickInit(void) + { + char s[16]; + int i, fd; ++#ifdef __OpenBSD__ ++ struct usb_device_info di; ++#endif + + numjoysticks = 0; + + SDL_memset(joynames, 0, sizeof(joynames)); + SDL_memset(joydevnames, 0, sizeof(joydevnames)); ++#ifdef __OpenBSD__ ++ SDL_memset(joyguids, 0, sizeof(char) * MAX_JOYS * GUID_LEN); ++#endif + + for (i = 0; i < MAX_UHID_JOYS; i++) { + SDL_Joystick nj; +@@ -232,6 +244,22 @@ BSD_JoystickInit(void) + + if (BSD_JoystickOpen(&nj, numjoysticks) == 0) { + BSD_JoystickClose(&nj); ++#ifdef __OpenBSD__ ++ /* get GUID string and store it in joyguids */ ++ fd = open(s, O_RDONLY); ++ if (fd != -1) { ++ if (ioctl(fd, USB_GET_DEVICEINFO, &di) != -1) { ++ SDL_snprintf(joyguids[numjoysticks], ++ SDL_arraysize(joyguids[numjoysticks]), ++ "%02x%02x0000%02x%02x0000%02x%02x0000%02x%02x0000", ++ DEV_USB & 0xFF, DEV_USB >> 8, ++ di.udi_vendorNo & 0xFF, di.udi_vendorNo >> 8, ++ di.udi_productNo & 0xFF, di.udi_productNo >> 8, ++ di.udi_releaseNo & 0xFF, di.udi_releaseNo >> 8); ++ } ++ } ++ close(fd); ++#endif + numjoysticks++; + } else { + SDL_free(joynames[numjoysticks]); +@@ -544,6 +572,7 @@ BSD_JoystickUpdate(SDL_Joystick *joy) + Sint32 v; + #ifdef __OpenBSD__ + Sint32 dpad[4] = {0, 0, 0, 0}; ++ int actualbutton; + #endif + + #if defined(__FREEBSD__) || SDL_JOYSTICK_USBHID_MACHINE_JOYSTICK_H || defined(__FreeBSD_kernel__) || defined(__DragonFly_) +@@ -618,6 +647,18 @@ BSD_JoystickUpdate(SDL_Joystick *joy) + naxe = joy->hwdata->axis_map[joyaxe]; + /* scaleaxe */ + v = (Sint32) hid_get_data(REP_BUF_DATA(rep), &hitem); ++#ifdef __OpenBSD__ ++ /* XInput controllermapping relies on inverted Y axes. ++ * These devices have a 16bit signed space, as opposed ++ * to older DInput devices (8bit unsigned), so ++ * hitem.logical_maximum can be used to differentiate them. ++ */ ++ if ((joyaxe == JOYAXE_Y || joyaxe == JOYAXE_RY) ++ && hitem.logical_maximum > 255) { ++ if (v != 0) ++ v = ~v; ++ } ++#endif + v -= (hitem.logical_maximum + + hitem.logical_minimum + 1) / 2; + v *= 32768 / +@@ -652,7 +693,12 @@ BSD_JoystickUpdate(SDL_Joystick *joy) + } + case HUP_BUTTON: + v = (Sint32) hid_get_data(REP_BUF_DATA(rep), &hitem); ++#ifdef __OpenBSD__ ++ actualbutton = HID_USAGE(hitem.usage) - 1; /* sdl buttons are zero-based */ ++ SDL_PrivateJoystickButton(joy, actualbutton, v); ++#else + SDL_PrivateJoystickButton(joy, nbutton, v); ++#endif + nbutton++; + break; + default: +@@ -697,11 +743,16 @@ static SDL_JoystickGUID + BSD_JoystickGetDeviceGUID( int device_index ) + { + SDL_JoystickGUID guid; ++#ifdef __OpenBSD__ ++ guid = SDL_JoystickGetGUIDFromString(joyguids[device_index]); ++ return guid; ++#else + /* the GUID is just the first 16 chars of the name for now */ + const char *name = BSD_JoystickGetDeviceName( device_index ); + SDL_zero( guid ); + SDL_memcpy( &guid, name, SDL_min( sizeof(guid), SDL_strlen( name ) ) ); + return guid; ++#endif + } + + static int
