refactor(avatar-rendering): drop camera param from IsVisibleInCamera, pass CameraComponent as query data

Author: NickKhalow

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Systems/AvatarShapeVisibilitySystem.cs

@@ -32,9 +32,6 @@ public partial class AvatarShapeVisibilitySystem : BaseUnityLoopSystem
 
         private SingleInstanceEntity camera;
         private GameObject? playerCamera;
-        // Camera the cached frustum planes were built for. IsVisibleInCamera validates against this so a
-        // caller cannot read planes computed for a different camera (or before any were computed).
-        private Camera? planesCamera;
 
         public AvatarShapeVisibilitySystem(World world, IUserBlockingCache userBlockingCache, IRendererFeaturesCache outlineFeature, float startFadeDithering, float endFadeDithering, bool includeBannedUsersFromScene) : base(world)
         {
@@ -68,27 +65,21 @@ protected override void Update(float t)
 
             if (outlineFeature != null && outlineFeature.isActive)
             {
-                Camera cam = camera.GetCameraComponent(World).Camera;
-                CalculateFrustumPlanes(cam);
-                GetAvatarsVisibleWithOutlineQuery(World, cam);
+                CameraComponent cameraComponent = camera.GetCameraComponent(World);
+                CalculateFrustumPlanes(cameraComponent.Camera);
+                GetAvatarsVisibleWithOutlineQuery(World, cameraComponent);
             }
         }
 
-        public void CalculateFrustumPlanes(Camera camera)
+        internal void CalculateFrustumPlanes(Camera camera)
         {
             GeometryUtility.CalculateFrustumPlanes(camera, planes);
-            planesCamera = camera;
         }
 
         // Tests the AABB against the frustum planes cached by the most recent CalculateFrustumPlanes call.
-        // Extraction runs once per tick in Update (not per avatar), so this does not recompute the planes;
-        // the camera argument is validated to match the camera those cached planes were built for.
-        public bool IsVisibleInCamera(Camera camera, Bounds bounds)
-        {
-            UnityEngine.Assertions.Assert.IsTrue(ReferenceEquals(planesCamera, camera),
-                "IsVisibleInCamera reads planes cached by CalculateFrustumPlanes; call it for this camera in the current tick first.");
-            return GeometryUtility.TestPlanesAABB(planes, bounds);
-        }
+        // Extraction runs once per tick in Update (not per avatar), so this does not recompute the planes.
+        internal bool IsVisibleInCamera(Bounds bounds) =>
+            GeometryUtility.TestPlanesAABB(planes, bounds);
 
         public bool IsWithinCameraDistance(Camera camera, Transform objectTransform, float maxDistancesquared)
         {
@@ -98,9 +89,9 @@ public bool IsWithinCameraDistance(Camera camera, Transform objectTransform, flo
         }
 
         [Query]
-        private void GetAvatarsVisibleWithOutline([Data] Camera cam, in AvatarBase avatarBase, ref AvatarShapeComponent avatarShape)
+        private void GetAvatarsVisibleWithOutline([Data] in CameraComponent cameraComponent, in AvatarBase avatarBase, ref AvatarShapeComponent avatarShape)
         {
-            if (avatarShape.IsPreview || (IsWithinCameraDistance(cam, avatarBase.HeadAnchorPoint, 64.0f) && IsVisibleInCamera(cam, avatarBase.AvatarSkinnedMeshRenderer.bounds)))
+            if (avatarShape.IsPreview || (IsWithinCameraDistance(cameraComponent.Camera, avatarBase.HeadAnchorPoint, 64.0f) && IsVisibleInCamera(avatarBase.AvatarSkinnedMeshRenderer.bounds)))
             {
                 RendererFeature_AvatarOutline.m_AvatarOutlineRenderers.AddRange(avatarShape.OutlineCompatibleRenderers);
             }

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Tests/EditMode/AvatarShapeVisibilitySystemShould.cs

@@ -131,40 +131,12 @@ public void ReturnTrueWhenObjectIsVisibleInCamera()
 
             // Act
             system!.CalculateFrustumPlanes(testCamera);
-            bool isVisible = system.IsVisibleInCamera(testCamera, bounds);
+            bool isVisible = system.IsVisibleInCamera(bounds);
 
             // Assert
             Assert.IsTrue(isVisible);
         }
 
-        [Test]
-        public void IsVisibleInCameraRejectsPlanesFromDifferentCamera()
-        {
-            // The frustum-plane extraction is hoisted out of the per-avatar loop into CalculateFrustumPlanes,
-            // so IsVisibleInCamera reads cached planes. Querying it with a camera whose planes were not the
-            // ones just computed must be caught, not silently answered against another camera's planes.
-            var bounds = new Bounds(new Vector3(0, 1, 5), Vector3.one);
-
-            var otherCameraGo = new GameObject("OtherCamera");
-            createdGameObjects.Add(otherCameraGo);
-            Camera otherCamera = otherCameraGo.AddComponent<Camera>();
-
-            system.CalculateFrustumPlanes(testCamera);
-
-            bool previousRaiseExceptions = UnityEngine.Assertions.Assert.raiseExceptions;
-            UnityEngine.Assertions.Assert.raiseExceptions = true;
-
-            try
-            {
-                Assert.Throws<UnityEngine.Assertions.AssertionException>(
-                    () => system.IsVisibleInCamera(otherCamera, bounds));
-            }
-            finally
-            {
-                UnityEngine.Assertions.Assert.raiseExceptions = previousRaiseExceptions;
-            }
-        }
-
         [Test]
         public void ReturnFalseWhenObjectIsNotVisibleInCamera()
         {
@@ -174,7 +146,7 @@ public void ReturnFalseWhenObjectIsNotVisibleInCamera()
 
             // Act
             system!.CalculateFrustumPlanes(testCamera);
-            bool isVisible = system.IsVisibleInCamera(testCamera, bounds);
+            bool isVisible = system.IsVisibleInCamera(bounds);
 
             // Assert
             Assert.IsFalse(isVisible);