Re: xbox 360 emulation with logitech cordless and dinput joy disable

[Stefan Dösinger]

Hi,

The patch needs a few improvements before it can be accepted into Wine. I 
can't comment on the dinput / xinput specifics of the patch, so this is a non-
exhaustive list of general suggestions.

Concerning the big picture, this patch needs a few discussions about 
xinput.dll design. There have been some discussions about that in the past, 
and I hope that some dinput devs can help out.

Am Dienstag, 12. Juni 2012, 23:48:53 schrieb Giovanni Ongaro:
> From: root <root@joe.(none)>

Please configure git correctly, see http://wiki.winehq.org/GitWine section 
3.3.

Also consider using a non-root user, but that's your personal setup, so not 
really my business.
> -
> if (getenv("LINIXFORCENOJOY") != NULL) return FALSE; /*JoeFix disable 

joystick*/
The problem with dinput vs xinput conflicts should probably be solved in a 
different manner. That aside, such configurations are usually done via 
registry keys. Also comments like /* I changed this here */ are useless, the 
git history stores this information anyway.
> - <...> //JoeFix DEAD ZONE

This is a C++ comment and not allowed in Wine. See 
http://wiki.winehq.org/SubmittingPatches
> - fdJoy=open("/dev/input/js0",O_RDONLY | O_NONBLOCK);

Xinput.dll should not open joystick devices directy, and definitely not a 
hardcoded one. I don't know if there's consensus about how xinput should be 
implemented, but basing it on top of dinput might be a start.

