Add a small refactor to InputActionState & some tests for the area.

Author: Darren-Kelly-Unity

Assets/Tests/InputSystem/CoreTests_ActionsPriority.cs

@@ -417,4 +417,126 @@ public void AltShiftW_Only_Triggers_TeamChat(ThreeInputActionDataWrapper<InputAc
         // Release(keyboard.leftShiftKey);
         // Release(keyboard.altKey);
     }
+
+    [Test]
+    [Category("Actions Priority")]
+    public void Actions_Priority_ControlGroupingTable_StrideAndElementIndicesMatchInterleavedLayout()
+    {
+        Assert.That(InputActionState.ControlGroupingTable.Stride, Is.EqualTo(2));
+        Assert.That(InputActionState.ControlGroupingTable.GroupElementIndex(3), Is.EqualTo(6));
+        Assert.That(InputActionState.ControlGroupingTable.PriorityElementIndex(3), Is.EqualTo(7));
+    }
+
+    [Test]
+    [Category("Actions Priority")]
+    public void Actions_Priority_InputActionStateMonitorIndex_RoundTripsComponents()
+    {
+        var index = InputActionStateMonitorIndex.Create(mapIndex: 7, controlIndex: 0x00abcdef, bindingIndex: 0x0bcd,
+            priority: 200);
+
+        Assert.That(index.MapIndex, Is.EqualTo(7));
+        Assert.That(index.ControlIndex, Is.EqualTo(0x00abcdef));
+        Assert.That(index.BindingIndex, Is.EqualTo(0x0bcd));
+        Assert.That(index.Priority, Is.EqualTo(200));
+    }
+
+    [Test]
+    [Category("Actions Priority")]
+    public void Actions_Priority_InputActionStateMonitorIndex_FromPacked_MatchesCreateOutput()
+    {
+        var created = InputActionStateMonitorIndex.Create(3, 100, 200, 42);
+        var roundTrip = InputActionStateMonitorIndex.FromPacked(created.Packed);
+
+        Assert.That(roundTrip.MapIndex, Is.EqualTo(created.MapIndex));
+        Assert.That(roundTrip.ControlIndex, Is.EqualTo(created.ControlIndex));
+        Assert.That(roundTrip.BindingIndex, Is.EqualTo(created.BindingIndex));
+        Assert.That(roundTrip.Priority, Is.EqualTo(created.Priority));
+    }
+
+    [Test]
+    [Category("Actions Priority")]
+    public void Actions_Priority_InputActionStateMonitorIndex_PriorityUsesLowEightBitsInPackedRepresentation()
+    {
+        var index = InputActionStateMonitorIndex.Create(0, 1, 0, priority: 300);
+        Assert.That(index.Priority, Is.EqualTo(300 & 0xff));
+    }
+
+    [Test]
+    [Category("Actions Priority")]
+    public void Actions_Priority_InputActionStateMonitorIndex_ImplicitConversionToLongMatchesPackedProperty()
+    {
+        var index = InputActionStateMonitorIndex.Create(1, 2, 3, 4);
+        long asLong = index;
+        Assert.That(asLong, Is.EqualTo(index.Packed));
+    }
+
+    [Test]
+    [Category("Actions Priority")]
+    public unsafe void Actions_Priority_ControlGrouping_SamePhysicalControlSharesGroupId()
+    {
+        var keyboard = InputSystem.AddDevice<Keyboard>();
+        var map = new InputActionMap("priority_group_test");
+        map.AddAction("a", binding: "<Keyboard>/z");
+        map.AddAction("b", binding: "<Keyboard>/z");
+        map.Enable();
+
+        var state = map.m_State;
+        Assert.That(state, Is.Not.Null);
+        Assert.That(state.memory.controlGroupingInitialized, Is.True);
+
+        for (var i = 0; i < state.totalControlCount; ++i)
+        {
+            for (var j = i + 1; j < state.totalControlCount; ++j)
+            {
+                if (state.controls[i] != state.controls[j])
+                    continue;
+
+                var gi = InputActionState.ControlGroupingTable.GroupElementIndex(i);
+                var gj = InputActionState.ControlGroupingTable.GroupElementIndex(j);
+                Assert.That(state.memory.controlGroupingAndPriority[gi], Is.EqualTo(state.memory.controlGroupingAndPriority[gj]));
+                Assert.That(state.memory.controlGroupingAndPriority[gi], Is.Not.EqualTo(0));
+                return;
+            }
+        }
+
+        Assert.Fail("Expected two control slots bound to the same physical control.");
+    }
+
+    [Test]
+    [Category("Actions Priority")]
+    public unsafe void Actions_Priority_ControlGrouping_WritesPerControlSlotPriorityFromAction()
+    {
+        var keyboard = InputSystem.AddDevice<Keyboard>();
+        var map = new InputActionMap("priority_per_slot_test");
+        var actionLow = map.AddAction("low", binding: "<Keyboard>/x");
+        var actionHigh = map.AddAction("high", binding: "<Keyboard>/x");
+        actionLow.Priority = 4;
+        actionHigh.Priority = 11;
+        map.Enable();
+
+        var state = map.m_State;
+        Assert.That(state, Is.Not.Null);
+
+        var lowIndex = -1;
+        var highIndex = -1;
+        for (var i = 0; i < state.totalControlCount; ++i)
+        {
+            if (state.controls[i] != keyboard.xKey)
+                continue;
+            var bindingIndex = state.controlIndexToBindingIndex[i];
+            var actionIndex = state.bindingStates[bindingIndex].actionIndex;
+            if (actionIndex == actionLow.m_ActionIndexInState)
+                lowIndex = i;
+            else if (actionIndex == actionHigh.m_ActionIndexInState)
+                highIndex = i;
+        }
+
+        Assert.That(lowIndex, Is.GreaterThanOrEqualTo(0));
+        Assert.That(highIndex, Is.GreaterThanOrEqualTo(0));
+
+        var pLow = InputActionState.ControlGroupingTable.PriorityElementIndex(lowIndex);
+        var pHigh = InputActionState.ControlGroupingTable.PriorityElementIndex(highIndex);
+        Assert.That(state.memory.controlGroupingAndPriority[pLow], Is.EqualTo(4));
+        Assert.That(state.memory.controlGroupingAndPriority[pHigh], Is.EqualTo(11));
+    }
 }

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs

