microsoft / MixedRealityToolkit-Unity

This repository is for the legacy Mixed Reality Toolkit (MRTK) v2. For the latest version of the MRTK please visit https://github.com/MixedRealityToolkit/MixedRealityToolkit-Unity
https://aka.ms/mrtkdocs
MIT License
6.01k stars 2.12k forks source link

SpatialUnderstandingExample needs to have UI fixed #620

Closed Zod- closed 7 years ago

Zod- commented 7 years ago

Trying out the SpatialUnderstandingExample I noticed that the buttons kept flickering when you keep your cursor on them. Turns out the GazeManager keeps cycling through the different layers of the UI elements and fires GazeManager_FocusedChanged of the InputManager constantly.

How to reproduce:

  1. Open SpatialUnderstandingExample scene in the Unity-Editor.
  2. Scan the room till you can complete the scan process to get the UI to pop up.
  3. Place your cursor on any layered UI elements (tab buttons, regular buttons).
  4. GazeManager will now keep firing focus changed event with the different elements in layers under the cursor.

For exampling just printing the name of the HitObject element when placing the cursor on one of the buttons and leaving it there for a few seconds: hitobject

StephenHodgson commented 7 years ago

Which line did you place the debug for the HitObject?

Zod- commented 7 years ago

https://github.com/Microsoft/HoloToolkit-Unity/blob/master/Assets/HoloToolkit/Input/Scripts/Gaze/GazeManager.cs#L146-L149

            if (previousFocusObject != HitObject && FocusedObjectChanged != null)
            {
                var msg = HitObject != null ? HitObject.name : "";
                Debug.Log("HitObject: "+ msg);
                FocusedObjectChanged(previousFocusObject, HitObject);
            }
StephenHodgson commented 7 years ago

@kmikkel Regression from https://github.com/Microsoft/HoloToolkit-Unity/pull/606

kmikkel commented 7 years ago

Hi, yes I have managed to reproduce this issue. There are a few potential problems:

  1. It seems like the distance property of the UI elements after the RaycastAll() call is equal for several UI elements. Ie the canvas, the button, the text etc.
  2. The distance property is a float wich have its problems with accuracy could be a problem. Values for items having the same distance being rounded so they seem to switch place.
  3. This may be a bug in Unity so that the RaycastAll call is not reporting the correct distance and depth.

I have tried to order on both distance and depth. But it seems like the RaycastAll is inconsequent in what it returns. So the result is that it randomly after ordering hits by distance and depth are that the UI button seems to flicker. I suspect this may be a issue/bug in Unity.

Zod- commented 7 years ago

@kmikkel Are you comparing the depth of elements from different canvases? Comparing the distance between 2 canvases and the depth for elements within a single canvas is fine. However the depth between 2 canvases shouldn't make sense. So you would have to determine first which canvas the elements belong to before you can compare distance and/or depth.

kmikkel commented 7 years ago

Thanks for the feedback. Good point. I only have one canvas in my test scene. But it still behaves strange. I am using Unity 5.5.0p1 in my test. I am sorry, but I don't have time to look more into this anymore at the moment.

Zod- commented 7 years ago

I did some more testing and found more issues. It's not just the regression that causes the focus change firing, it only makes it slightly worse. Here doing the same reproduction steps with a commit just before the merge: capture

The issue is that with this UI setup, raycasts simply don't work. I think the general idea behind the current design is wrong or has to be used differently. The main issue is that the UI all exists in the same plane. The canvas and all it's child ui elements have the same z-coordinate so they all have the same distance to the camera. However because of floating point variations in the raycast calculations it will alwas choose a random one that is the "clostest".

The 3D raycast will choose a random ui-element of the canvas layer which will then be compared to a different random ui-element from the ui raycast. The regression only adds a few more randomly selected ui-elements for the second one which makes it worse.

A solution would be to add a small z-position to every single ui element so that the raycasts can actually detect the depth that they have. Like this the current system could be kept but every ui would have to be set up like this.

My second idea would be to have different layers for the canvas and its ui-elements. Only the canvas itself would be hit by the 3D raycast but not any of the ui-elements. After detecting a canvas got hit we then can do a ui raycast for the other layer where we determine which is the top element by their depth in the canvas.

Ben-Howard-bencinStudios commented 7 years ago

Has this been addressed?

Zod- commented 7 years ago

@bencinStudios I have a working version over at https://github.com/Zod-/HoloToolkit-Unity/tree/feature/ui_raycasts, but it requires a very specific setup.

  1. I introduced a new public variable in the Gaze Manager called RaycastUILayerMask.
  2. I removed that layer from the default RaycastLayerMask. So the default layermask would be for example Default, TransparentFX and Water. The other layermask would be just UI.
  3. Canvas needs a layer that is in RaycastLayerMask but not in RaycastUILayerMask. e.g. Default.
  4. All children of the Canvas that you want to hit need to be in the RaycastUILayerMask.
  5. Canvas needs a box collider covering the canvas and no Z-size is fine.

You can look at the SpatialUnderstandingExample or InputManagerTest to see an example

kmikkel commented 7 years ago

@bencinStudios I have solved the problem with changing focus on UI elements by adding a small z-value offset for the elements. So that the no element have the same z value as its parent. Doing this will stop the focus to switch.

Ben-Howard-bencinStudios commented 7 years ago

Thanks @Zod- , and thanks @kmikkel ... going with ease on this one, the z-value offset seems to have worked for the most part, still very subtle flicker when button or input field is gazed at, but notably better both visually and functionally. Was the quick hack I needed to meet the deadline, thanks all!

StephenHodgson commented 7 years ago

The issue is two fold:

  1. The UI elements are all at the same z depth, and should be brought out a bit.
  2. There are items that should have the raycast target flag set false, (Like all the text elements on the buttons).

Once these two issues are addressed in the scene, it should work fine. There's no bug in the GazeManager.