A good start would be to write a few tests to test how Windows announces(or 
doesn't announce) Xbox gamepads and other gamepads/joysticks to games. My 
understanding is that xinput.dll only works for Xbox gamepads. Do those show 
up in dinput on Windows?
> +#define DEAD_ZONE 4096
> ...
> -
>       if (e.number == 4) {
> -
>             if (e.value == 1) {
> -
>               FIXME("TRIGGER LEFT PRESSED\n");
> -
>               pState->Gamepad.bLeftTrigger=32767;
> -
>             }
> -
>             else {
> -
>               pState->Gamepad.bLeftTrigger=0;
> -
>             }
>
> ...
> +#define JOYMAX 30000
> +#define JOYMIN -30000

The hardcoded numbers look questionable. The 4 and 1 are probably specific to 
the joystick, the 32767 might have a symbolic name like XINPUT_MAX or 
something.

A part of this seems to be needed due to the attempt to emulate an xbox 
gamepad with a different device. Personally I'm not opposed to doing that, and 
you might need some device specific tricks to pull this off. However, this 
needs a more structured approach than hardcoding the values needed for one 
specific Logitech device in the code.

Best regards,
Stefan

xbox 360 emulation with logitech cordless and dinput joy disable

[Roderick Colenbrander]

On Wed, Jun 13, 2012 at 8:52 AM, Stefan Dösinger  wrote:
> Hi,
>
> The patch needs a few improvements before it can be accepted into Wine. I
> can't comment on the dinput / xinput specifics of the patch, so this is a non-
> exhaustive list of general suggestions.
>
> Concerning the big picture, this patch needs a few discussions about
> xinput.dll design. There have been some discussions about that in the past,
> and I hope that some dinput devs can help out.
>
> Am Dienstag, 12. Juni 2012, 23:48:53 schrieb Giovanni Ongaro:
> > From: root <root@joe.(none)>
>
> Please configure git correctly, see http://wiki.winehq.org/GitWine section
> 3.3.
>
> Also consider using a non-root user, but that's your personal setup, so not
> really my business.
> > - if (getenv("LINIXFORCENOJOY") != NULL) return FALSE; /*JoeFix disable
>
> joystick*/
> The problem with dinput vs xinput conflicts should probably be solved in a
> different manner. That aside, such configurations are usually done via
> registry keys. Also comments like /* I changed this here */ are useless, the
> git history stores this information anyway.
> > - <...> //JoeFix DEAD ZONE
>
> This is a C++ comment and not allowed in Wine. See
> http://wiki.winehq.org/SubmittingPatches
> > - fdJoy=open("/dev/input/js0",O_RDONLY | O_NONBLOCK);
>
> Xinput.dll should not open joystick devices directy, and definitely not a
> hardcoded one. I don't know if there's consensus about how xinput should be
> implemented, but basing it on top of dinput might be a start.

Xinput should not be layered on top of dinput. On Windows it is really
a separate relatively low-level dll. The main problem with layering it
on top of dinput is that we have to invent some Wine-specific
extensions for force feedback which differs from dinput (as far as I
remember).

Definitely xbox support should not use the classic JS interface, but
the event interface (that's also the only one which can be used for
other xbox features like the rumble stuff, xbox leds, ..).
> A good start would be to write a few tests to test how Windows announces(or
> doesn't announce) Xbox gamepads and other gamepads/joysticks to games. My
> understanding is that xinput.dll only works for Xbox gamepads. Do those show
> up in dinput on Windows?

Xbox gamepads show up twice. Once the xbox360 gamepad driver is
loaded, it 'injects' a fake USB Hid gamepad device which is used by
dinput (and likely the legacy joystick API). There is no good way to
differentiate between the two gamepads. Microsoft recommends
applications which support both dinput and xinput to do enumeration
through dinput and then use WMI to check if the gamepad is an xbox
one. This doesn't work on Wine because we don't have proper WMI
support (yet).
> > +#define DEAD_ZONE 4096
> > ...
> > - if (e.number == 4) {
> > - if (e.value == 1) {
> > - FIXME("TRIGGER LEFT PRESSED\n");
> > - pState->Gamepad.bLeftTrigger=32767;
> > - }
> > - else {
> > - pState->Gamepad.bLeftTrigger=0;
> > - }
> >
> > ...
> > +#define JOYMAX 30000
> > +#define JOYMIN -30000
>
> The hardcoded numbers look questionable. The 4 and 1 are probably specific to
> the joystick, the 32767 might have a symbolic name like XINPUT_MAX or
> something.

The event interface provides proper limits for each axis. I think the
js interface has too (forgot the specifics).

Definitely axis and button names can't be hard coded to numbers. As I
mention later on these numbers (especially through the js-interface)
are device specific. Use BTN_A, ABS_X, and so on, but it is impossible
to do this for different gamepads.
> A part of this seems to be needed due to the attempt to emulate an xbox
> gamepad with a different device. Personally I'm not opposed to doing that, and
> you might need some device specific tricks to pull this off. However, this
> needs a more structured approach than hardcoding the values needed for one
> specific Logitech device in the code.

There are a lot of gamepads from different vendors which are xinput
compatible. Supporting all of them is easy since they all use the same
button names since they use the same driver. We can support non-xinput
compatible ones, but it easily becomes tricky. What do you do if a
gamepad doesn't have the same buttons? How do you map different button
names (xpad devices use BTN_A/BTN_B, while others use BTN_BASE,
BTN_BASE2, ..) to the layout of the gamepad? Further input ranges
could in theory be different (you can scale values, but you could get
still run into issues).

Supporting non-xinput devices can be done, but I'm reluctant to doing
it in Wine. It is impossible to get it right (every device is
different) and it would be complicated for users too.

Then there is the whole design part. We had this discussion a couple
of times already. Alexandre used to be reluctant about adding
OS-specific code to xinput. Personally I think it is the only way to
properly implement xinput. Even though the xinput API is very easy
(mostly a single poll call) implementing it is tricky. How are you
going to open the device? There is nothing like a 'create device' and
'destroy device' call. Do you keep device handles open or do you
open/close every call? How do you deal with plug-and-play? The API is
designed for hot plugging devices and even the device number of a
gamepad can change when you plug in more devices.

The old event based interface patches floating around wine-patches
were not bad. At some point I cleaned them up, but didn't submit them
because we didn't finish the discussion on what the design should be.

Roderick
> Best regards,
> Stefan

xbox 360 emulation with logitech cordless and dinput joy disable

[Giovanni Ongaro]

Thanks for the comments i could put the hardcoded values into registry
keys if you are really interested in the patch, i know hardcoding is not
an good practice but i made the patch to suit my needs and published it
later for the community, not pretending it was ok, but i tested it with
many game and it works pretty well