@@ -112,7 +112,26 @@ internal unsafe class InputActionState : IInputStateChangeMonitor, ICloneable, I
         public BindingState* bindingStates => memory.bindingStates;
         public InteractionState* interactionStates => memory.interactionStates;
         public int* controlIndexToBindingIndex => memory.controlIndexToBindingIndex;
-        public ushort* controlGroupingAndPriority => memory.controlGroupingAndPriority;
+        private ushort* controlGroupingAndPriority => memory.controlGroupingAndPriority;
+
+        /// <summary>
+        /// Layout of <see cref="UnmanagedMemory.controlGroupingAndPriority"/>: interleaved ushort pairs (group id, binding priority) per control slot.
+        /// </summary>
+        internal static class ControlGroupingTable
+        {
+            public const int Stride = 2;
+
+            public static int GroupElementIndex(int controlIndex) => controlIndex * Stride;
+
+            public static int PriorityElementIndex(int controlIndex) => controlIndex * Stride + 1;
+        }
+
+        private uint GetControlMonitorGroupIndex(int controlIndex) =>
+            controlGroupingAndPriority[ControlGroupingTable.GroupElementIndex(controlIndex)];
+
+        private int GetControlBindingPriority(int controlIndex) =>
+            controlGroupingAndPriority[ControlGroupingTable.PriorityElementIndex(controlIndex)];
+
         public float* controlMagnitudes => memory.controlMagnitudes;
         public uint* enabledControls => (uint*)memory.enabledControls;
 
@@ -160,10 +179,10 @@ private void ComputeControlGroupingIfNecessary()
 
                 var priority = Math.Clamp(action != null ? action.Priority : 0, 0, 65535);
 
-                controlGroupingAndPriority[i * 2 + 1] = (ushort)priority;
+                controlGroupingAndPriority[ControlGroupingTable.PriorityElementIndex(i)] = (ushort)priority;
 
                 // Compute grouping. If already set, skip.
