diff --git a/Assets/Tests/InputSystem.Editor/InputTestFixtureTeardownTests.cs b/Assets/Tests/InputSystem.Editor/InputTestFixtureTeardownTests.cs new file mode 100644 index 0000000000..5e2e50872f --- /dev/null +++ b/Assets/Tests/InputSystem.Editor/InputTestFixtureTeardownTests.cs @@ -0,0 +1,106 @@ +using NUnit.Framework; +using UnityEngine; +using UnityEngine.InputSystem; + +/// +/// Regression tests for IN-107889: InputTestFixture.TearDown() fails to reset Input System state +/// between consecutive scene-based tests. +/// +/// +/// Bug: When an belonging to the project-wide +/// asset is enabled before a test's runs, the TearDown/Restore +/// cycle leaves the action map disconnected from its saved . +/// +/// Root cause: +/// 1. TestHook_DisableActions() calls Disable() + OnSetupChanged() on the +/// project-wide asset's maps. Disable() modifies the action state memory (phases → Disabled) +/// on the same InputActionState objects that were just saved in the snapshot, because +/// Disable() is called after SaveAndResetState() but references the same managed objects. +/// 2. OnSetupChanged() sets map.m_State = null, disconnecting the map from its state. +/// 3. Restore() restores s_GlobalState (the registry) but does NOT restore the per-map +/// back-references (InputActionMap.m_State, m_MapIndexInState) or the action phase +/// memory. +/// +/// The fix: +/// 1. TestHook_DisableActions() should disconnect maps without modifying the saved state's memory, +/// so the saved snapshot retains the correct enabled phases. +/// 2. Restore() should re-link the back-references after RestoreSavedState() and +/// recompute m_EnabledActionsCount from the restored action phases. +/// +internal class InputTestFixtureTeardownTests : InputTestFixture +{ + // Simulates a scene with a PlayerInput component using project-wide actions. + // Created once before any [SetUp] runs, like a scene's OnEnable(). + private InputActionAsset m_PreTestAsset; + private InputActionMap m_PreTestMap; + private InputAction m_PreTestAction; + + [OneTimeSetUp] + public void SimulateSceneLoad() + { + // Create an action asset simulating project-wide actions that a scene's + // PlayerInput component would use. + m_PreTestAsset = ScriptableObject.CreateInstance(); + m_PreTestAsset.hideFlags = HideFlags.HideAndDontSave; + m_PreTestMap = m_PreTestAsset.AddActionMap("Player"); + m_PreTestAction = m_PreTestMap.AddAction("Fire", InputActionType.Button); + + // Enable the map - creates an InputActionState and registers it in s_GlobalState. + // This simulates PlayerInput.ActivateInput() running before the test starts. + m_PreTestMap.Enable(); + + // Register as project-wide actions on the real manager. + // This causes TestHook_DisableActions() to treat our asset like project-wide actions + // (i.e. call Disable() + OnSetupChanged() on it during each Setup()). + InputSystem.s_Manager.actions = m_PreTestAsset; + } + + [OneTimeTearDown] + public void VerifyPreTestStatePreserved() + { + // After all test TearDowns, the action map should still be linked to its saved + // InputActionState and the enabled count should reflect the pre-test enabled state. + // + // With the bug: map.m_State is null and map.enabled is false because: + // - TestHook_DisableActions() called Disable() on the map (clearing enabled state + // in the shared state memory) then OnSetupChanged() (setting m_State = null) + // - Restore() restores s_GlobalState but not map.m_State or m_EnabledActionsCount + // + // With the fix: map.m_State is properly re-linked and map.enabled is true because: + // - TestHook_DisableActions() no longer modifies the saved state's memory + // - Restore() calls RelinkRestoredStates() to restore back-references and + // recompute m_EnabledActionsCount from the action phase memory + + Assert.That(m_PreTestMap.m_State, Is.Not.Null, + "Action map should be linked to its saved InputActionState after Restore(). " + + "m_State was cleared by OnSetupChanged() during TestHook_DisableActions() " + + "and was not re-linked by Restore()."); + + Assert.That(m_PreTestMap.enabled, Is.True, + "Action map should be enabled after Restore(): it was enabled before any test ran " + + "and should be restored to that enabled state after TearDown()."); + + // Clean up + InputSystem.s_Manager.actions = null; + Object.DestroyImmediate(m_PreTestAsset); + } + + [Test] + [Order(1)] + public void TearDown_FirstTest_ProjectWideActionsAreReenabledForTest() + { + // During this test, project-wide actions may have been re-enabled by TestHook_EnableActions + // (or left disabled if TestHook_EnableActions is a no-op for the test manager). + // We're just running to trigger a Setup/TearDown cycle. + Assert.Pass("First test ran successfully (triggering Setup/TearDown cycle)"); + } + + [Test] + [Order(2)] + public void TearDown_SecondTest_StateRemainsCorrectAfterSecondCycle() + { + // After the first test's TearDown() + this Setup(), verify setup completes without errors. + // The [OneTimeTearDown] contains the actual assertion for the post-Restore() state. + Assert.Pass("Second test ran successfully (triggering second Setup/TearDown cycle)"); + } +} diff --git a/Assets/Tests/InputSystem/TouchscreenMultiDisplayTests.cs b/Assets/Tests/InputSystem/TouchscreenMultiDisplayTests.cs new file mode 100644 index 0000000000..e6c440ac0f --- /dev/null +++ b/Assets/Tests/InputSystem/TouchscreenMultiDisplayTests.cs @@ -0,0 +1,84 @@ +using NUnit.Framework; +using UnityEngine; +using UnityEngine.InputSystem; +using UnityEngine.InputSystem.LowLevel; +using TouchPhase = UnityEngine.InputSystem.TouchPhase; + +// Tests covering touch tracking across multiple physical touchscreen monitors. +// Regression coverage for IN-108611: touches from one screen incorrectly matching +// ongoing touches from another screen when both screens report the same touchId. +[TestFixture] +internal class TouchscreenMultiDisplayTests : CoreTestsFixture +{ + // When two physical touchscreens both have an active touch with the same touchId, + // a Move event from screen 2 must update screen 2's touch slot, not screen 1's. + // + // Failure mode (pre-fix): OnStateEvent matches ongoing touches by touchId alone, + // so screen 2's Move event finds screen 1's slot first (both have touchId=1) and + // incorrectly updates it, leaving screen 1's position changed and screen 2 stale. + [Test] + [Category("Devices")] + public void Devices_TouchMoveOnSecondDisplay_DoesNotUpdateTouchOnFirstDisplay() + { + var device = InputSystem.AddDevice(); + + // Finger down on display 0 (touchId=1). + InputSystem.QueueStateEvent(device, new TouchState + { + phase = TouchPhase.Began, + touchId = 1, + position = new Vector2(100, 100), + displayIndex = 0, + }); + + // Finger down on display 1 — same touchId, different screen. + InputSystem.QueueStateEvent(device, new TouchState + { + phase = TouchPhase.Began, + touchId = 1, + position = new Vector2(200, 200), + displayIndex = 1, + }); + + InputSystem.Update(); + + // Both touches should be allocated to separate slots. + Assert.That(device.touches[0].phase.ReadValue(), Is.EqualTo(TouchPhase.Began)); + Assert.That(device.touches[1].phase.ReadValue(), Is.EqualTo(TouchPhase.Began)); + + var display0SlotIndex = -1; + var display1SlotIndex = -1; + for (var i = 0; i < 2; i++) + { + var displayIdx = device.touches[i].displayIndex.ReadValue(); + if (displayIdx == 0) display0SlotIndex = i; + else if (displayIdx == 1) display1SlotIndex = i; + } + + Assert.That(display0SlotIndex, Is.Not.EqualTo(-1), "No touch slot found for display 0"); + Assert.That(display1SlotIndex, Is.Not.EqualTo(-1), "No touch slot found for display 1"); + + var display0PositionBefore = device.touches[display0SlotIndex].position.ReadValue(); + + // Swipe on display 1 (same touchId=1 as display 0's held touch). + InputSystem.QueueStateEvent(device, new TouchState + { + phase = TouchPhase.Moved, + touchId = 1, + position = new Vector2(300, 300), + displayIndex = 1, + }); + + InputSystem.Update(); + + // Display 0's touch must be unchanged. + Assert.That(device.touches[display0SlotIndex].position.ReadValue(), Is.EqualTo(display0PositionBefore), + "Touch on display 0 was incorrectly updated by a Move event from display 1"); + + // Display 1's touch must reflect the new position. + Assert.That(device.touches[display1SlotIndex].position.ReadValue(), Is.EqualTo(new Vector2(300, 300)), + "Touch on display 1 was not updated by its own Move event"); + + Assert.That(device.touches[display1SlotIndex].phase.ReadValue(), Is.EqualTo(TouchPhase.Moved)); + } +} diff --git a/Assets/Tests/InputSystem/TouchscreenMultiDisplayTests.cs.meta b/Assets/Tests/InputSystem/TouchscreenMultiDisplayTests.cs.meta new file mode 100644 index 0000000000..a9587bb70a --- /dev/null +++ b/Assets/Tests/InputSystem/TouchscreenMultiDisplayTests.cs.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: a5c39c3a4a0654c28a5b7754add41d2a \ No newline at end of file diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs index 4325fc1988..d442f8197c 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionState.cs @@ -4532,6 +4532,64 @@ internal static ISavedState SaveAndResetState() return savedState; } + /// + /// After restores the global registry via + /// RestoreSavedState(), the per-map back-references on each + /// (m_State, m_MapIndexInState) and the per-action index + /// (InputAction.m_ActionIndexInState) may be stale because they were cleared by + /// Destroy() during StaticDisposeCurrentState(). This method re-links those + /// references from the restored and recomputes + /// m_EnabledActionsCount from the action phase memory so that maps and actions + /// correctly reflect their pre-test enabled state. See IN-107889. + /// + internal static void RelinkRestoredStates() + { + var count = s_GlobalState.globalList.length; + for (var i = 0; i < count; ++i) + { + var handle = s_GlobalState.globalList[i]; + if (!handle.IsAllocated) + continue; + if (handle.Target is InputActionState state) + state.RelinkMapsAndRecomputeEnabledCount(); + } + } + + private unsafe void RelinkMapsAndRecomputeEnabledCount() + { + for (var mapIndex = 0; mapIndex < totalMapCount; ++mapIndex) + { + var map = maps[mapIndex]; + if (map == null) + continue; + + map.m_State = this; + map.m_MapIndexInState = mapIndex; + + if (map.m_Asset != null && map.m_Asset.m_SharedStateForAllMaps == null) + map.m_Asset.m_SharedStateForAllMaps = this; + + var indices = mapIndices[mapIndex]; + var mapActions = map.m_Actions; + if (mapActions != null) + { + for (var k = 0; k < indices.actionCount; ++k) + mapActions[k].m_ActionIndexInState = indices.actionStartIndex + k; + } + + // Recompute m_EnabledActionsCount from the restored action phase memory. + // This correctly reflects enabled/disabled state without any explicit Disable() + // call having been made (see TestHook_DisableActions changes for IN-107889). + var enabledCount = 0; + for (var k = 0; k < indices.actionCount; ++k) + { + if (!actionStates[indices.actionStartIndex + k].isDisabled) + ++enabledCount; + } + map.m_EnabledActionsCount = enabledCount; + } + } + private void AddToGlobalList() { CompactGlobalList(); diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Devices/Touchscreen.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Devices/Touchscreen.cs index 7a82e6a177..d90811dcd4 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Devices/Touchscreen.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Devices/Touchscreen.cs @@ -694,7 +694,7 @@ protected override void FinishSetup() var touchId = newTouchState.touchId; for (var i = 0; i < touchControlCount; ++i) { - if (currentTouchState[i].touchId == touchId) + if (currentTouchState[i].touchId == touchId && currentTouchState[i].displayIndex == newTouchState.displayIndex) { // Preserve primary touch state. var isPrimaryTouch = currentTouchState[i].isPrimaryTouch; @@ -915,7 +915,7 @@ unsafe bool IInputStateCallbackReceiver.GetStateOffsetForEvent(InputControl cont for (var i = 0; i < touchControlCount; ++i) { var touch = ¤tTouchState[i]; - if (touch->touchId == eventTouchId || (!touch->isInProgress && eventTouchPhase.IsActive())) + if ((touch->touchId == eventTouchId && touch->displayIndex == eventTouchState->displayIndex) || (!touch->isInProgress && eventTouchPhase.IsActive())) { offset = primaryTouch.m_StateBlock.byteOffset + primaryTouch.m_StateBlock.alignedSizeInBytes - m_StateBlock.byteOffset + (uint)(i * UnsafeUtility.SizeOf()); @@ -1002,7 +1002,7 @@ internal static unsafe bool MergeForward(InputEventPtr currentEventPtr, InputEve var currentState = (TouchState*)currentEvent->state; var nextState = (TouchState*)nextEvent->state; - if (currentState->touchId != nextState->touchId || currentState->phaseId != nextState->phaseId || currentState->flags != nextState->flags) + if (currentState->touchId != nextState->touchId || currentState->phaseId != nextState->phaseId || currentState->flags != nextState->flags || currentState->displayIndex != nextState->displayIndex) return false; nextState->delta += currentState->delta; diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs index a2cd47bf74..200bb336b7 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs @@ -3518,9 +3518,48 @@ internal static void InitializeInPlayer(IInputRuntime runtime = null, bool loadS #if UNITY_INCLUDE_TESTS internal static void TestHook_DisableActions() { - DisableActions(triggerSetupChanged: true); - if (s_Manager != null) + // Disconnect the project-wide action maps from their InputActionState without + // calling Disable(), which would corrupt the saved state snapshot. + // + // The normal DisableActions() path calls Disable() then OnSetupChanged() on the + // project-wide asset. Disable() modifies action phase memory on the *same* managed + // InputActionState objects that were already captured in the SaveAndResetState() + // snapshot (GCHandles point to the live objects; no deep copy is made). This means + // the restored snapshot has disabled phases even though the maps were enabled when + // the snapshot was taken, causing Restore() to leave maps incorrectly disabled. + // + // Instead, we just null out the map back-references and let Restore() re-link them. + // Control monitors registered through the old runtime don't need unsubscribing here + // because the test manager installs a new runtime; the old runtime is never polled + // during the test. See IN-107889. + var projectWideActions = s_Manager?.actions; + if (projectWideActions != null) + { + DisconnectActionMaps(projectWideActions); s_Manager.actions = null; + } + + // Also disconnect the configured project-wide asset even if manager.actions is null. + // RelinkRestoredStates() restores m_EnabledActionsCount on maps after Restore(). If a + // subsequent test's manager.actions is null (e.g. a previous OneTimeTearDown cleared it), + // TestHook_DisableActions would be a no-op and those maps would keep m_State set and + // m_EnabledActionsCount == m_Actions.Length. InputActionMap.Enable() would then + // early-return thinking all actions are already enabled, but the state is not in + // s_GlobalState (which was cleared by SaveAndResetState()). See IN-107889. + var configuredActions = InputManager.s_GetProjectWideActions?.Invoke(); + if (configuredActions != null && configuredActions != projectWideActions) + DisconnectActionMaps(configuredActions); + } + + private static void DisconnectActionMaps(InputActionAsset asset) + { + foreach (var map in asset.actionMaps) + { + map.m_State = null; + map.m_MapIndexInState = InputActionState.kInvalidIndex; + map.m_EnabledActionsCount = 0; + } + asset.m_SharedStateForAllMaps = null; } internal static void TestHook_EnableActions() diff --git a/Packages/com.unity.inputsystem/InputSystem/Runtime/Plugins/EnhancedTouch/TouchSimulation.cs b/Packages/com.unity.inputsystem/InputSystem/Runtime/Plugins/EnhancedTouch/TouchSimulation.cs index d7bbc15ccd..d3ad485357 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Runtime/Plugins/EnhancedTouch/TouchSimulation.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Runtime/Plugins/EnhancedTouch/TouchSimulation.cs @@ -253,6 +253,9 @@ protected void OnEnable() if (m_TouchIds == null) m_TouchIds = new int[simulatedTouchscreen.touches.Count]; + if (m_TouchDisplayIndices == null) + m_TouchDisplayIndices = new byte[simulatedTouchscreen.touches.Count]; + foreach (var device in InputSystem.devices) OnDeviceChange(device, InputDeviceChange.Added); @@ -306,10 +309,12 @@ private unsafe void UpdateTouch(int touchIndex, int pointerIndex, TouchPhase pha touch.startPosition = position; touch.touchId = ++m_LastTouchId; m_TouchIds[touchIndex] = m_LastTouchId; + m_TouchDisplayIndices[touchIndex] = displayIndex; } else { touch.touchId = m_TouchIds[touchIndex]; + touch.displayIndex = m_TouchDisplayIndices[touchIndex]; } //NOTE: Processing these events still happen in the current frame. @@ -327,6 +332,7 @@ private unsafe void UpdateTouch(int touchIndex, int pointerIndex, TouchPhase pha [NonSerialized] private int[] m_CurrentDisplayIndices; [NonSerialized] private ButtonControl[] m_Touches; [NonSerialized] private int[] m_TouchIds; + [NonSerialized] private byte[] m_TouchDisplayIndices; [NonSerialized] private int m_LastTouchId; [NonSerialized] private Action m_OnDeviceChange; diff --git a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestStateManager.cs b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestStateManager.cs index 438dab1d3f..184aebf074 100644 --- a/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestStateManager.cs +++ b/Packages/com.unity.inputsystem/Tests/TestFixture/InputTestStateManager.cs @@ -121,6 +121,10 @@ public void Restore() state.inputUserState.RestoreSavedState(); state.touchState.RestoreSavedState(); state.inputActionState.RestoreSavedState(); + // Re-link per-map/per-action back-references that were cleared during + // StaticDisposeCurrentState(). Also recomputes m_EnabledActionsCount + // from the restored action phase memory. See IN-107889. + InputActionState.RelinkRestoredStates(); InputSystemTestHooks.TestHook_RestoreFromSavedState(state.manager, state.remote, state.remoteConnection); InputUpdate.Restore(state.managerState.updateState);