Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added check for valid uiRaycastResults and fixed pointer registration #1350

Merged
merged 11 commits into from
Nov 15, 2017
Merged

Added check for valid uiRaycastResults and fixed pointer registration #1350

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a1f24f6
Added check for valid uiRaycastResults
StephenHodgson Nov 12, 2017
3134b08
removed the OnValidate call in FocusManager to reduce the warning mes…
StephenHodgson Nov 12, 2017
ff3cd54
Fixed pointer registration
StephenHodgson Nov 13, 2017
2137df0
removed extra line space.
StephenHodgson Nov 13, 2017
3224a97
recombined if statements
StephenHodgson Nov 13, 2017
a49e812
Updated Simulated Hand show/hide logic.
StephenHodgson Nov 13, 2017
86919fa
Added Assets for rays
StephenHodgson Nov 13, 2017
1d1dc67
Updated the GetPointer method to return the first pointer if no valid…
StephenHodgson Nov 14, 2017
7f094ca
Merge branch 'MRTK-Master' into MRTK-uiRaycastHotfix
StephenHodgson Nov 14, 2017
92fa3bd
Reverted the unregistering of pointers on source lost.
StephenHodgson Nov 14, 2017
87ddac5
Added suggestions from Lars about directly getting the pointer extent…
StephenHodgson Nov 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 79 additions & 66 deletions Assets/HoloToolkit/Input/Scripts/Focus/FocusManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,7 @@ protected override void Awake()

private void Start()
{
if (gazeManagerPointingData == null)
{
if (GazeManager.IsInitialized)
{
gazeManagerPointingData = new PointerData(GazeManager.Instance);
}
}
else
{
Debug.Assert(ReferenceEquals(gazeManagerPointingData.PointingSource, GazeManager.Instance));
}

if ((pointers.Count == 0) && autoRegisterGazePointerIfNoPointersRegistered && GazeManager.IsInitialized)
if (pointers.Count == 0 && autoRegisterGazePointerIfNoPointersRegistered && GazeManager.IsInitialized)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autoRegisterGazePointerIfNoPointersRegistered
what about:
autoRegisterGazeIfPointersEmpty ?

(matter of taste, feel free to ignore)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that field name is obnoxiously long haha

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI in Unity 2017.0.2f3 I'm getting 3 assertion failures on startup in the InputManagerTest scene. But they're unrelated to the pointer issue:

Assertion failed: Main camera does not exist!
UnityEngine.XR.WSA.HolographicSettings:get_IsDisplayOpaque()

IsDisplayOpaque is being called by FadeScript and BoundaryManager. These failures don't happen in Unity MRTP 4 so I'm guessing it's an already-fixed XR bug on Unity's end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I'm trying to understand why you'd ever want to skip registering the gaze pointer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that was a hold over from only having active pointers registered. It looks like you've changed the way that works though, but I haven't had a chance to review all these recent changes.

{
RegisterPointer(GazeManager.Instance);
}
Expand All @@ -75,18 +63,6 @@ private void Update()
UpdateFocusedObjects();
}

/// <summary>
/// It's assumed that if you're using the MRTK's input system you'll
/// need to update your canvases to use the proper raycast camera for input.
/// </summary>
private void OnValidate()
{
if (UIRaycastCamera != null)
{
UpdateCanvasEventSystems();
}
}

#endregion

#region Settings
Expand Down Expand Up @@ -273,27 +249,33 @@ public Camera UIRaycastCamera

