Patch to fix HOTAS / button/axis limit problems

[Andrew Eikum]

Hey Cláudio,

Thanks for working on this. I think you're on the right track. I have
some thoughts below.

On Sun, May 17, 2020 at 12:24:07PM +0200, Cláudio Sampaio wrote:
> Hi all, I am a heavy user of wine and proton for games and for some time
> the failures some games had with my HOTAS (Hands On Throttle-And-Stick)
> have been bothering me. I tried to take a look at wine code and found out
> that the maximum numbers of buttons for a single device was hardwired to 32
> and the maximum number of analog axes was hardwired to 4 pairs (8 axes). My
> own HOTAS, a very common Logitech X52 Pro, has a bigger number of buttons
> -- 39, so the higher buttons would "carry over" to register like they were
> lower buttons -- and this prevented it from working on games. I increased
> these hardwired numbers to 64 and 8 respectively, and the patch seems to
> have largely worked on all games I used it -- for example, I am only able
> to play Elite: Dangerous with my patched proton version.
> 

Maybe you know this, but one thing to be careful of when working with
both Proton and upstream is that Proton uses an SDL backend for dinput
which is not in upstream Wine. If you're working in upstream, you'll
be using the Linux input API backend. If you're making changes in
dinput (see below), you'll likely need to implement your fix in both
places, once in joystick_linux.c for upstream, and again in
joystick_sdl.c for Proton.

