Gamepad: Improve unknown gamepad heuristics and mappings

Gamepad: Improve unknown gamepad heuristics and mappings

This CL improves the logic for mapping unknown gamepads by trying
several different axes for triggers, right stick, and d-pad.

With this change, Moga Pro, Nexus Player (ASUS) Gamepad, and Razer
Serval controllers are working correctly without an explicit mapping.

Before the change, the d-pad on Moga, Serval, and ASUS Gamepad were
non-functional. The triggers on the Moga didn't work at all, and the
Serval and ASUS Gamepad's triggers were swapped with the shoulder
buttons.

TEST=http://html5gamepad.com/
BUG=615656

[cgutman, 2016-06-16 04:07:05]

Description was changed from

==========
Gamepad: Improve unknown gamepad heuristics and mappings

This CL improves the logic for mapping unknown gamepads by trying
several different axes for triggers, right stick, and d-pad.

With this change, Moga Pro, Nexus Player (ASUS) Gamepad, and Razer
Serval controllers are working correctly without an explicit mapping.

Before the change, the d-pad on Moga, Serval, and ASUS Gamepad were
non-functional. The triggers on the Moga didn't work at all, and the
Serval and ASUS Gamepad's triggers were swapped with the shoulder
buttons.

TEST=http://html5gamepad.com/
BUG=615656
==========

to

==========
Gamepad: Improve unknown gamepad heuristics and mappings

This CL improves the logic for mapping unknown gamepads by trying
several different axes for triggers, right stick, and d-pad.

With this change, Moga Pro, Nexus Player (ASUS) Gamepad, and Razer
Serval controllers are working correctly without an explicit mapping.

Before the change, the d-pad on Moga, Serval, and ASUS Gamepad were
non-functional. The triggers on the Moga didn't work at all, and the
Serval and ASUS Gamepad's triggers were swapped with the shoulder
buttons.

TEST=http://html5gamepad.com/
BUG=615656
==========

[cgutman, 2016-06-16 04:07:05]

aicommander@gmail.com changed reviewers:
+ bajones@chromium.org, jdduke@chromium.org, scottmg@chromium.org

[cgutman, 2016-06-16 04:11:30]

Hopefully this will address the problems that people are having with the current
unknown gamepad mapping on Android. I tested with Moga Pro, Nexus Player
Gamepad, and Razer Serval controllers. Via the JUnit tests, I also verified that
the SHIELD controller would be properly mapped in the unknown mapping too
(though it has an explicit mapping).

[Changwan Ryu, 2016-06-16 06:48:43]

changwan@chromium.org changed reviewers:
+ aelias@chromium.org, changwan@chromium.org
- jdduke@chromium.org

[bajones, 2016-06-16 17:19:35]

I like the intent behind this, but I'm a little confused by the implementation.
It seems like the ___Populated() functions only work if the inputs have a
current non-zero value, which means that the mapping that's done would actually
shift when the buttons/triggers/axes are in their neutral positions. That will
probably work fine, but if there exists a device that has both AXIS_GAS and
AXIS_THROTTLE or AXIS_RTRIGGER and either of those the mappings would seem to
actually jump around at runtime depending on what's being pushed. (Unless I'm
reading this wrong.)

Oh, and it appears that you'd never get input from AXIS_THROTTLE if the
AXIS_BRAKE input has any value, since that would cause triggersPopulated to
evaluate true after mapPedalAxesToBottomShoulder.

I'd love to be able to handle mapping more automatically, but we need to be
careful with it. In my opinion accidentally missing an input is a lesser problem
than the inputs shifting around at runtime.

[cgutman, 2016-06-16 18:47:28]