public void RegisterPointer(IPointingSource pointingSource)
{
Debug.Assert(pointingSource != null);
Debug.Assert(pointingSource != null, "Can't register a pointer if you give us one.");

if (TryGetPointerIndex(pointingSource) != null)
int pointerIndex;
PointerData pointer;

if (TryGetPointerIndex(pointingSource, out pointerIndex))
{
// This pointing source is already registered and active.
return;
}

PointerData pointer;

if (pointingSource is GazeManager)
{
if (gazeManagerPointingData == null)
{
gazeManagerPointingData = new PointerData(pointingSource);
if (GazeManager.IsInitialized)
{
gazeManagerPointingData = new PointerData(GazeManager.Instance);
}
}
else
{
Debug.Assert(ReferenceEquals(gazeManagerPointingData.PointingSource, GazeManager.Instance));
gazeManagerPointingData.ResetFocusedObjects();
}

Debug.Assert(gazeManagerPointingData != null);
pointer = gazeManagerPointingData;
}
else
Expand All @@ -306,14 +288,17 @@ public void RegisterPointer(IPointingSource pointingSource)

public void UnregisterPointer(IPointingSource pointingSource)
{
Debug.Assert(pointingSource != null);
Debug.Assert(pointingSource != null, "Can't unregister a pointer if you give us one.");

int? iPointer = TryGetPointerIndex(pointingSource);
Debug.Assert(iPointer != null);
int pointerIndex;
TryGetPointerIndex(pointingSource, out pointerIndex);

PointerData pointer;
GetPointer(pointingSource, out pointer);

PointerData pointer = pointers[iPointer.Value];
// Should we be protecting against unregistering the GazeManager?

pointers.RemoveAt(iPointer.Value);
pointers.RemoveAt(pointerIndex);

// Raise focus events if needed:

Expand Down Expand Up @@ -343,13 +328,11 @@ public void UnregisterPointer(IPointingSource pointingSource)

public FocusDetails? TryGetFocusDetails(BaseEventData eventData)
{
for (int iPointer = 0; iPointer < pointers.Count; iPointer++)
for (int i = 0; i < pointers.Count; i++)
{
PointerData pointer = pointers[iPointer];

if (pointer.PointingSource.OwnsInput(eventData))
if (pointers[i].PointingSource.OwnsInput(eventData))
{
return pointer.End;
return pointers[i].End;
}
}

Expand All @@ -368,20 +351,20 @@ public GameObject TryGetFocusedObject(BaseEventData eventData)
IPointingSource pointingSource;
TryGetPointingSource(eventData, out pointingSource);
PointerInputEventData pointerInputEventData = GetSpecificPointerEventData(pointingSource);

Debug.Assert(pointerInputEventData != null);
pointerInputEventData.selectedObject = details.Value.Object;

return details.Value.Object;
}

public bool TryGetPointingSource(BaseEventData eventData, out IPointingSource pointingSource)
{
for (int iPointer = 0; iPointer < pointers.Count; iPointer++)
for (int i = 0; i < pointers.Count; i++)
{
PointerData pointer = pointers[iPointer];

if (pointer.PointingSource.OwnsInput(eventData))
if (pointers[i].PointingSource.OwnsInput(eventData))
{
pointingSource = pointer.PointingSource;
pointingSource = pointers[i].PointingSource;
return true;
}
}
Expand All @@ -392,12 +375,28 @@ public bool TryGetPointingSource(BaseEventData eventData, out IPointingSource po

public FocusDetails GetFocusDetails(IPointingSource pointingSource)
{
return GetPointer(pointingSource).End;
PointerData pointerData;
FocusDetails details = default(FocusDetails);

if (GetPointer(pointingSource, out pointerData))
{
details = pointerData.End;
}

return details;
}

public GameObject GetFocusedObject(IPointingSource pointingSource)
{
return GetPointer(pointingSource).End.Object;
PointerData pointerData;
GameObject focusedObject = null;

if (GetPointer(pointingSource, out pointerData))
{
focusedObject = pointerData.End.Object;
}

return focusedObject;
}

/// <summary>
Expand Down Expand Up @@ -438,12 +437,19 @@ public PointerInputEventData GetGazePointerEventData()

public PointerInputEventData GetSpecificPointerEventData(IPointingSource pointer)
{
return GetPointer(pointer).UnityUIPointerData;
PointerData pointerEventData;
return GetPointer(pointer, out pointerEventData) ? pointerEventData.UnityUIPointerData : null;
}

public float GetPointingExtent(IPointingSource pointingSource)
{
return GetPointingExtent(GetPointer(pointingSource));
PointerData pointerData;
return GetPointer(pointingSource, out pointerData) ? GetPointingExtent(pointerData) : pointingExtent;
}

private float GetPointingExtent(PointerData pointer)
{
return pointer.PointingSource.ExtentOverride ?? pointingExtent;
}

#endregion
Expand Down Expand Up @@ -542,6 +548,9 @@ private void RaycastPhysics(PointerData pointer, LayerMask[] prioritizedLayerMas
bool isHit = false;
int rayStepIndex = 0;

Debug.Assert(pointer.PointingSource.Rays != null, "No valid rays for " + pointer.GetType());
Debug.Assert(pointer.PointingSource.Rays.Length > 0, "No valid rays for " + pointer.GetType());

// Check raycast for each step in the pointing source
for (int i = 0; i < pointer.PointingSource.Rays.Length; i++)
{
Expand Down Expand Up @@ -601,6 +610,9 @@ private void RaycastUnityUI(PointerData pointer, LayerMask[] prioritizedLayerMas
RayStep rayStep = default(RayStep);
int rayStepIndex = 0;

Debug.Assert(pointer.PointingSource.Rays != null, "No valid rays for " + pointer.GetType());
Debug.Assert(pointer.PointingSource.Rays.Length > 0, "No valid rays for " + pointer.GetType());

// Cast rays for every step until we score a hit
for (int i = 0; i < pointer.PointingSource.Rays.Length; i++)
{
Expand All @@ -613,7 +625,7 @@ private void RaycastUnityUI(PointerData pointer, LayerMask[] prioritizedLayerMas
}

// Check if we need to overwrite the physics raycast info
if ((pointer.End.Object == null || overridePhysicsRaycast) && uiRaycastResult.module.eventCamera == UIRaycastCamera)
if ((pointer.End.Object == null || overridePhysicsRaycast) && uiRaycastResult.isValid && uiRaycastResult.module.eventCamera == UIRaycastCamera)
{
newUiRaycastPosition.x = uiRaycastResult.screenPosition.x;
newUiRaycastPosition.y = uiRaycastResult.screenPosition.y;
Expand Down Expand Up @@ -783,32 +795,33 @@ private void RaisePointerSpecificFocusChangedEvents(IPointingSource pointer, Gam
}
}

private PointerData GetPointer(IPointingSource pointingSource)
private bool GetPointer(IPointingSource pointingSource, out PointerData pointerData)
{
int? iPointer = TryGetPointerIndex(pointingSource);
Debug.Assert(iPointer != null);
return pointers[iPointer.Value];
int pointerIndex;

if (TryGetPointerIndex(pointingSource, out pointerIndex))
{
pointerData = pointers[pointerIndex];
return true;
}

pointerData = null;
return false;
}

private int? TryGetPointerIndex(IPointingSource pointingSource)
private bool TryGetPointerIndex(IPointingSource pointingSource, out int pointerIndex)
{
int? found = null;

for (int i = 0; i < pointers.Count; i++)
{
if (pointers[i].PointingSource == pointingSource)
if (pointingSource == pointers[i].PointingSource)
{
found = i;
break;
pointerIndex = i;
return true;
}
}

return found;
}

private float GetPointingExtent(PointerData pointer)
{
return (pointer.PointingSource.ExtentOverride ?? pointingExtent);
pointerIndex = -1;
return false;
}

private RaycastHit? PrioritizeHits(RaycastHit[] hits, LayerMask[] layerMasks)
Expand Down
5 changes: 2 additions & 3 deletions Assets/HoloToolkit/Input/Scripts/Focus/InputSourcePointer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public bool InteractionEnabled
[Obsolete("Will be removed in a later version. Use OnPreRaycast / OnPostRaycast instead.")]
public void UpdatePointer()
{

}

public virtual void OnPreRaycast()
Expand All @@ -62,9 +61,9 @@ public virtual void OnPreRaycast()
}
else
{
Debug.Assert(InputSource.SupportsInputInfo(InputSourceId, SupportedInputInfo.Pointing));
Debug.Assert(InputSource.SupportsInputInfo(InputSourceId, SupportedInputInfo.Pointing), string.Format("{0} with id {1} does not support pointing!", InputSource, InputSourceId));

Ray pointingRay = default(Ray);
Ray pointingRay;
if (InputSource.TryGetPointingRay(InputSourceId, out pointingRay))
{
rays[0].CopyRay(pointingRay, FocusManager.Instance.GetPointingExtent(this));
Expand Down
7 changes: 3 additions & 4 deletions Assets/HoloToolkit/Input/Scripts/Focus/SimpleSinglePointerSelector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,12 @@ private void Start()
started = true;

InputManager.AssertIsInitialized();
GazeManager.AssertIsInitialized();
FocusManager.AssertIsInitialized();
GazeManager.AssertIsInitialized();

AddInputManagerListenerIfNeeded();
FindCursorIfNeeded();
ConnectBestAvailablePointer();

Debug.Assert(currentPointer != null, this);
}

private void OnEnable()
Expand Down Expand Up @@ -173,6 +171,8 @@ private void SetPointer(IPointingSource newPointer)
Cursor.Pointer = newPointer;
}
}

Debug.Assert(currentPointer != null, "No Pointer Set!");
}

private void ConnectBestAvailablePointer()
Expand Down Expand Up @@ -228,7 +228,6 @@ private void HandleInputAction(InputEventData eventData)
// TODO: robertes: see if we can treat voice separately from the other simple committers,
// so voice doesn't steal from a pointing controller. I think input Kind would need
// to come through with the event data.

SetPointer(GazeManager.Instance);
pointerWasChanged = true;
}
Expand Down
14 changes: 5 additions & 9 deletions Assets/HoloToolkit/Input/Scripts/Gaze/GazeManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,14 @@ public Vector3 GazeNormal
[Tooltip("True to draw a debug view of the ray.")]
public bool DebugDrawRay;
public PointerResult Result { get; set; }

[Obsolete("Will be removed in a later version. Use Rays instead.")]
public Ray Ray { get { return Rays[0]; } }

public RayStep[] Rays { get { return rays; } }

private RayStep[] rays = new RayStep[1] { new RayStep(Vector3.zero, Vector3.zero) };

public float? ExtentOverride
{
get { return MaxGazeCollisionDistance; }
Expand All @@ -133,10 +133,7 @@ public LayerMask[] PrioritizedLayerMasksOverride

public bool InteractionEnabled
{
get
{
return true;
}
get { return true; }
}

private float lastHitDistance = 2.0f;
Expand Down Expand Up @@ -203,7 +200,7 @@ private void UpdateGazeInfo()
newGazeNormal = Stabilizer.StableRay.direction;
}

Rays[0].UpdateRayStep(newGazeOrigin, newGazeOrigin + (newGazeNormal * FocusManager.Instance.GetPointingExtent (this)));
Rays[0].UpdateRayStep(newGazeOrigin, newGazeOrigin + (newGazeNormal * FocusManager.Instance.GetPointingExtent(this)));
}

UpdateHitPosition();
Expand All @@ -212,7 +209,6 @@ private void UpdateGazeInfo()
[Obsolete("Will be removed in a later version. Use OnPreRaycast / OnPostRaycast instead.")]
public void UpdatePointer()
{

}

public virtual void OnPreRaycast()
Expand Down
4 changes: 2 additions & 2 deletions Assets/HoloToolkit/Input/Scripts/Utilities/Interactions/CustomInputSelector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ private void Awake()
bool spawnControllers = false;

#if UNITY_2017_2_OR_NEWER
spawnControllers = (!XRDevice.isPresent && XRSettings.enabled || Application.isEditor) && simulateHandsInEditor;
spawnControllers = !XRDevice.isPresent && XRSettings.enabled && simulateHandsInEditor;
#else
spawnControllers = !VRDevice.isPresent;
spawnControllers = simulateHandsInEditor;
#endif
if (spawnControllers)
{
Expand Down