Re: winejoystick.drv: Add axis remapping support

[Vitaliy Margolen]

Adolfo R. Brandes wrote:
> Greetings!
>
> The purpose of this patch is to add registry axis remapping and POV
> hat support to winejoystick.drv, much like what's already in place for
> dinput/joystick_linux.c.  I needed this for X-Wing Alliance
> (http://appdb.winehq.org/objectManager.php?sClass=application&iId=2110),
> which recognizes my Thrustmaster Top Gun Fox 2 Pro USB joystick, but
> not the POV hat and with the Rudder and Throttle axes switched around.
>
> It uses the following conventions:
>
> HKCU\Software\Wine\Joystick<joyname> = <axeslist>
>
> or
>
> HKCU\Software\Wine\AppDefaults\app.exe\Joystick<joyname> = <axeslist>
>
> Where <joyname> is the device name (as shown in jstest), and
> <axeslist> is a comma-separated list of axis names.  However, in
> contrast to joystick_linux.c, the axis names are the same as in
> jstest.  For example, in my case jstest reports:

Don't do that. Keep it uniform. As-is it's a pain to configure. You want to 
introduce even more?

First few general notes about your patch - do not mix tabs and spaces! Set 
tab width to be _always_ 8 spaces! For best results just don't use tabs. Do 
not change formating, of the function unless you changing big part of it and 
you changing it to default Wine formating (4 spaces, curly brackets on their 
own line).
> +static char *axis_name(char *str, unsigned char type, int len) {
> -
>   switch(type) {
> -
>           case ABS_X:
> -
>                   memcpy(str,"X",len - 1);
> -
>                   break;
> -
>           case ABS_Y:

The way you using this function it makes much more sense to return static 
text ex:
static const char *axis_name(unsigned char type)
{
     switch (type)
     {
         case ABS_X: return "X";
         case ABS_Y: return "Y";
         case ABS_Z: return "Z";
         default: return "?";
     }
}

BTW what will that "?" do? I don't see you comparing it against anything.
> -
>                   else
> -
>                           ERR("Not all joystick axes mapped: %d axes, %d arguments\n", nrOfAxes, j);

Disabling axis might not be a bad thing, especially for testing. I don't 
think you need to worry about extra axis on the device that are not mapped 
to anything.
> -
>   for (i = 0; i < nrOfAxes; i++)
> -
>           switch(axis_map[i]) {
> -
>           case ABS_HAT0X:
> -
>           case ABS_HAT0Y:
> -
>                   lpCaps->wCaps |= JOYCAPS_HASPOV;
> -
>                   break;
> -
>           }

You need both X and Y axis for it to work. You have to check that both are 
present. Also what about multiple POVs? Here you checking for only one.
>     switch(nrOfAxes) {
> -
>   case 6: lpCaps->wCaps |= JOYCAPS_HASV;
> -
>   case 5: lpCaps->wCaps |= JOYCAPS_HASU;
> -
>   case 4: lpCaps->wCaps |= JOYCAPS_HASR;
> -
>   case 3: lpCaps->wCaps |= JOYCAPS_HASZ;
> -
>       /* FIXME: don't know how to detect for
> -
>          JOYCAPS_HASPOV, JOYCAPS_POV4DIR, JOYCAPS_POVCTS */
> -
>   case 6:
> -
>           lpCaps->wCaps |= JOYCAPS_HASV;
> -
>           lpCaps->wVmax = 0xFFFF;
> -
>   case 5:
> -
>           lpCaps->wCaps |= JOYCAPS_HASU;
> -
>           lpCaps->wUmax = 0xFFFF;
> -
>   case 4:
> -
>           lpCaps->wCaps |= JOYCAPS_HASR;
> -
>           lpCaps->wRmax = 0xFFFF;
> -
>   case 3:
> -
>           lpCaps->wCaps |= JOYCAPS_HASZ;
> -
>           lpCaps->wZmax = 0xFFFF;

Now that you can remap axis it's not guaranteed that 6th axes is the speed 
and so on.

Vitaliy.

winejoystick.drv: Add axis remapping support

[Adolfo R. Brandes]

Hi there,

On Wed, Feb 27, 2008 at 3:44 PM, Vitaliy Margolen
 wrote:
> Don't do that. Keep it uniform. As-is it's a pain to configure. You want to
>  introduce even more?

Actually, this was my shot at improving that particular aspect of
the configuration.  As it is in dinput/joystick_linux.c, it's a bit of
a shot in the dark as to what is "Slider1", "Slider2", and so on.  The
way I did it, you can run jstest to find out what each of your axes
are named, and use those names in regedit.

I realize this introduces confusion.  It's just that I think the
previous standard requires a bit of a revision, in that there should
be a more reliable way of determining what each axis in your joystick
is called (and what wine expects it to be called).  Maybe
joystick_linux should be patched too?
> do not mix tabs and spaces!

Sorry about that, you're right.  I'm used to using tabs, and have
just found out the hard way that wine uses 4 spaces. ;)
> The way you using this function it makes much more sense to return static
>  text ex:
>  static const char *axis_name(unsigned char type)

Good idea, it really does make much more sense.
> BTW what will that "?" do? I don't see you comparing it against anything.

The "?"s represent gaps in the ABS_* series of constants in
linux/input.h, which is the basis for the naming scheme I followed
(and jstest also follows, which is the whole point).  The premise is
that all axes will always be properly identified by linux as one of
those constants.  The "?" is just a way of failing on purpose, i.e.,
when an axis is not properly named.
> Disabling axis might not be a bad thing, especially for testing. I don't
>  think you need to worry about extra axis on the device that are not mapped
>  to anything.

I was worried about having two (or more) linux axes mapped to the
same wine one.  That's the reason behind the fatal error.
> You need both X and Y axis for [a POV] to work. You have to check that both are
>  present.

You're right, I better improve the logic here, too.
> Also what about multiple POVs? Here you checking for only one.

It seems that the only pertinent JOYINFOEX constants from mmsystem.h
are "JOY_HASPOV" and "JOY_RETURNPOV". No "JOY_HASPOV2" ou
"JOY_RETURNPOV2". I'm assuming that more hats simply won't register.
Bear with me, everything I know about the Windows joystick API has
been gathered from reading this driver, and it seems like there's a
fundamental difference between DirectInput devices and "regular" ones.
 Correct me if I'm wrong, but it looks to me like multiple POVs (or
more than 6 axes) aren't supported here.

However, it makes sense for the user to map HAT2X and HAT2Y to the
"u" and "v" axes, for example.  I'll see what I can do.
> Now that you can remap axis it's not guaranteed that 6th axes is the speed
>  and so on.

Yes, that's correct.  I previously tried a different approach,
basing the selection of x, y, z, r, u, and v on what the axis is
actually named (i.e., "Throttle" = z, "Rudder" = r), but that actually
moots the whole point.  It turns out that it doesn't matter what the
axis' nominal function is; what matters is the order in which they
appear.  For instance, my joystick's rudder is actually named "Rz",
which would wrongly map to "v" (as opposed to "r").  What's important
is not for wine to know the difference between "Throttle" and
"Rudder", but for the user to know what each axis is named and in what
order they are presented to the application, so that he can then
reoder them as he pleases.

I'm gonna work on a better version of the patch, and thanks for the tips!

Adolfo

winejoystick.drv: Add axis remapping support

[Vitaliy Margolen]

Adolfo R. Brandes wrote:
> Hi there,
>
> On Wed, Feb 27, 2008 at 3:44 PM, Vitaliy Margolen
>  wrote:
> > Don't do that. Keep it uniform. As-is it's a pain to configure. You want to
> >  introduce even more?
>
> Actually, this was my shot at improving that particular aspect of
> the configuration.  As it is in dinput/joystick_linux.c, it's a bit of
> a shot in the dark as to what is "Slider1", "Slider2", and so on.  The
> way I did it, you can run jstest to find out what each of your axes
> are named, and use those names in regedit.
>
> I realize this introduces confusion.  It's just that I think the
> previous standard requires a bit of a revision, in that there should
> be a more reliable way of determining what each axis in your joystick
> is called (and what wine expects it to be called).  Maybe
> joystick_linux should be patched too?

I'm not sure I like this. The names dinput joystick uses are the names 
windows uses in number of place. Also they are mentioned on MSDN and part of 
the API. Also for backwards compatibility if you change dinput joystick that 
means it will break all current configurations.

Not that I like the way it's done now. But it needs to be part of the 
winecfg for example. Then it wouldn't matter what we use internally.
> > Disabling axis might not be a bad thing, especially for testing. I don't
> >  think you need to worry about extra axis on the device that are not mapped
> >  to anything.
>
> I was worried about having two (or more) linux axes mapped to the
> same wine one.  That's the reason behind the fatal error.

Then you check for that particular case.
> > Also what about multiple POVs? Here you checking for only one.
>
> It seems that the only pertinent JOYINFOEX constants from mmsystem.h
> are "JOY_HASPOV" and "JOY_RETURNPOV". No "JOY_HASPOV2" ou
> "JOY_RETURNPOV2". I'm assuming that more hats simply won't register.
> Bear with me, everything I know about the Windows joystick API has
> been gathered from reading this driver, and it seems like there's a
> fundamental difference between DirectInput devices and "regular" ones.
>  Correct me if I'm wrong, but it looks to me like multiple POVs (or
> more than 6 axes) aren't supported here.

Yeah it seems the old interface does not support more then one POV. Sorry 
missed that.
> > Now that you can remap axis it's not guaranteed that 6th axes is the speed
> >  and so on.
>
> Yes, that's correct.  I previously tried a different approach,
> basing the selection of x, y, z, r, u, and v on what the axis is
> actually named (i.e., "Throttle" = z, "Rudder" = r), but that actually
> moots the whole point.  It turns out that it doesn't matter what the
> axis' nominal function is; what matters is the order in which they
> appear.  For instance, my joystick's rudder is actually named "Rz",
> which would wrongly map to "v" (as opposed to "r").  What's important
> is not for wine to know the difference between "Throttle" and
> "Rudder", but for the user to know what each axis is named and in what
> order they are presented to the application, so that he can then
> reoder them as he pleases.

I see, then you should also check for any gaps in mapping. If user skipped 
one axes it won't work.

Vitaliy.