[v4 2/4] winejoystick.drv/joystick_osx.c: fixes Slider mapping

[Ken Thomases]

On Jun 13, 2016, at 12:11 AM, David Lawrie <david.dljunk at gmail.com> wrote:
> 
> Sliders, Dials, and Wheels now map to Z, U (Ry), or V (Rx) depending on
> availability. Thus, also adds support for up to 3 such HID elements.

> dlls/winejoystick.drv/joystick_osx.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/dlls/winejoystick.drv/joystick_osx.c b/dlls/winejoystick.drv/joystick_osx.c
> index 77dbef8..4e59ca3 100644
> --- a/dlls/winejoystick.drv/joystick_osx.c
> +++ b/dlls/winejoystick.drv/joystick_osx.c
> @@ -442,8 +442,26 @@ static void collect_joystick_elements(joystick_t* joystick, IOHIDElementRef coll
>                         break;
>                     }
>                     case kHIDUsage_GD_Slider:
> -                        TRACE("kIOHIDElementTypeInput_Misc / kHIDUsage_GD_Slider; ignoring\n");
> +                    case kHIDUsage_GD_Dial:
> +                    case kHIDUsage_GD_Wheel:
> +                    {
> +                        //if one axis is taken, fall to the next until axes are filled

Line comments (//) are not allowed in most of Wine.

> +                        int possible_axes[3] = {AXIS_Z,AXIS_RY,AXIS_RX};
> +                        int axis = 0;
> +                        TRACE("kIOHIDElementTypeInput_Misc / kHIDUsage_GD_<axis> (%d)", usage);
> +                        while(axis < 3 && joystick->axes[possible_axes[axis]].element)
> +                            axis++;
> +                        if (axis == 3)
> +                            TRACE("    ignoring\n");
> +                        else
> +                        {
> +                            TRACE(" axis %d\n", possible_axes[axis]);

Rather than breaking the trace line across multiple TRACEs, you should just move the TRACE to after the selection of an axis.  Note that in the existing code, the "ignoring" TRACEs are on a separate line, which is why they are indented.  It doesn't make sense to have that extra space if that isn't on a separate line.

Alternatively, you could put a newline on the first TRACE and indent the axis trace as a subordinate line.


> +                            joystick->axes[possible_axes[axis]].element = (IOHIDElementRef)CFRetain(child);
> +                            joystick->axes[possible_axes[axis]].min_value = IOHIDElementGetLogicalMin(child);
> +                            joystick->axes[possible_axes[axis]].max_value = IOHIDElementGetLogicalMax(child);
> +                        }
>                         break;
> +                    }

Is there reason to expect that these elements (sliders, dials, wheels) will be listed after the "proper" axes (x, y, z, etc.) in the children array?  Since the proper axes are specific and these elements should only be mapped to unused axes, I think they should be enumerated in a separate pass after everything else has had a shot.  Note that collect_joystick_elements() is recursive, so it's not right to do it in a separate loop within collect_joystick_elements().  Probably, you'll want to call it twice from open_joystick(), with a flag to indicate which mode it should operate in.

-Ken