Fix: avatar indices and NRE in FinishAvatarMatricesCalculationSystem

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Components/AvatarTransformMatrixComponent.cs

@@ -2,18 +2,20 @@
 using DCL.Diagnostics;
 using DCL.Utility.Types;
 using UnityEngine;
+using System;
+using System.Runtime.InteropServices;
 
 namespace DCL.AvatarRendering.AvatarShape.Components
 {
     public struct AvatarTransformMatrixComponent
     {
-        public int IndexInGlobalJobArray;
+        public GlobalJobArrayIndex IndexInGlobalJobArray;
         public BoneArray bones;
 
         public static AvatarTransformMatrixComponent Create(BoneArray bones) =>
             new ()
             {
-                IndexInGlobalJobArray = -1,
+                IndexInGlobalJobArray = GlobalJobArrayIndex.Uninitialized(),
                 bones = bones,
             };
 
@@ -23,6 +25,53 @@ public static AvatarTransformMatrixComponent NewDefault() =>
 #endif
     }
 
+    [StructLayout(LayoutKind.Sequential)]
+    public readonly struct GlobalJobArrayIndex
+    {
+        private const int UNINITIALIZED = -100;
+        private const int NOT_ASSIGNED = -1;
+
+        private readonly int index;
+
+        private GlobalJobArrayIndex(int index)
+        {
+            this.index = index;
+        }
+
+        public static GlobalJobArrayIndex Uninitialized()
+        {
+            return new (UNINITIALIZED);
+        }
+
+        public static GlobalJobArrayIndex Unassign()
+        {
+            return new (NOT_ASSIGNED);
+        }
+
+        /// <summary>
+        /// Caller guarantees the int value is equal or greater than 0.
+        /// Otherwise use Uninitialized or Unassign consts
+        ///
+        ///    if (value < 0)
+        ///        throw new Exception($"Value is invalid: {value}");
+        /// </summary>
+        public static GlobalJobArrayIndex ValidUnsafe(int value)
+        {
+            return new GlobalJobArrayIndex(value);
+        }
+
+        public bool IsValid()
+        {
+            return index >= 0;
+        }
+
+        public bool TryGetValue(out int value)
+        {
+            value = index;
+            return index >= 0;
+        }
+    }
+
     /// <summary>
     /// Guarantees amount of bones in the array
     /// </summary>

Explorer/Assets/DCL/AvatarRendering/AvatarShape/Components/AvatarTransformMatrixJobWrapper.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using DCL.AvatarRendering.AvatarShape.ComputeShader;
 using DCL.AvatarRendering.AvatarShape.UnityInterface;
+using DCL.Diagnostics;
 using Unity.Collections;
 using Unity.Collections.LowLevel.Unsafe;
 using Unity.Jobs;
@@ -29,7 +30,7 @@ public class AvatarTransformMatrixJobWrapper : IDisposable
 
         private JobHandle handle;
 
-        private readonly Stack<int> releasedIndexes;
+        private readonly Stack<GlobalJobArrayIndex> releasedIndexes;
 
         private int avatarIndex;
         private int nextResizeValue;
@@ -53,7 +54,7 @@ public AvatarTransformMatrixJobWrapper()
             currentAvatarAmountSupported = AVATAR_ARRAY_SIZE;
 
             nextResizeValue = 2;
-            releasedIndexes = new Stack<int>();
+            releasedIndexes = new Stack<GlobalJobArrayIndex>();
         }
 
         public void ScheduleBoneMatrixCalculation()
