Implementation of Controller Model Visualization Task

[StephenHodgson]

Overview

I've broken out the visualization of the controllers into their own profile, to give more fine grained control over the controller models. Part of the reason for this change was the fact that the input mapping profile didn't allow us to expose override models and other settings because of the pop out windows.

I've added a IMixedRealityControllerVisualizer that supports getting the controller's GameObejct reference without breaking the architecture design patterns we've established. (You'll notice that I updated the BaseController class to properly reference this interface, and not the transform or game object reference).

I've enhanced the controller visualization profile's model object fields to ensure we're assigning valid prefabs only (and not just mesh assets) as well as adding the user defined concrete type that implements IMixedRealityControllerVisualizer.

Here's a little snippet example of how to get the controller visuals from the input system:
(You'll notice that we can get any number of detected controllers)

public class MyClass : MonoBehaviour, IMixedRealitySourceStateHandler
{
    private List<Transform> leftHandControllerTransforms = new List<Transform>();
    private List<Transform> rightHandControllerTransforms = new List<Transform>();

    private void Start()
    {
        foreach (var visualizer in MixedRealityManager.VisualizationSystem.DetectedVisualizers)
        {
            // Skip the controllers that don't have visuals
            if (visualizer == null) { continue; }

            if (visualizer.Controller is WindowsMixedRealityController)
            {
                // Do something for a specific controller type
            }

            switch (visualizer.Handedness)
            {
                case Handedness.Left:
                    leftHandControllerTransforms.Add(visualizer.VisualizerGameObjectReference.transform);
                    break;
                case Handedness.Right:
                    rightHandControllerTransforms.Add(visualizer.VisualizerGameObjectReference.transform);
                    break;
            }
        }
    }

    public void OnSourceDetected(SourceStateEventData eventData)
    {
        if (eventData.Controller?.Visualization != null)
        {
            switch (eventData.Controller.ControllerHandedness)
            {
                case Handedness.Left:
                    leftHandControllerTransforms.Add(eventData.Controller.Visualization.VisualizerGameObjectReference.transform);
                    break;
                case Handedness.Right:
                    rightHandControllerTransforms.Add(eventData.Controller.Visualization.VisualizerGameObjectReference.transform);
                    break;
            }
        }
    }

    public void OnSourceLost(SourceStateEventData eventData)
    {
        if (eventData.Controller?.Visualization != null)
        {
            switch (eventData.Controller.ControllerHandedness)
            {
                case Handedness.Left:
                    leftHandControllerTransforms.Remove(eventData.Controller.Visualization.VisualizerGameObjectReference.transform);
                    break;
                case Handedness.Right:
                    rightHandControllerTransforms.Remove(eventData.Controller.Visualization.VisualizerGameObjectReference.transform);
                    break;
            }
        }
    }
}

Changes

• Fixes: vNext Task: Improve and better implement Controller Model Visualization #2799
• Fixes: Solvers need to track controller transforms #2785 By giving the ability to get a controller's visualization and in extension it's transform.

TODO

• Test type passed correctly

Breaking Changes

• Removed unused id's from interaction mapping (we usually use the array item's position in the array as the id)
• Broke out controller visualization options in Controller Mapping profile into its own profile

[SimonDarksideJ]

Added an update to this PR for reivew
StephenHodgson#32

Notably, this makes the Visualization system its very own Manager and decentralises the scene management and abstracts the visualization layer

[StephenHodgson]

@SimonDarksideJ Can you provide an example of how we would get the controllers with your CR on my PR?

[SimonDarksideJ]

As it's now a manager, the call would look like any other manager call, e.g.

IMixedRealityVisualizationSystem visualizer = MixedRealityManager.Instance.GetManager<IMixedRealityVisualizationSystem>();

foreach (MixedRealityVisualizer controller in Visualizer.DetectedVisualizers)
{
    if (controller.Handedness == hand)
    {
        return controller;
    }
}

Obviously like any other system provided by a Manager (including the Input System) you'd want to cache the reference and in the case of the Solver handler, make it a Static reference so it's the same across all handlers.

Although, I did note we now have two Handedness references in the scope, one for the Visualiser and one for the referenced controller. So maybe we could even from the one for the visualizer, unless there is a case where a visualizer could be in a different hand? or None)

[StephenHodgson]