-                if (controlGroupingAndPriority[i * 2] == 0)
+                if (controlGroupingAndPriority[ControlGroupingTable.GroupElementIndex(i)] == 0)
                 {
                     for (var n = 0; n < totalControlCount; ++n)
                     {
@@ -176,10 +195,10 @@ private void ComputeControlGroupingIfNecessary()
                         if (control != otherControl)
                             continue;
 
-                        controlGroupingAndPriority[n * 2] = (ushort)currentGroup;
+                        controlGroupingAndPriority[ControlGroupingTable.GroupElementIndex(n)] = (ushort)currentGroup;
                     }
 
-                    controlGroupingAndPriority[i * 2] = (ushort)currentGroup;
+                    controlGroupingAndPriority[ControlGroupingTable.GroupElementIndex(i)] = (ushort)currentGroup;
 
                     ++currentGroup;
                 }
@@ -1150,7 +1169,8 @@ private void EnableControls(int mapIndex, int controlStartIndex, int numControls
                 var bindingStatePtr = &bindingStates[bindingIndex];
                 if (bindingStatePtr->wantsInitialStateCheck)
                     SetInitialStateCheckPending(bindingStatePtr, true);
-                manager.AddStateChangeMonitor(controls[controlIndex], this, mapControlAndBindingIndex, controlGroupingAndPriority[controlIndex * 2]);
+                manager.AddStateChangeMonitor(controls[controlIndex], this, mapControlAndBindingIndex,
+                    GetControlMonitorGroupIndex(controlIndex));
 
                 SetControlEnabled(controlIndex, true);
             }
@@ -1344,19 +1364,21 @@ void IInputStateChangeMonitor.NotifyControlStateChanged(InputControl control, do
                 return;
             #endif
 
-            SplitUpMapAndControlAndBindingIndex(mapControlAndBindingIndex, out var mapIndex, out var controlIndex, out var bindingIndex);
-            ProcessControlStateChange(mapIndex, controlIndex, bindingIndex, time, eventPtr);
+            var monitorIndex = InputActionStateMonitorIndex.FromPacked(mapControlAndBindingIndex);
+            ProcessControlStateChange(monitorIndex.MapIndex, monitorIndex.ControlIndex, monitorIndex.BindingIndex, time,
+                eventPtr);
         }
 
         void IInputStateChangeMonitor.NotifyTimerExpired(InputControl control, double time,
             long mapControlAndBindingIndex, int interactionIndex)
         {
-            SplitUpMapAndControlAndBindingIndex(mapControlAndBindingIndex, out var mapIndex, out var controlIndex, out var bindingIndex);
-            ProcessTimeout(time, mapIndex, controlIndex, bindingIndex, interactionIndex);
+            var monitorIndex = InputActionStateMonitorIndex.FromPacked(mapControlAndBindingIndex);
+            ProcessTimeout(time, monitorIndex.MapIndex, monitorIndex.ControlIndex, monitorIndex.BindingIndex,
+                interactionIndex);
         }
 
         /// <summary>
-        /// Bit pack the mapIndex, controlIndex, bindingIndex and complexity components into a single long monitor index value.
+        /// Bit pack the mapIndex, controlIndex, bindingIndex and binding-priority components into a single long monitor index value.
         /// </summary>
         /// <param name="mapIndex">The mapIndex value to pack.</param>
         /// <param name="controlIndex">The controlIndex value to pack.</param>