(The correct fix for this is to implement dinput on top of hidapi, but
that's a whole huge task in itself.)

> However, there have been some problems, like a few sliders not working --
> slider 7 (you could check the numbering of axes with a program like
> antimicro on Linux) does not seem to register in Star Citizen, for example.
> I noticed from joystick.c that the analog axes seem to have specific names
> and functionality, like DIJOFS_RX and DIJOFS_SLIDER, and if I understood
> what's going on, it might prevent the code to work generally with obscure
> devices that have a couple number of sliders, is that correct? How could I
> get the code to recognize and translate my axes to games?
> 

Dinput has a concept of "data formats", which map hardware values
(axis N) to logical values (left joystick X position). The most
commonly used format is called "dfDIJoystick2", which you can find in
<dlls/dinput/data_formats.c>. Unfortunately this gets super
complicated and Wine's support here isn't great. Notice that it has 6
axes and 2 sliders up at the top, and then a bunch more down at the
bottom.c.  A lot of the Wine handling for data formats assumes that
we'll only be using those top few items, for example see the axis
handling in <dlls/dinput/joystick_linux.c:alloc_device>. I think these
will need to be corrected to use the correct data formats for these
extra axes.

You're right that there's likely more assumptions like that made
elsewhere in dinput. The code there is very old and not the best
quality.

> Also, for this patch to be complete I will also need to change the GUI for
> testing joystick via "wine control", and I do not have much experience with
> Windows programming. Any help would be welcome.
> 

This would be nice, but I don't think it's a requirement to fix the
problem in upstream.

Some further comments on your patch follow...

> I have also described the problem on this forum post:
> https://forum.winehq.org/viewtopic.php?f=2&t=33615&p=126895#p126895
> And the bug report for it at https://bugs.winehq.org/show_bug.cgi?id=48799
> And I have also talked about it on gamingonlinux:
> https://www.gamingonlinux.com/forum/topic/4330
> 
> This patch has around 50 to 60 hours of testing.
> -- 
> Cláudio "Patola" Sampaio
> MakerLinux Labs - Campinas, SP
> Resume <https://github.com/Patola/resume> Gmail <patola at gmail.com> -
> MakerLinux <patola at makerlinux.com.br> - YOUTUBE
> <https://www.youtube.com/user/makerlinux>!
> Facebook <https://www.facebook.com/patolinux> - Facebook da MakerLinux
> <https://www.facebook.com/makerlinux> - Lattes
> <http://buscatextual.cnpq.br/buscatextual/visualizacv.do?id=K4763932Z5>

> diff --git a/dlls/joy.cpl/joy.h b/dlls/joy.cpl/joy.h
> index ec7af4f787..672e8995e1 100644
> --- a/dlls/joy.cpl/joy.h
> +++ b/dlls/joy.cpl/joy.h
> @@ -47,8 +47,8 @@ struct Joystick {
>      struct Effect *effects;
>  };
>  
> -#define TEST_MAX_BUTTONS    32
> -#define TEST_MAX_AXES       4
> +#define TEST_MAX_BUTTONS    64
> +#define TEST_MAX_AXES       8
>  
>  struct Graphics {
>      HWND hwnd;
> diff --git a/dlls/joy.cpl/main.c b/dlls/joy.cpl/main.c
> index 4ad9cf848c..35dba75978 100644
> --- a/dlls/joy.cpl/main.c
> +++ b/dlls/joy.cpl/main.c
> @@ -414,6 +414,16 @@ static DWORD WINAPI input_thread(void *param)
>          axes_pos[1][1] = state.lRy;
>          axes_pos[2][0] = state.lZ;
>          axes_pos[2][1] = state.lRz;
> +	axes_pos[3][0] = 0;
> +	axes_pos[3][1] = 0;
> +	axes_pos[4][0] = 0;
> +	axes_pos[4][1] = 0;
> +	axes_pos[5][0] = 0;
> +	axes_pos[5][1] = 0;
> +	axes_pos[6][0] = 0;
> +	axes_pos[6][1] = 0;
> +	axes_pos[7][0] = 0;
> +	axes_pos[7][1] = 0;
>  

Small point here, since this is an RFC patch, but in general be
careful about tabs vs spaces and following nearby formatting.

>          /* Set pov values */
>          for (j = 0; j < ARRAY_SIZE(pov_val); j++)
> diff --git a/dlls/winebus.sys/bus_sdl.c b/dlls/winebus.sys/bus_sdl.c
> index 0560e4bb12..13d3377b03 100644
> --- a/dlls/winebus.sys/bus_sdl.c
> +++ b/dlls/winebus.sys/bus_sdl.c
> @@ -402,10 +402,10 @@ static BOOL build_report_descriptor(struct platform_private *ext)
>      report_size = 0;
>  
>      axis_count = pSDL_JoystickNumAxes(ext->sdl_joystick);
> -    if (axis_count > 6)
> +    if (axis_count > 16)
>      {
> -        FIXME("Clamping joystick to 6 axis\n");
> -        axis_count = 6;
> +        FIXME("Clamping joystick to 16 axis\n");
> +        axis_count = 16;
>      }
>  
>      ext->axis_start = report_size;
> @@ -421,9 +421,9 @@ static BOOL build_report_descriptor(struct platform_private *ext)
>      ext->ball_start = report_size;
>      if (ball_count)
>      {
> -        if ((ball_count*2) + axis_count > 9)
> +        if ((ball_count*2) + axis_count > 19)
>          {
> -            FIXME("Capping ball + axis at 9\n");
> +            FIXME("Capping ball + axis at 19\n");
>              ball_count = (9-axis_count)/2;
>          }
>          descript_size += sizeof(REPORT_AXIS_HEADER);

These are clamped to those values because they access the
joystick_usages array, which is only 9 members long. Increasing those
without changing that array usage will cause invalid accesses.
Probably it should be rewritten not to use a hard-coded array, or
generate useful values past the array.

> diff --git a/dlls/winejoystick.drv/joystick_linux.c b/dlls/winejoystick.drv/joystick_linux.c
> index 8d1a7b1a25..e579d99aa7 100644
> --- a/dlls/winejoystick.drv/joystick_linux.c
> +++ b/dlls/winejoystick.drv/joystick_linux.c

These driver files are only used by the winmm joystick driver, which
is probably not relevant to your problem.

> @@ -260,9 +260,9 @@ LRESULT driver_joyGetDevCaps(DWORD_PTR dwDevID, LPJOYCAPSW lpCaps, DWORD dwSize)
>  	lpCaps->wUmax = 0xFFFF;
>  	lpCaps->wVmin = 0;
>  	lpCaps->wVmax = 0xFFFF;
> -	lpCaps->wMaxAxes = 6; /* same as MS Joystick Driver */
> +	lpCaps->wMaxAxes = 16; /* same as MS Joystick Driver */
>  	lpCaps->wNumAxes = 0; /* nr of axes in use */
> -	lpCaps->wMaxButtons = 32; /* same as MS Joystick Driver */
> +	lpCaps->wMaxButtons = 64; /* same as MS Joystick Driver */
>  	lpCaps->szRegKey[0] = 0;
>  	lpCaps->szOEMVxD[0] = 0;
>  	lpCaps->wCaps = 0;
> @@ -326,6 +326,7 @@ LRESULT driver_joyGetPosEx(DWORD_PTR dwDevID, LPJOYINFOEX lpInfo)
>  	    switch (jstck->axesMap[ev.number]) {
>  	    case 0: /* X */
>  	    case 8: /* Wheel */
> +            case 40: /* Mouse-like */
>  		jstck->x = ev.value;
>  		break;
>  	    case 1: /* Y */

Andrew