@@ -70,20 +71,26 @@ public void CompleteBoneMatrixCalculations()
 
         public void UpdateAvatar(AvatarBase avatarBase, ref AvatarTransformMatrixComponent transformMatrixComponent)
         {
-            if (transformMatrixComponent.IndexInGlobalJobArray == -1)
+            if (transformMatrixComponent.IndexInGlobalJobArray.IsValid() == false)
             {
                 if (releasedIndexes.Count > 0)
                     transformMatrixComponent.IndexInGlobalJobArray = releasedIndexes.Pop();
                 else
                 {
-                    transformMatrixComponent.IndexInGlobalJobArray = avatarIndex;
+                    transformMatrixComponent.IndexInGlobalJobArray = GlobalJobArrayIndex.ValidUnsafe(avatarIndex);
                     avatarIndex++;
                 }
             }
 
+            if (transformMatrixComponent.IndexInGlobalJobArray.TryGetValue(out int validIndex) == false)
+            {
+                ReportHub.LogError(ReportCategory.AVATAR, "Invalid index after direct assignment");
+                return;
+            }
+
             Profiler.BeginSample("Calculate localToWorldMatrix on MainThread");
 
-            int globalIndexOffset = transformMatrixComponent.IndexInGlobalJobArray * BONES_ARRAY_LENGTH;
+            int globalIndexOffset = validIndex * BONES_ARRAY_LENGTH;
 
             //Add all bones to the bonesCombined array with the current available index
             for (int i = 0; i < BONES_ARRAY_LENGTH; i++)
@@ -92,8 +99,8 @@ public void UpdateAvatar(AvatarBase avatarBase, ref AvatarTransformMatrixCompone
             Profiler.EndSample();
 
             //Setup of data
-            matrixFromAllAvatars[transformMatrixComponent.IndexInGlobalJobArray] = avatarBase.transform.worldToLocalMatrix;
-            updateAvatar[transformMatrixComponent.IndexInGlobalJobArray] = true;
+            matrixFromAllAvatars[validIndex] = avatarBase.transform.worldToLocalMatrix;
+            updateAvatar[validIndex] = true;
 
             if (avatarIndex >= currentAvatarAmountSupported - 1)
                 ResizeArrays();
@@ -128,14 +135,14 @@ public void ReleaseAvatar(ref AvatarTransformMatrixComponent avatarTransformMatr
         {
             if (disposed) return;
 
-            if (avatarTransformMatrixComponent.IndexInGlobalJobArray == -1)
+            if (avatarTransformMatrixComponent.IndexInGlobalJobArray.TryGetValue(out int validIndex) == false)
                 return;
 
             //Dont update this index anymore until reset
-            updateAvatar[avatarTransformMatrixComponent.IndexInGlobalJobArray] = false;
+            updateAvatar[validIndex] = false;
             releasedIndexes.Push(avatarTransformMatrixComponent.IndexInGlobalJobArray);
 
-            avatarTransformMatrixComponent.IndexInGlobalJobArray = -1;
+            avatarTransformMatrixComponent.IndexInGlobalJobArray = GlobalJobArrayIndex.Unassign();
         }
 
         /// <summary>

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

@@ -4,10 +4,12 @@
 using Arch.SystemGroups.DefaultSystemGroups;
 using DCL.AvatarRendering.AvatarShape.Components;
 using DCL.AvatarRendering.AvatarShape.ComputeShader;
+using DCL.Diagnostics;
 using ECS.Abstract;
 using ECS.LifeCycle.Components;
 using Unity.Collections;
 using Unity.Mathematics;
+using System;
 
 namespace DCL.AvatarRendering.AvatarShape
 {
@@ -38,7 +40,10 @@ protected override void Update(float t)
         private void Execute(ref AvatarTransformMatrixComponent avatarTransformMatrixComponent,
             ref AvatarCustomSkinningComponent computeShaderSkinning)
         {
-            skinningStrategy.ComputeSkinning(currentResult, avatarTransformMatrixComponent.IndexInGlobalJobArray, ref computeShaderSkinning);
+            if (avatarTransformMatrixComponent.IndexInGlobalJobArray.TryGetValue(out int validIndex))
+                skinningStrategy.ComputeSkinning(currentResult, validIndex, ref computeShaderSkinning);
+            else
+                ReportHub.LogException(new Exception("Attempt to process an invalid avatar"), ReportCategory.AVATAR);
         }
     }
 }

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

@@ -25,11 +25,12 @@ public void AddNewAvatarIncrementsAvatarIndex()
     {
         var avatarBase = new GameObject().AddComponent<AvatarBase>();
         var transformMatrixComponent = AvatarTransformMatrixComponent.NewDefault();
-        int initialIndex = transformMatrixComponent.IndexInGlobalJobArray;
+        transformMatrixComponent.IndexInGlobalJobArray.TryGetValue(out int initialIndex);
 
         jobWrapper.UpdateAvatar(avatarBase, ref transformMatrixComponent);
 
-        Assert.AreNotEqual(initialIndex, transformMatrixComponent.IndexInGlobalJobArray);
+        transformMatrixComponent.IndexInGlobalJobArray.TryGetValue(out int afterIndex);
+        Assert.AreNotEqual(initialIndex, afterIndex);
     }
 
     [Test]
@@ -56,14 +57,14 @@ public void ReleasedIndexesAreReused()
 
         // Add the first avatar
         jobWrapper.UpdateAvatar(avatarBase, ref transformMatrixComponent1);
-        int firstIndex = transformMatrixComponent1.IndexInGlobalJobArray;
+        transformMatrixComponent1.IndexInGlobalJobArray.TryGetValue(out int firstIndex);
 
         // Release the first avatar
         jobWrapper.ReleaseAvatar(ref transformMatrixComponent1);
 
         // Add a second avatar and check if it reuses the released index
         jobWrapper.UpdateAvatar(avatarBase, ref transformMatrixComponent2);
-        int secondIndex = transformMatrixComponent2.IndexInGlobalJobArray;
+        transformMatrixComponent2.IndexInGlobalJobArray.TryGetValue(out int secondIndex);
 
         Assert.AreEqual(firstIndex, secondIndex);
     }