Merge pull request #7283 from julenka/fixfov

Fixes edge cases when checking if collider is in field of view

Author: Julia Schwarz

Assets/MixedRealityToolkit.SDK/Features/UX/Scripts/Pointers/PokePointer.cs

@@ -210,7 +210,7 @@ private bool FindClosestTouchableForLayerMask(LayerMask layerMask, out BaseNearI
                 var touchable = collider.GetComponent<BaseNearInteractionTouchable>();
                 if (touchable)
                 {
-                    if (IgnoreCollidersNotInFOV && !mainCam.IsInFOVConeCached(collider))
+                    if (IgnoreCollidersNotInFOV && !mainCam.IsInFOVCached(collider))
                     {
                         continue;
                     }

Assets/MixedRealityToolkit.SDK/Features/UX/Scripts/Pointers/SpherePointer.cs

@@ -290,7 +290,7 @@ public bool TryUpdateQueryBufferForLayerMask(LayerMask layerMask, Vector3 pointe
                     {
                         if (ignoreCollidersNotInFOV)
                         {
-                            if (!mainCam.IsInFOVConeCached(collider))
+                            if (!mainCam.IsInFOVCached(collider))
                             {
                                 // Additional check: is grabbable in the camera frustrum
                                 // We do this so that if grabbable is not visible it is not accidentally grabbed

Assets/MixedRealityToolkit.Tests/EditModeTests/Core/Extensions/CameraExtensionTests.cs

@@ -116,12 +116,12 @@ public void TearDown()
         /// Test that the Camera extension method IsInFOVConeCached returns valid results for colliders whose bounds are renderable to the camera
         /// </summary>
         [Test]
-        public void TestIsInFOVConeCached()
+        public void TestIsInFOVCached()
         {
             for (int i = 0; i < TestColliders.Count; i++)
             {
                 var test = TestColliders[i];
-                Assert.AreEqual(test.ShouldBeInFOVCamera1, testCamera.IsInFOVConeCached(test.Collider), $"TestCollider[{i}] did not match");
+                Assert.AreEqual(test.ShouldBeInFOVCamera1, testCamera.IsInFOVCached(test.Collider), $"TestCollider[{i}] did not match");
             }
         }
 
@@ -130,13 +130,13 @@ public void TestIsInFOVConeCached()
         /// facing different directions.
         /// </summary>
         [Test]
-        public void TestIsInFOVConeCachedSecondCamera()
+        public void TestIsInFOVCachedSecondCamera()
         {
             for (int i = 0; i < TestColliders.Count; i++)
             {
                 var test = TestColliders[i];
-                Assert.AreEqual(test.ShouldBeInFOVCamera1, testCamera.IsInFOVConeCached(test.Collider), $"TestCollider[{i}] did not match");
-                Assert.AreEqual(test.ShouldBeInFOVCamera2, testCamera2.IsInFOVConeCached(test.Collider), $"TestColliderSecondCamera[{i}] did not match");
+                Assert.AreEqual(test.ShouldBeInFOVCamera1, testCamera.IsInFOVCached(test.Collider), $"TestCollider[{i}] did not match");
+                Assert.AreEqual(test.ShouldBeInFOVCamera2, testCamera2.IsInFOVCached(test.Collider), $"TestColliderSecondCamera[{i}] did not match");
             }
         }
 

Assets/MixedRealityToolkit.Tests/PlayModeTests/PointerTests.cs

@@ -101,49 +101,38 @@ public IEnumerator TestLinePointers()
         }
 
 
-        private IEnumerator TestPointerFieldOfViewHelper(IMixedRealityPointer myPointer, GameObject cube, TestHand testHand)
+        /// <summary>
+        /// Test pointers are correctly enabled when interacting with colliders that are visible, but whose
+        /// bounds are outside the camera FOV.
+        /// </summary>
+        [UnityTest]
+        public IEnumerator TestPointerFOVLargeCollider()
         {
-            cube.transform.SetPositionAndRotation(Vector3.forward * 1f, Quaternion.identity);
-            cube.transform.localScale = Vector3.one * 0.1f;
-            yield return testHand.MoveTo(cube.transform.position);
+            var rightHand = new TestHand(Handedness.Right);
             yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
-            Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled, cube in front camera. Cube size {cube.transform.localScale} location {cube.transform.position}.");
 
-            // Make cube no longer visible
-            cube.transform.Translate(Vector3.up * 10);
-            yield return testHand.MoveTo(cube.transform.position);
+            var cube = GameObject.CreatePrimitive(PrimitiveType.Cube);
+            cube.AddComponent<NearInteractionGrabbable>();
+            cube.AddComponent<NearInteractionTouchableVolume>();
+            yield return rightHand.Show(Vector3.zero);
             yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
-            Assert.IsFalse(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should NOT be enabled, cube behind camera. Cube size {cube.transform.localScale} location {cube.transform.position}.");
 
-            // For sphere and poke pointers, test that setting IgnoreCollidersNotInFOV works
-            if (myPointer is SpherePointer spherePointer)
-            {
-                spherePointer.IgnoreCollidersNotInFOV = false;
-                yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
-                Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled because IgnoreCollidersNotInFOV is false.");
-                spherePointer.IgnoreCollidersNotInFOV = true;
-            }
-            else if (myPointer is PokePointer pokePointer)
-            {
-                pokePointer.IgnoreCollidersNotInFOV = false;
-                yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
-                Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled because IgnoreCollidersNotInFOV is false.");
-                pokePointer.IgnoreCollidersNotInFOV = true;
-            }
+            var spherePointer = PointerUtils.GetPointer<SpherePointer>(Handedness.Right);
+            var pokePointer = PointerUtils.GetPointer<PokePointer>(Handedness.Right);
 
-            // Move it back to be visible again
-            cube.transform.Translate(Vector3.up * -10f);
-            yield return testHand.MoveTo(cube.transform.position);
-            yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
-            Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled because it is near object inside of FOV. Cube size {cube.transform.localScale} location {cube.transform.position}.");
+            yield return TestPointerFOVLargeColliderHelper(spherePointer, cube, rightHand);
+            yield return TestPointerFOVLargeColliderHelper(pokePointer, cube, rightHand);
+
+            rightHand.Hide();
+            GameObject.Destroy(cube);
         }
 
         /// <summary>
         /// Tests that pointers behave correctly when interacting with objects inside and outside
         /// its field of view
         /// </summary>
         [UnityTest]
-        public IEnumerator TestPointerFieldOfView()
+        public IEnumerator TestPointerFOV()
         {
             var rightHand = new TestHand(Handedness.Right);
             yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
@@ -157,14 +146,13 @@ public IEnumerator TestPointerFieldOfView()
             var spherePointer = PointerUtils.GetPointer<SpherePointer>(Handedness.Right);
             var pokePointer = PointerUtils.GetPointer<PokePointer>(Handedness.Right);
 
-            yield return TestPointerFieldOfViewHelper(spherePointer, cube, rightHand);
-            yield return TestPointerFieldOfViewHelper(pokePointer, cube, rightHand);
+            yield return TestPointerFOVHelper(spherePointer, cube, rightHand);
+            yield return TestPointerFOVHelper(pokePointer, cube, rightHand);
 
             rightHand.Hide();
             GameObject.Destroy(cube);
         }
 
-        
         /// <summary>
         /// Tests that sphere pointer grabs object when hand is inside a giant grabbable
         /// </summary>
@@ -369,6 +357,69 @@ private static T CreatePointerPrefab<T>(string prefabPath,
             return pointer;
         }
 
+        private IEnumerator TestPointerFOVHelper(IMixedRealityPointer myPointer, GameObject cube, TestHand testHand)
+        {
+            // Cube in front of camera
+            cube.transform.SetPositionAndRotation(Vector3.forward * 1f, Quaternion.identity);
+            cube.transform.localScale = Vector3.one * 0.1f;
+            yield return testHand.MoveTo(cube.transform.position);
+            yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
+            Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled, cube in front camera. Cube size {cube.transform.localScale} location {cube.transform.position}.");
+
+            // Cube above camera
+            cube.transform.Translate(Vector3.up * 10);
+            yield return testHand.MoveTo(cube.transform.position);
+            yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
+            Assert.IsFalse(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should NOT be enabled, cube behind camera. Cube size {cube.transform.localScale} location {cube.transform.position}.");
+
+            // For sphere and poke pointers, test that setting IgnoreCollidersNotInFOV works
+            if (myPointer is SpherePointer spherePointer)
+            {
+                spherePointer.IgnoreCollidersNotInFOV = false;
+                yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
+                Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled because IgnoreCollidersNotInFOV is false.");
+                spherePointer.IgnoreCollidersNotInFOV = true;
+            }
+            else if (myPointer is PokePointer pokePointer)
+            {
+                pokePointer.IgnoreCollidersNotInFOV = false;
+                yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
+                Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled because IgnoreCollidersNotInFOV is false.");
+                pokePointer.IgnoreCollidersNotInFOV = true;
+            }
+
+            // Move it back to be visible again
+            cube.transform.Translate(Vector3.up * -10f);
+            yield return testHand.MoveTo(cube.transform.position);
+            yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
+            Assert.IsTrue(myPointer.IsInteractionEnabled, $"Pointer {myPointer.PointerName} should be enabled because it is near object inside of FOV. Cube size {cube.transform.localScale} location {cube.transform.position}.");
+        }
+
+        private IEnumerator TestPointerFOVLargeColliderHelper(IMixedRealityPointer myPointer, GameObject cube, TestHand testHand)
+        {
+            cube.transform.localScale = new Vector3(3, 3, 0.05f);
+            float[] yOffsets = new float[] { -1f, 0f, 1f };
+            float[] xOffsets = new float[] { -1f, 0f, 1f };
+            float[] zOffsets = new float[] { 1f, -1f };
+            var collider = cube.GetComponent<BoxCollider>();
+            foreach (var zOffset in zOffsets)
+            {
+                foreach (var yOffset in yOffsets)
+                {
+                    foreach (var xOffset in xOffsets)
+                    {
+                        var cameraPos = CameraCache.Main.transform.position;
+                        var pos = new Vector3(cameraPos.x + xOffset, cameraPos.y + yOffset, cameraPos.z + zOffset);
+                        cube.transform.position = pos;
+                        yield return testHand.MoveTo(cube.transform.position);
+                        yield return PlayModeTestUtilities.WaitForInputSystemUpdate();
+                        bool isInFov = CameraCache.Main.IsInFOVCached(collider);
+                        Assert.IsTrue(zOffset == 1f ? myPointer.IsInteractionEnabled : !myPointer.IsInteractionEnabled,
+                            $"Pointer {myPointer.PointerName} in incorrect state. IsInFOV {isInFov} Cube size {cube.transform.localScale} location {cube.transform.position}.");
+                    }
+                }
+            }
+        }
         #endregion
     }
 }

Assets/MixedRealityToolkit/Utilities/CameraFOVChecker.cs

@@ -5,79 +5,97 @@
 using System;
 using System.Collections.Generic;
 using UnityEngine;
+using UnityEngine.Profiling;
 
 namespace Microsoft.MixedReality.Toolkit
 {
     /// <summary>
-    /// Camera extension methods to test if colliders are within an FOV cone. Uses
+    /// Camera extension methods to test if colliders are within camera's FOV. Uses
     /// caching to improve performance and ensure values are only computed once per frame
     /// </summary>
     public static class CameraFOVChecker
     {
 
         // Help to clear caches when new frame runs
-        static private int inFOVConeLastCalculatedFrame = -1;
+        static private int inFOVLastCalculatedFrame = -1;
         // Map from grabbable => is the grabbable in FOV for this frame. Cleared every frame
-        private static Dictionary<Tuple<Collider, Camera>, bool> inFOVConeColliderCache = new Dictionary<Tuple<Collider, Camera>, bool>();
+        private static Dictionary<Tuple<Collider, Camera>, bool> inFOVColliderCache = new Dictionary<Tuple<Collider, Camera>, bool>();
         // List of corners shared across all sphere pointer query instances --
         // used to store list of corners for a bounds. Shared and static
         // to avoid allocating memory each frame
-        private static List<Vector3> inFOVConeBoundsCornerPoints = new List<Vector3>();
+        private static List<Vector3> inFOVBoundsCornerPoints = new List<Vector3>();
 
         /// <summary>
         /// Returns true if a collider's bounds is within the camera FOV. 
         /// Utilizes a cache to test if this collider has been seen before and returns current frame's calculated result.
+        /// NOTE: This is a 'loose' FOV check -- it can return true in cases when the collider is actually not in the FOV
+        /// because it does an axis-aligned check when testing for large colliders. So, if the axis aligned bounds are in the bounds of the camera, it will return true.
         /// </summary>
         /// <param name="myCollider">The collider to test</param>
-        public static bool IsInFOVConeCached(this Camera cam,
-            Collider myCollider)
+        public static bool IsInFOVCached(this Camera cam, Collider myCollider)
         {
             // if the collider's size is zero, it is not visible. Return false.
-            if(myCollider.bounds.size == Vector3.zero || myCollider.transform.localScale == Vector3.zero)
+            if (myCollider.bounds.size == Vector3.zero || myCollider.transform.localScale == Vector3.zero)
             {
                 return false;
             }
-            
+
             Tuple<Collider, Camera> cameraColliderPair = new Tuple<Collider, Camera>(myCollider, cam);
             bool result = false;
-            if (inFOVConeLastCalculatedFrame != Time.frameCount)
+            if (inFOVLastCalculatedFrame != Time.frameCount)
             {
-                inFOVConeColliderCache.Clear();
-                inFOVConeLastCalculatedFrame = Time.frameCount;
+                inFOVColliderCache.Clear();
+                inFOVLastCalculatedFrame = Time.frameCount;
             }
-            else if (inFOVConeColliderCache.TryGetValue(cameraColliderPair, out result))
+            else if (inFOVColliderCache.TryGetValue(cameraColliderPair, out result))
             {
                 return result;
             }
 
-            inFOVConeBoundsCornerPoints.Clear();
-            BoundsExtensions.GetColliderBoundsPoints(myCollider, inFOVConeBoundsCornerPoints, 0);
+            inFOVBoundsCornerPoints.Clear();
+            BoundsExtensions.GetColliderBoundsPoints(myCollider, inFOVBoundsCornerPoints, 0);
+
 
             float xMin = float.MaxValue, yMin = float.MaxValue, zMin = float.MaxValue;
             float xMax = float.MinValue, yMax = float.MinValue, zMax = float.MinValue;
-            for (int i = 0; i < inFOVConeBoundsCornerPoints.Count; i++)
+            for (int i = 0; i < inFOVBoundsCornerPoints.Count; i++)
             {
-                var corner = inFOVConeBoundsCornerPoints[i];
-                if (cam.IsInFOVCone(corner, 0))
+                var corner = inFOVBoundsCornerPoints[i];
+                Vector3 screenPoint = cam.WorldToViewportPoint(corner);
+
+                bool isInFOV = screenPoint.z >= 0 && screenPoint.z <= cam.farClipPlane
+                    && screenPoint.x >= 0 && screenPoint.x <= 1
+                    && screenPoint.y >= 0 && screenPoint.y <= 1;
+
+                if (isInFOV)
                 {
-                    inFOVConeColliderCache.Add(cameraColliderPair, true);
+                    inFOVColliderCache.Add(cameraColliderPair, true);
                     return true;
                 }
 
-                xMin = Mathf.Min(xMin, corner.x);
-                yMin = Mathf.Min(yMin, corner.y);
-                zMin = Mathf.Min(zMin, corner.z);
-                xMax = Mathf.Max(xMax, corner.x);
-                yMax = Mathf.Max(yMax, corner.y);
-                zMax = Mathf.Max(zMax, corner.z);
+                // if the point is behind the camera, the x and y viewport positions are negated
+                var zViewport = screenPoint.z;
+                var xViewport = zViewport >= 0 ? screenPoint.x : -screenPoint.x;
+                var yViewport = zViewport >= 0 ? screenPoint.y : -screenPoint.y;
+                xMin = Mathf.Min(xMin, xViewport);
+                yMin = Mathf.Min(yMin, yViewport);
+                zMin = Mathf.Min(zMin, zViewport);
+                xMax = Mathf.Max(xMax, xViewport);
+                yMax = Mathf.Max(yMax, yViewport);
+                zMax = Mathf.Max(zMax, zViewport);
             }
 
-            var cameraPos = cam.transform.position;
-            result = xMin <= cameraPos.x && cameraPos.x <= xMax
-                && yMin <= cameraPos.y && cameraPos.y <= yMax
-                && zMin <= cameraPos.z && cameraPos.z <= zMax;
+            // Check that collider is visible even if all corners are not visible
+            // such as when having a large collider
+            result =
+                zMax > 0 // Front of collider is in front of the camera.
+                && zMin < cam.farClipPlane // Back of collider is not too far away.
+                && xMin < 1 // Left edge is not too far to the right.
+                && xMax > 0 // Right edge is not too far to the left.
+                && yMin < 1 // Bottom edge is not too high.
+                && yMax > 0; // Top edge is not too low.
 
-            inFOVConeColliderCache.Add(cameraColliderPair, result);
+            inFOVColliderCache.Add(cameraColliderPair, result);
 
             return result;
         }