Mixedrealitytoolkit-unity: Task vNext - Remove PositionOnly and RotationOnly states. Add position accuracy check

[davidkline-ms]

Overview

From past (DirectInput) experience and a lengthy discussion with the platform PM team, we believe that most applications are going to run code that looks like:

if (controllerState == Tracked) 
{ DoSomething(); }
else
{ ControllerLostHandling(); }

This would cause the PositionOnly and RotationOnly states to be largely ignored.

WMR (and others?) provides approximate positioning when the controller leaves the tracking sensor's view, so the controller is always Tracked (once initial tracking occurs). To provide applications that wish to take advantage of the knowledge that the controller is "behind the head", we propose adding IsPositionApproximate() to the controller interface and allow specific controller classes to implement as appropriate.

Requires

• Removal of ControllerState.PositionOnly
• Removal of ControllerState.OrientationOnly
• Update of any code that refences these states to instead use ControllerState.Tracked
• Addition of "bool IsPositionApproximate()" method to IMixedRealityController interface
• Default implementation of IsPositionApproximate in BaseController class
• Full implementation of IsPositionApproximate in WindowsMixedRealityController class
• Documentation for IsPositionApproximate

Acceptance Criteria

Verify ControllerState is:

• [x] NotTracked until the first time the controller is seen by the tracking sensors
• [x] Tracked from that point forward (even when not visible to the sensors)

Confirm that new IsPositionApproximate() method returns:

• [ ] True if the controller is "behind the head"
• [ ] False when the controller is visible to the HMD controller tracking sensor

[StephenHodgson]

I'm not sure we will need this change. I added approximate tracking to the controller state Enum.

[davidkline-ms]

This is direct feedback from one of our systems PMs as well as from my own experience in how developers write their code (ex: using hr == S_OK as opposed to SUCCEEDED(hr)). We believe that many apps will key off of ControllerState.Tracked and completely miss ControllerState.ApproximateTracking.

I believe approximate tracking is a more advanced use case that we should handle separately from ControllerState enum values.

[StephenHodgson]

I'm not sure I understand exactly what you're trying to propose

[davidkline-ms]

I am proposing to simplify the ControllerStates to essentially NotTracked and Tracked (with None and Other) and to allow advanced scenarios to inquire as to if the position data returned is approximate (this matches the underlying WMR platform and possibly others).

Apps that use this would look like:

if (controller.State == ControllerState.Tracked)
{
    if (controller.IsPositionApproximate())
    { HandleApproximatePositioning(); }
    else
    { HandlePrecicePositioning(); }
}

[davidkline-ms]

The new behavior is based on WMR's PositionAccuracy https://docs.microsoft.com/en-us/uwp/api/windows.ui.input.spatial.spatialinteractionsourcelocation.positionaccuracy

[StephenHodgson]

In either case, we def shouldn't be removing position only and rotation only states.

Remember this enum is for more than just controller tacking and is supposed to be platform independent.

[davidkline-ms]

let's not add an approximate tracking value though

[StephenHodgson]

I disagree

[davidkline-ms]

Can you describe usages for ControllerState other than tracking? I worry that we have too many states and that would make the core behavior more complex and confuse developers. We should be tracked or not.

[davidkline-ms]

I'd honestly rather have methods to drill into things like IsPositionReported, IsPositionApproximate and IsOrientationReported

[davidkline-ms]

Simplifies the loop for developers to "is my device tracked" and then making the other data available

[StephenHodgson]

I have a PR that renamed ControllerState to TrackingState to clarify its usage a bit more.

Essentually it's just a way to know at a glance what the tracking state of a tracked pose is, not matter if it is a headset, phone, controller, etc. Any device really.

[StephenHodgson]

I don't mind having a bool for that, but I'm more concerned with is removing things we'll need for other types of tracked items.

[davidkline-ms]

So, for apps to handle all possible returns, they would need to:
if (state == Tracked || state == PositionOnly || state == OrientationOnly)

This is what I am trying to avoid by moving these to bools in the object (headset or controller). From experience where multiple "success" values are returned from an API, even devs who know to check for multiple tend to only check for the most common case (ex: hr == S_OK vs SUCCEEDED(hr)).

[StephenHodgson]

I'm fine with exposing some bools that make this "prettier", but I don't wanna remove states from the enum.

[keveleigh]

When is the TrackingState enum useful? It doesn't seem more useful than asking if each individual feature (position, orientation, etc) is available (like Unity and their TryGetPosition/Orientation/etc methods) and acting based on availability. What is an example where I have to use this TrackingState because the availability of data doesn't tell me what I need?

Maybe something like ControllerCapabilities which would stay the same throughout the lifetime of the controller would be useful for setting up what I expect to ever possibly get from a specific controller.

[SimonDarksideJ]

Ok, winding back this conversation a little, just to point out one small observation.

ON WINDOWS MIXED REALITY, we may do:
if (controller.State == ControllerState.Tracked)

But for other platforms, this may not be the same (sorry to muddy the waters), so let's be specific.

I have to agree in my own testing, WMR is a true boolean between tracked and not tracked. The SourceValue is nonsensical between values of 1 & 0, as there really isn't an in-between atm.

SO remembering that we are targetting MULTIPLE VR platforms, we cannot change the ControllerStates enum to just tracked and not tracked.

However, I agree that for WMR only, we may only implement Tracked and NotTracked.

How we handle "IsPositionApproximate" is a trickier proposition.

So I propose.

• We update the WMR tracking options to only use Tracked / Not Tracked (It was originally)
• Add the IsPositionApproximate function for WMR feeding from the source value
• We will evaluate the other ControllerStates / Tracking states when we add other MR platforms.

[SimonDarksideJ]

(Although I like how this discussion is a good point about why we discuss change before coding :D)

[keveleigh]

remembering that we are targetting MULTIPLE VR platforms

Are you referring to platforms that have orientation-only controllers and something like HoloLens with position-only hands, or something else?

Do you have an example where this enum provides more information than checking for the existence of position data or orientation data and acting based on their existence? It feels like this is trying to conflate several pieces of data into one, and I don't yet see the benefit, since I'll have to go talk to/check for the other values anyway.

[davidkline-ms]

remembering that we are targetting MULTIPLE VR platforms

We are definitely keeping other platforms in mind. From experience with different engines, the ones I have encountered are generally Tracked or Not Tracked. There are occasionally some other forms (ex: Orientation only) though quite often apps were keying off of Tracked exclusively, then updating position and/or orientation as data was provided them.

Following the principle of "it is easier to give than to take away", I would like to start the Alpha with the core tracking states of None, Tracked, NotTracked and Other (if we see Other, we know we need to add something) as well as the three bools that this discussion has covered: IsPositionReported, IsOrientationReported and IsPositionApproximate (covering systems, like WMR that are capable of returning inferred position data).

I have been experimenting with a POC and, when combined with #2351, I believe we will achieve both simplicity and flexibility for controllers and HMDs alike.

[davidkline-ms]

@StephenHodgson

I'm fine with exposing some bools that make this "prettier", but I don't wanna remove states from the enum.

My concern here is that we can only return one TrackingState, as implemented currently, which one should devices provide? which should apps expect to see? This is one of the primary drivers for wanting to simplify the TrackingState enum and add the properties to controllers, hmds, any other trackable objects.

[davidkline-ms]

This was agreed upon in shiproom 6/27/2018. I will coordinate this change with #2351.

[davidkline-ms]

For completeness, support for a concept of rotation accuracy will be part of this change.

[davidkline-ms]

Fixed with #2369