Still not sure about how I feel about a viz manager, but I'd like to hear @keveleigh and @Ecnassianer's input as well. What do you guys think?

[SimonDarksideJ]

Sorry to still be a stick in the mud, but we simply add the visualizer to the controller, still for the same reasons:

• You are creating a circular reverence from Controller to Visualizer to Controller, etc.
• Can't remove the controller reference from the visualizer as you need to know which controller it came from. So you can't remove that to break the circular reference.

If the Visualizer is a thing that is needed in the SourceDetected event, then simply add the visualizer to the EVENT not the controller (using the interface).

    public void OnSourceDetected(SourceStateEventData eventData)
    {
        if (eventData.Visualizer != null)
        {
            switch (eventData.Visualizer.Handedness)
            {
                case Handedness.Left:
                    leftHandControllerTransforms.Add(eventData.Visualizer.VisualizerGameObjectReference.transform);
                    break;
                case Handedness.Right:
                    rightHandControllerTransforms.Add(eventData.Visualizer.VisualizerGameObjectReference.transform);
                    break;
            }
        }
    }

Question

If we do move the reference to the visualizer, do we even need the "DetectedVisualizers" hashset any more? Would GameObjects being able to discover the visualizer from the Event be enough to meet @davidkline-ms requirement of discovering visualizers?

[SimonDarksideJ]

on another point, we should probably close all of the final SolverHanlder PR's until this PR is solved (no pun intended) as the implementation will have to change to meet the output of this PR anyway

[StephenHodgson]

We pretty much do the same thing with pointers (granted it's an array of them). I know the circular reference isn't ideal, but I think it solves the problem without a complex solution.

My favorite circular reference in unity is:
gameObject.transform.gameObect.transform...
🤣

[StephenHodgson]

And yes, we'd still need the hashset to query against in the event a source is detected before our listener was subscribed.

[StephenHodgson]

If we were to add the visualizer to the source detected event, the list of visualizers would need to move into the input system. (Which I think is okay, seeing as the profile and viz manager both depend on the input system)

[SimonDarksideJ]

No need to move the list of detected Visualisers to the Input System, simply passing the reference down in the event is enough.
Still don't want a hard dependency with other components and the input system. Else those who do want to (for what ever reason) will get a lot more than they bargained for and then not bother.

[StephenHodgson]

Still don't want a hard dependency with other components and the input system.

You can't know the position of a controller without the input system, and without the input system the Controller's rendered model isn't going to be where it needs to be. Unless you made the visualization part of the input system or the controller itself (but we already agreed the transform position shouldn't be part of the device layer so we can port the system to other engines).

We need to know the position of the rendered controller. We can't do that without the input system sending the positional data to the controller, or the controller keeping track of its own position.

[StephenHodgson]

In either case, my main goal is to have this function pretty much exactly like pointers, where we set a prefab and have it get created as needed.

[Ecnassianer]

We spoke about this in another thread which I can't find at the moment, but can we change the name from "Visualizer" to something else. This PR seems like the place to make that change. Here are some things that I think would more clearly reveal the purpose of these classes:

SceneRepresentation (i.e, ControllerSceneRepresentation or ControllerRepresentation)
DeviceAvatar (i.e, ControllerDeviceAvatar)
SceneObject
TrackingProxy
TrackedProxy
MixedRealityProxy
RealityProxy

Let me know if you want a recap of why I think Visualizer is a misleading name.

[StephenHodgson]

How is it misleading?

[Ecnassianer]

"Visualizer" seems to imply that the focus of the class is in rendering a visual representation of the controller. It's about pushing pixels to the screen. But what I understand now is the "visualizer" class is all the data that the device puts into the scene, not just the visuals.

Consider approaching this problem from a naive perspective: I don't want to show mechanical controllers in my fantasy game. Instead I want to use information about the controller state (button states, location, etc) to draw some kind of particle fairies around where the controllers would be that always look at your hands, but aren't necessarily in the same place as your hands. Given that prompt, a "Controller Visualizer" isn't nearly the first thing I would look for, since I explicitly don't want to visualize controllers, I just want to get scene data about the controller.

The more we build out the "Visualization System", the lower percentage of it actually has to do with visualizing controllers. We're already seeing this class grow in a non-visualization direction, we should anticipate further growth in other directions with a more generalized name.

[Ecnassianer]