@@ -1366,42 +1388,14 @@ void IInputStateChangeMonitor.NotifyTimerExpired(InputControl control, double ti
         /// monitors. While we could look up map and binding indices from control indices, keeping
         /// all the information together avoids having to unnecessarily jump around in memory to grab
         /// the various pieces of data.
-        /// The complexity component is implicitly derived and does not need to be passed as an argument.
+        /// Priority is read from <see cref="ControlGroupingTable"/> data for <paramref name="controlIndex"/>.
         /// </remarks>
         private long ToCombinedMapAndControlAndBindingIndex(int mapIndex, int controlIndex, int bindingIndex)
         {
             // We have limits on the numbers of maps, controls, and bindings we allow in any single
             // action state (see TriggerState.kMaxNumXXX).
-            var complexity = controlGroupingAndPriority[controlIndex * 2 + 1];
-            var result = (long)controlIndex;
-            result |= (long)bindingIndex << 24;
-            result |= (long)mapIndex << 40;
-            result |= (long)complexity << 48;
-            return result;
-        }
-
-        /// <summary>
-        /// Extract the mapIndex, controlIndex and bindingIndex components from the provided bit packed argument (monitor index).
-        /// </summary>
-        /// <param name="mapControlAndBindingIndex">Represents a monitor index, which is a bit packed field containing multiple components.</param>
-        /// <param name="mapIndex">Will hold the extracted mapIndex value after the function completes.</param>
-        /// <param name="controlIndex">Will hold the extracted controlIndex value after the function completes.</param>
-        /// <param name="bindingIndex">Will hold the extracted bindingIndex value after the function completes.</param>
-        private void SplitUpMapAndControlAndBindingIndex(long mapControlAndBindingIndex, out int mapIndex,
-            out int controlIndex, out int bindingIndex)
-        {
-            controlIndex = (int)(mapControlAndBindingIndex & 0x00ffffff);
-            bindingIndex = (int)((mapControlAndBindingIndex >> 24) & 0xffff);
-            mapIndex = (int)((mapControlAndBindingIndex >> 40) & 0xff);
-        }
-
-        /// <summary>
-        /// Extract the 'complexity' component from the provided bit packed argument (monitor index).
-        /// </summary>
-        /// <param name="mapControlAndBindingIndex">Represents a monitor index, which is a bit packed field containing multiple components.</param>
-        internal static int GetPriorityFromMonitorIndex(long mapControlAndBindingIndex)
-        {
-            return (int)((mapControlAndBindingIndex >> 48) & 0xff);
+            return InputActionStateMonitorIndex.Create(mapIndex, controlIndex, bindingIndex,
+                GetControlBindingPriority(controlIndex)).Packed;
         }
 
         /// <summary>
@@ -2462,7 +2456,7 @@ private void ChangePhaseOfActionInternal(int actionIndex, TriggerState* actionSt
                 // When we perform an action, we mark the event handled such that FireStateChangeNotifications()
                 // can then reset state monitors in the same group (strictly lower binding priority only).
                 // NOTE: We don't consume for controls at binding priority 0. Those we fire in unison.
-                if (controlGroupingAndPriority[trigger.controlIndex * 2 + 1] > 0 &&
+                if (GetControlBindingPriority(trigger.controlIndex) > 0 &&
                     // we can end up switching to performed state from an interaction with a timeout, at which point
                     // the original event will probably have been removed from memory, so make sure to check
                     // we still have one

Packages/com.unity.inputsystem/InputSystem/InputManagerStateMonitors.cs

@@ -270,12 +270,12 @@ internal unsafe void FireStateChangeNotifications(int deviceIndex, double intern
                 if (!previouslyHandled && eventPtr->handled)
                 {
                     var groupIndex = listeners[i].groupIndex;
-                    var handlerPriority = InputActionState.GetPriorityFromMonitorIndex(listener.monitorIndex);
+                    var handlerPriority = InputActionStateMonitorIndex.FromPacked(listener.monitorIndex).Priority;
                     for (var n = i + 1; n < signals.length; ++n)
                     {
                         if (listeners[n].groupIndex == groupIndex && listeners[n].monitor == listener.monitor)
                         {
-                            var candidatePriority = InputActionState.GetPriorityFromMonitorIndex(listeners[n].monitorIndex);
+                            var candidatePriority = InputActionStateMonitorIndex.FromPacked(listeners[n].monitorIndex).Priority;
                             if (candidatePriority < handlerPriority)
                                 signals.ClearBit(n);
                         }
@@ -412,8 +412,8 @@ public void SortMonitorsByIndex()
                 {
                     for (var j = i; j > 0; --j)
                     {
-                        var firstPriority = InputActionState.GetPriorityFromMonitorIndex(listeners[j - 1].monitorIndex);
-                        var secondPriority = InputActionState.GetPriorityFromMonitorIndex(listeners[j].monitorIndex);
+                        var firstPriority = InputActionStateMonitorIndex.FromPacked(listeners[j - 1].monitorIndex).Priority;
+                        var secondPriority = InputActionStateMonitorIndex.FromPacked(listeners[j].monitorIndex).Priority;
                         if (firstPriority >= secondPriority)
                             break;