Mixedrealitytoolkit-unity: Cannot test whether grip position can be obtained from a WSA hand

[PJBowron]

Overview

Forgive me if this covered elsewhere: it no longer seems possible to 'properly' test if a position can be obtained from a WSA 'hand' input source (via InteractionInputSource). Browsing through the current code, it appears that though a hand supplies a position through its 'grip' ability, and this gets reflected up through the SourceCapabilities, asking for the capability fails because GetSupportedInputInfo doesn't consider grip info (position or rotation) at all.

Expected Behavior

Asking for 'Position' support will succeed for an InteractionInputSource with an underlying WSA hand source

Actual Behavior

The only supported info from a hand is 'Selected', IIRC.

Repro

Set up a default scene including the InputManager with an InteractionInputSource.
Add a script that implements a Handler interface (e.g. IInputClickHandler)
In the Interface delegate, ask the InputEventData if its InputSource supports the SupportedInputInfo.Position info

Mixed Reality Toolkit Release Version

Local version is some flavour of 2017.2, but current code as browsed on GitHub looks the same

[StephenHodgson]

I'm not sure the API even supports grip for hands, but I'll double check.

Do you have repro steps for me to test this?

[PJBowron]

@StephenHodgson I've updated the issue - marked some key changes in bold and added a repro.

I guess the issue revolves around whether GetSupportedInputInfo should consider 'Grip' data (I can't see that it shouldn't) and whether the SupportedInputInfo enum should distinguish between Pointer and Grip position and rotation support.

[PJBowron]

@StephenHodgson Regarding the question of API support for Grip: WSA.Input does, via InteractionSourceState.sourcePose, and although the toolkit exposes this via TryGetGripPosition, you just can't test for support with GetSupportedInputInfo.

I'm happy to update the code to address this.

[StephenHodgson]

I'm happy to update the code to address this.

Yes please :)

[keveleigh]

Ahh, good catch! Yep, the hand's positional data is routed through the "grip" position, I'd think because that also aligns with where the hand would be placed on a motion controller. GetSupportedInputInfo should definitely distinguish between Pointer and Grip position and rotation support, like you've already proposed. Thanks!

Your mention of Select being supported is interesting too. It looks like that data only comes through state.anyPressed, while currently the Toolkit's input system only cares about state.selectPressed, where I didn't see any hand data come through from my tests just now.
We could probably update GetSelect to take into account if the source is a hand, then to check state.anyPressed instead of state.selectPressed.

EDIT: That statement was only partially right. On RS1, that is the behavior. In the future™, it looks like that info is also routed through state.selectPressed and state.selectPressedAmount.

[PJBowron]

I've just put up an initial drop of my changes -
@keveleigh I didn't go near the select issue yet as I wasn't entirely clear on it :) But maybe it would warrant its own PR anyway.

[timGerken]

via #1870

[StephenHodgson]

Do we also need to fix this in the master branch?

[timGerken]

@StephenHodgson good question.
@PJBowron ?

[PJBowron]

Hi guys.
Apologies, I'm not sure of the scope of the question; if it's: does this situation exist in the master branch - then yes.
If it's more to do with the release schedule, then I'm not too familiar with how things work. If we're talking about merging to master so it turns up in a future release - then sure.
If you're considering a hotfix to the existing release? Probably not important enough for that. The facility to obtain both pointer and grip data already exists, this update just lets you properly test for the capabilities.

[timGerken]

@PJBowron I think, if the fix is righteous, the right thing to do is cherry-pick it from the Dev_Working_Branch into Patch4_Dev. That branch will merge into master "real soon now".

[PJBowron]

Apologies for letting this go stale; @davidkline-ms is it too late for me to cherry-pick this for Patch4_Dev? I'm assuming you're done with that, now that 2017.2.1.4-rc2 is out?

[davidkline-ms]

@PJBowron,

2017.2.1.4 is scheduled to be released in the next couple of days. Please feel free to target your fix to the may18_dev branch.

Thanks!
David

[PJBowron]

@davidkline-ms
Finally gotten around to resubmitting new PR: #2050

[davidkline-ms]

This was released in the 2017.4.0.0 release candidate