I think the Loop in Start(), Then Listen for OnSourceDetected and OnSourceLost pattern is necessary to support (so we can support things like having the world behave differently when a controller leaves), but I also think we should offer some kind of shorthand that manages all that boilerplate for you. Something like this:
MixedRealityManager.VisualizationSystem.GetMotionController( Handedness handedness )

There are a lot of lighterweight situations that don't want to manager the controller themselves. They just want a transform or whatever.

[StephenHodgson]

I'm cool with "proxy".

[SimonDarksideJ]

To look at this from another pose (pun intended) @Ecnassianer
WHat you have described is the Controller pose Synchronizer. WHich consumes the Input Pose event data to feed information from the controller state.

Whereas, this "visualizer" provides a visual representation of A controller which receives information from the pose Synchronizer to display it on the screen.

So, you have a few options as a MRTK dev:

• Simply inherit from the Pose State / Input State handlers and manage the incoming controller data yourself
• Inherit from the Controller Pose Synchronizer which will automatically transform the object you attach it to, to data coming from a controller
• Use (or modify) the MRTK visualization functionality, which automatically projects a configured controller model (and eventually animations) in to the visible scene.

I'm not adverse to renaming it to the Controller Proxy / Controller Proxy Manager (as it's also the terminology used by VRTK) but want to be sure we are talking about the same thing.

[StephenHodgson]

@SimonDarksideJ I wasn't able to easily overload the on source detected like we wanted to do and pass the visualizer (proxy?) controller through with it.

Any other ideas?

At the moment it's pretty much identical to how pointers are setup and used (which was the goal, seeing as they serve virtually the same function, sans the controller proxy itself should be unique to the controller, but there could be multiple pointers).

I'm not so keen on having our own Visualization System as it's completely driven by the input system, and our goal was to have systems not be depend on each other. All that manager does is just keep a list of the device visualizers and I'm not sure why the Input System can't just facilitate that.

If we wanted to make it its own full blown system, we should also add event handlers when visualization is added/removed, etc

[david-c-kline]

I'm not so keen on having our own Visualization System as it's completely driven by the input system, and our goal was to have systems not be depend on each other. All that manager does is just keep a list of the device visualizers and I'm not sure why the Input System can't just facilitate that.
If we wanted to make it its own full blown system, we should also add event handlers when visualization is added/removed, etc

I would rather we enable the input system to provide the controller gameobject(s) and avoid making another system that would introduce cross component dependencies.

[david-c-kline]

(but we already agreed the transform position shouldn't be part of the device layer so we can port the system to other engines).

Except that other engines are going to have different requirements... I wouldn't worry about porting at this point.

[StephenHodgson]

I wouldn't worry about porting at this point.

haha isn't that the truth! 😆

[StephenHodgson]

Okay I can't do a hard reset on this, so I'll need to open a new PR. I'll be sure to link this one.

[SimonDarksideJ]

This statement is not really valid

I'm not so keen on having our own Visualization System as it's completely driven by the input system, and our goal was to have systems not be depend on each other. All that manager does is just keep a list of the device visualizers and I'm not sure why the Input System can't just facilitate that.

This was only the start of the (controller) visualization system, who responsibility was to maintain and control the visualized elements in the scene.

Moving yet another function in to the Input system is a mistake imho, as it breaks the rule of only having individual component systems responsible for a single area / function. Now in order to replace the input system (granted not something we advise people to do, they should contribute to this one), they are going to have to rewrite:

• Input routing
• Input source handling
• Focus
• Gaze
• Pointers
• Pointer visualization (as this is a scene task)
  and now Controller visualization

A single system should not be this big and have responsibility over so many features, which was one of the fundamental issues with the old HTK that we were trying to break free from.
This PR provided the right level of abstraction and only needed to address a final requirement of making the data/visualizer reference available in the source detected event.

Really not happy with having an input system also being responsible for scene management of features (pointer visualization was a very fine line as it already generated pointers) doing both the job of routing and now UX, but that seems to be the consensus for the approach.

[StephenHodgson]

😅🤔 I know you are correct.

But I'm not sure how to facilitate the visual parts with being driven by input itself. Seems like the visualizer system would be too dependant on the input system, which was another requirement, as there shouldn't be dependencies on other systems.

I'm trying to figure out a middle ground where we can all agree so we can move forward.