On 2016/06/16 17:19:35, bajones wrote:
> I like the intent behind this, but I'm a little confused by the
implementation.
> It seems like the ___Populated() functions only work if the inputs have a
> current non-zero value, which means that the mapping that's done would
actually
> shift when the buttons/triggers/axes are in their neutral positions. That will
> probably work fine, but if there exists a device that has both AXIS_GAS and
> AXIS_THROTTLE or AXIS_RTRIGGER and either of those the mappings would seem to
> actually jump around at runtime depending on what's being pushed. (Unless I'm
> reading this wrong.)
That is correct. I have seen some devices which have multiple sets of axes, like
both AXIS_BRAKE/AXIS_GAS and AXIS_RTRIGGER/AXIS_LTRIGGER, but in these cases,
the
device exposes identical trigger information through both axes (as a
compatibility
"enhancement" for games that only look at one of them). This code would work
correctly
in these cases.

Very weird controllers that have axes mapped all wrong (say AXIS_RTRIGGER is
really the
right stick X or something), will definitely have some strange behavior, but the
gamepad
still wouldn't behave correctly with the existing code either. On Chrome
desktop, I'd be
more worried about this, but on mobile Chrome has never exposed unmapped
gamepads "raw"
like on desktop, so it's not possible for web apps to have custom mappings to
workaround
Chrome's default. Buttons and axes that weren't used in the unknown mapping
would
simply not be exposed at all.

> 
> Oh, and it appears that you'd never get input from AXIS_THROTTLE if the
> AXIS_BRAKE input has any value, since that would cause triggersPopulated to
> evaluate true after mapPedalAxesToBottomShoulder.
Good catch, these heuristics depend on the fact that none of the potential
mappings
share axes. This is a case where the behavior could flip between mappings. As
the CL is
now, it would not be possible to press both triggers at the same time on a
Xiaomi controller.

Granted, it's better than what the existing code does where neither trigger
would ever work,
but it's easily fixable. I'll fix the mapPedalAxesToBottomShoulder case to look
at both
AXIS_GAS and AXIS_THROTTLE and remove the altPedalAxes code.

> 
> I'd love to be able to handle mapping more automatically, but we need to be
> careful with it. In my opinion accidentally missing an input is a lesser
problem
> than the inputs shifting around at runtime.
There is another possible way we can do this. GamepadDevice has mAxes which
contains
all axes present on the device. We could perform the mapping based on the axes
present in
mAxes rather than the axis values if you're really worried about mappings
changing at
runtime. This would require some refactoring of the interface between
GamepadMappings
and GamepadDevice but it doesn't look too bad.

[bajones, 2016-06-16 21:24:30]

On 2016/06/16 18:47:28, cgutman wrote: 
> Granted, it's better than what the existing code does where neither trigger
> would ever work,
> but it's easily fixable. I'll fix the mapPedalAxesToBottomShoulder case to
look
> at both
> AXIS_GAS and AXIS_THROTTLE and remove the altPedalAxes code.

Thanks. Agreed that it would be good to get inputs that aren't available now
exposed, but let's make sure that inputs don't disappear at random.

> There is another possible way we can do this. GamepadDevice has mAxes which
> contains
> all axes present on the device. We could perform the mapping based on the axes
> present in
> mAxes rather than the axis values if you're really worried about mappings
> changing at
> runtime. This would require some refactoring of the interface between
> GamepadMappings
> and GamepadDevice but it doesn't look too bad.

I'd be curious to know what this looks like. Would it be difficult to do a
speculative patch for?

[cgutman, 2016-06-17 04:16:32]

On 2016/06/16 21:24:30, bajones wrote:
> I'd be curious to know what this looks like. Would it be difficult to do a
> speculative patch for?

I put something together here: https://codereview.chromium.org/2071223002/

Let me know if you like that approach better.

[cgutman, 2016-06-18 17:40:32]

On 2016/06/17 04:16:32, cgutman wrote:
> On 2016/06/16 21:24:30, bajones wrote:
> > I'd be curious to know what this looks like. Would it be difficult to do a
> > speculative patch for?
> 
> I put something together here: https://codereview.chromium.org/2071223002/
> 
> Let me know if you like that approach better.

Closed in favor of the better approach in
https://codereview.chromium.org/2071223002/