diff --git a/Assets/Tests/InputSystem/CoreTests_Actions.cs b/Assets/Tests/InputSystem/CoreTests_Actions.cs index fa24a4d915..c2479d58ce 100644 --- a/Assets/Tests/InputSystem/CoreTests_Actions.cs +++ b/Assets/Tests/InputSystem/CoreTests_Actions.cs @@ -1401,6 +1401,45 @@ public void Actions_ValueActionsEnabledInOnEvent_DoNotReactToCurrentStateOfContr } } + // Regression test for UUM-100125. + [Test] + [Category("Actions")] + public void Actions_InitialStateCheckAfterConfigurationChange_DoesNotTriggerForInactiveTouch() + { + var touchscreen = InputSystem.AddDevice(); + var action = new InputAction(type: InputActionType.Value, binding: "/primaryTouch/position"); + action.Enable(); + + // Run the first initial state check from enabling the action. + InputSystem.Update(); + + using (var trace = new InputActionTrace(action)) + { + BeginTouch(1, new Vector2(123, 234)); + EndTouch(1, new Vector2(345, 456)); + + Assert.That(touchscreen.primaryTouch.isInProgress, Is.False); + Assert.That(touchscreen.primaryTouch.position.ReadValue(), Is.Not.EqualTo(default(Vector2))); + + trace.Clear(); + + // Configuration change causes full re-resolve and schedules initial state check. + InputSystem.QueueConfigChangeEvent(touchscreen); + InputSystem.Update(); + InputSystem.Update(); + + // Full re-resolve may cancel the current action state. What must NOT happen is a synthetic + // Started/Performed pair from persisted inactive touch state. + Assert.AreEqual(1, trace.count); + foreach (var eventPtr in trace) + { + // The trace should only contain a Canceled event for the action. + Assert.AreEqual(InputActionPhase.Canceled, eventPtr.phase, + $"inactive touch state should not produce action callbacks, but received {eventPtr.phase}."); + } + } + } + // https://fogbugz.unity3d.com/f/cases/1192972/ [Test] [Category("Actions")] @@ -12481,17 +12520,20 @@ public void Actions_WithMultipleCompositeBindings_WithoutEvaluateMagnitude_Works // Now when enabling actionMap .. actionMap.Enable(); - // On the following update we will trigger OnBeforeUpdate which will rise started/performed - // from InputActionState.OnBeforeInitialUpdate as controls are "actuated" + // Inactive touches (ended before action was enabled) must NOT produce started/performed from + // OnBeforeInitialUpdate. Their persisted state (position, touchId) is non-default due to + // dontReset, but only TouchControl.isInProgress should be considered for initial-state check. + // Related to UUM-100125 and Actions_InitialStateCheckAfterConfigurationChange_DoesNotTriggerForInactiveTouch. InputSystem.Update(); - Assert.That(values.Count, Is.EqualTo(prepopulateTouchesBeforeEnablingAction ? 2 : 0)); // started+performed arrive from OnBeforeUpdate + Assert.That(values.Count, Is.EqualTo(0)); values.Clear(); - // Now subsequent touches should not be ignored BeginTouch(200, new Vector2(1, 1)); - Assert.That(values.Count, Is.EqualTo(1)); - Assert.That(values[0].InputId, Is.EqualTo(200)); - Assert.That(values[0].Position, Is.EqualTo(new Vector2(1, 1))); + // If prepopulated, action was never actuated (synthetic initial-check is suppressed), + // so BeginTouch fires started+performed (2 events). + Assert.That(values.Count, Is.EqualTo(prepopulateTouchesBeforeEnablingAction ? 2 : 1)); + Assert.That(values[values.Count - 1].InputId, Is.EqualTo(200)); + Assert.That(values[values.Count - 1].Position, Is.EqualTo(new Vector2(1, 1))); } // FIX: This test is currently checking if shortcut support is enabled by testing that the unwanted behaviour exists. diff --git a/Assets/Tests/InputSystem/Plugins/UITests.cs b/Assets/Tests/InputSystem/Plugins/UITests.cs index 6b246bd5db..5ec540177a 100644 --- a/Assets/Tests/InputSystem/Plugins/UITests.cs +++ b/Assets/Tests/InputSystem/Plugins/UITests.cs @@ -3922,22 +3922,13 @@ public void UI_CanDriveVirtualMouseCursorFromGamepad() // can have a reference to UITK that doesn't break things in previous versions of Unity. [UnityTest] [Category("UI")] - [TestCase(UIPointerBehavior.AllPointersAsIs, ExpectedResult = 1)] - [TestCase(UIPointerBehavior.SingleMouseOrPenButMultiTouchAndTrack, ExpectedResult = 1)] - [TestCase(UIPointerBehavior.SingleUnifiedPointer, ExpectedResult = 1)] -#if UNITY_ANDROID || UNITY_IOS || UNITY_TVOS - [Ignore("Currently fails on the farm but succeeds locally on Note 10+; needs looking into.")] -#endif -#if UNITY_STANDALONE_LINUX || UNITY_EDITOR_LINUX - [Ignore("Disabled to make test suite pass on Linux")] +#if UNITY_2022_3 && (UNITY_ANDROID || UNITY_IOS) + [Ignore("Issue with mouse support on Android and iOS for 2022.3.")] #endif [PrebuildSetup(typeof(UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule_Setup))] - public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule(UIPointerBehavior pointerBehavior) + public IEnumerator UI_UIToolkitInputModule_MouseClick_CapturesAndClicksButton() { var mouse = InputSystem.AddDevice(); - var gamepad = InputSystem.AddDevice(); - var touchscreen = InputSystem.AddDevice(); - var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive)); yield return null; Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected"); @@ -3946,88 +3937,158 @@ public IEnumerator UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule { var objects = scene.GetRootGameObjects(); var uiModule = objects.First(x => x.name == "EventSystem").GetComponent(); - InputSystem.settings.backgroundBehavior = InputSettings.BackgroundBehavior.IgnoreFocus; var uiDocument = objects.First(x => x.name == "UIDocument").GetComponent(); - var uiRoot = uiDocument.rootVisualElement; - var uiButton = uiRoot.Query("Button").First(); - var scrollView = uiRoot.Query("ScrollView").First(); - - uiModule.pointerBehavior = pointerBehavior; + var uiButton = uiDocument.rootVisualElement.Query("Button").First(); var clickReceived = false; uiButton.clicked += () => clickReceived = true; - // NOTE: We do *NOT* do the following as the gamepad submit action will *not* trigger a ClickEvent. - //uiButton.RegisterCallback(_ => clickReceived = true); yield return null; var buttonCenter = new Vector2(uiButton.worldBound.center.x, Screen.height - uiButton.worldBound.center.y); - var buttonOutside = new Vector2(uiButton.worldBound.max.x + 10, Screen.height - uiButton.worldBound.center.y); - var scrollViewCenter = new Vector2(scrollView.worldBound.center.x, Screen.height - scrollView.worldBound.center.y); - Set(mouse.position, buttonCenter, queueEventOnly: true); Press(mouse.leftButton, queueEventOnly: true); - - ////TODO: look at BaseInput and whether we need to override it in order for IME to go through our codepaths - ////TODO: look into or document raycasting aspect (GraphicRaycaster) when using UITK (disable raycaster?) - ////TODO: fix scroll wheel bindings on virtual cursor sample - yield return null; - Assert.That(uiButton.HasMouseCapture(), Is.True, "Expected uiButton to have mouse capture"); Release(mouse.leftButton, queueEventOnly: true); - yield return null; - Assert.That(uiButton.HasMouseCapture(), Is.False, "Expected uiButton to no longer have mouse capture"); - Assert.That(clickReceived, Is.True); + Assert.That(clickReceived, Is.True, "Expected mouse click callback on UITK button"); + } + finally + { + if (mouse.added) + InputSystem.RemoveDevice(mouse); + SceneManager.UnloadSceneAsync(scene); + } + + yield return null; + } + + [UnityTest] + [Category("UI")] + [PrebuildSetup(typeof(UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule_Setup))] + public IEnumerator UI_UIToolkitInputModule_MouseScroll_MovesScrollView() + { + var mouse = InputSystem.AddDevice(); + var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive)); + yield return null; + Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected"); - // Put mouse in upper right corner and scroll down. + try + { + var objects = scene.GetRootGameObjects(); + var uiModule = objects.First(x => x.name == "EventSystem").GetComponent(); + var uiDocument = objects.First(x => x.name == "UIDocument").GetComponent(); + var scrollView = uiDocument.rootVisualElement.Query("ScrollView").First(); + + yield return null; + + var scrollViewCenter = new Vector2(scrollView.worldBound.center.x, Screen.height - scrollView.worldBound.center.y); Assert.That(scrollView.verticalScroller.value, Is.Zero, "Expected verticalScroller to be all the way up"); Set(mouse.position, scrollViewCenter, queueEventOnly: true); yield return null; Set(mouse.scroll, new Vector2(0, -100), queueEventOnly: true); yield return null; - - ////FIXME: as of a time of writing, this line is broken on trunk due to the bug in UITK - // The bug is https://fogbugz.unity3d.com/f/cases/1323488/ - // just adding a define as a safeguard measure to reenable it when trunk goes to next version cycle Assert.That(scrollView.verticalScroller.value, Is.GreaterThan(0)); + } + finally + { + if (mouse.added) + InputSystem.RemoveDevice(mouse); + SceneManager.UnloadSceneAsync(scene); + } - // Try a button press with the gamepad. - // NOTE: The current version of UITK does not focus the button automatically. Fix for that is in the pipe. - // For now focus the button manually. + yield return null; + } + + [UnityTest] + [Category("UI")] + [PrebuildSetup(typeof(UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule_Setup))] + public IEnumerator UI_UIToolkitInputModule_GamepadSubmit_ClicksFocusedButton() + { + var gamepad = InputSystem.AddDevice(); + var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive)); + yield return null; + Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected"); + + try + { + var objects = scene.GetRootGameObjects(); + var uiModule = objects.First(x => x.name == "EventSystem").GetComponent(); + var uiDocument = objects.First(x => x.name == "UIDocument").GetComponent(); + var uiButton = uiDocument.rootVisualElement.Query("Button").First(); + + yield return null; + + var clickReceived = false; + uiButton.clicked += () => clickReceived = true; uiButton.Focus(); - clickReceived = false; + PressAndRelease(gamepad.buttonSouth, queueEventOnly: true); yield return null; + Assert.That(clickReceived, Is.True, "Expected gamepad submit to click focused UITK button"); + } + finally + { + if (gamepad.added) + InputSystem.RemoveDevice(gamepad); + SceneManager.UnloadSceneAsync(scene); + } - Assert.That(clickReceived, Is.True, "Expected to have received click"); + yield return null; + } - ////TODO: tracked device support (not yet supported by UITK) + [UnityTest] + [Category("UI")] + [TestCase(UIPointerBehavior.AllPointersAsIs, ExpectedResult = 1)] + [TestCase(UIPointerBehavior.SingleMouseOrPenButMultiTouchAndTrack, ExpectedResult = 1)] + [TestCase(UIPointerBehavior.SingleUnifiedPointer, ExpectedResult = 1)] +#if UNITY_2022_3 && (UNITY_ANDROID || UNITY_IOS) + [Ignore("Fails on CI for 2022.3 on Android and iOS.")] +#endif + [PrebuildSetup(typeof(UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule_Setup))] + public IEnumerator UI_UIToolkitInputModule_MultiTouchPointerOwnership(UIPointerBehavior pointerBehavior) + { + var touchscreen = InputSystem.AddDevice(); + var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive)); + yield return null; + Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected"); - static bool IsActive(VisualElement ve) - { - return ve.Query().Active().ToList().Contains(ve); - } + try + { + var objects = scene.GetRootGameObjects(); + var uiModule = objects.First(x => x.name == "EventSystem").GetComponent(); + var uiDocument = objects.First(x => x.name == "UIDocument").GetComponent(); + var uiButton = uiDocument.rootVisualElement.Query("Button").First(); - // Move the mouse away from the button to check that touch inputs are also able to activate it. - Set(mouse.position, buttonOutside, queueEventOnly: true); + uiModule.pointerBehavior = pointerBehavior; yield return null; - InputSystem.RemoveDevice(mouse); + + var buttonCenter = new Vector2(uiButton.worldBound.center.x, Screen.height - uiButton.worldBound.center.y); + var buttonOutside = new Vector2(uiButton.worldBound.max.x + 10, Screen.height - uiButton.worldBound.center.y); var uiButtonDownCount = 0; var uiButtonUpCount = 0; - uiButton.RegisterCallback(e => uiButtonDownCount++, TrickleDown.TrickleDown); - uiButton.RegisterCallback(e => uiButtonUpCount++, TrickleDown.TrickleDown); + var uiButtonDownPointerIds = new List(); + var uiButtonUpPointerIds = new List(); + uiButton.RegisterCallback(eventData => + { + uiButtonDownCount++; + uiButtonDownPointerIds.Add(eventData.pointerId); + }, TrickleDown.TrickleDown); + uiButton.RegisterCallback(eventData => + { + uiButtonUpCount++; + uiButtonUpPointerIds.Add(eventData.pointerId); + }, TrickleDown.TrickleDown); - // Case 1369081: Make sure button doesn't get "stuck" in an active state when multiple fingers are used. BeginTouch(1, buttonCenter, screen: touchscreen); yield return null; Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 1"); Assert.That(uiButtonUpCount, Is.EqualTo(0), "Expected uiButtonUpCount to be 0"); - Assert.That(IsActive(uiButton), Is.True, "Expected uiButton to be active"); + Assert.That(uiButtonDownPointerIds, Has.Count.EqualTo(1), "Expected one PointerDown pointerId"); BeginTouch(2, buttonOutside, screen: touchscreen); yield return null; @@ -4038,31 +4099,102 @@ static bool IsActive(VisualElement ve) if (pointerBehavior == UIPointerBehavior.SingleUnifiedPointer) { Assert.That(uiButtonUpCount, Is.EqualTo(1), "Expected uiButtonUpCount to be 1"); - Assert.That(IsActive(uiButton), Is.False, "Expected uiButton to no longer be active"); + Assert.That(uiButtonUpPointerIds, Has.Count.EqualTo(1), "Expected one PointerUp pointerId"); } else { Assert.That(uiButtonUpCount, Is.EqualTo(0), "Expected uiButtonUpCount to be 0"); - Assert.That(IsActive(uiButton), Is.True, "Expected uiButton to be active"); + Assert.That(uiButtonUpPointerIds, Is.Empty, "Expected no PointerUp pointerId from outside touch"); } EndTouch(1, buttonCenter, screen: touchscreen); yield return null; Assert.That(uiButtonDownCount, Is.EqualTo(1), "Expected uiButtonDownCount to be 1"); Assert.That(uiButtonUpCount, Is.EqualTo(1), "Expected uiButtonUpCount to be 1"); - Assert.That(IsActive(uiButton), Is.False, "Expected uiButton to no longer be active"); + Assert.That(uiButtonUpPointerIds, Has.Count.EqualTo(1), "Expected one PointerUp pointerId after releasing touch #1"); + Assert.That(uiButtonUpPointerIds[0], Is.EqualTo(uiButtonDownPointerIds[0]), + "Expected PointerUp ownership to match the pointer that pressed the button"); + Assert.That(IsActiveVisualElement(uiButton), Is.False, "Expected uiButton to no longer be active"); + } + finally + { + if (touchscreen.added) + InputSystem.RemoveDevice(touchscreen); + SceneManager.UnloadSceneAsync(scene); + } + + yield return null; + } - InputSystem.RemoveDevice(touchscreen); + [UnityTest] + [Category("UI")] +#if UNITY_2022_3 && (UNITY_ANDROID || UNITY_IOS) + [Ignore("Issue with mouse support on Android and iOS for 2022.3.")] +#endif + [TestCase(UIPointerBehavior.AllPointersAsIs, ExpectedResult = 1)] + [TestCase(UIPointerBehavior.SingleMouseOrPenButMultiTouchAndTrack, ExpectedResult = 1)] + [TestCase(UIPointerBehavior.SingleUnifiedPointer, ExpectedResult = 1)] + [PrebuildSetup(typeof(UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule_Setup))] + public IEnumerator UI_UIToolkitInputModule_MultiTouchVisualActiveState_FollowsPointerBehavior(UIPointerBehavior pointerBehavior) + { + var touchscreen = InputSystem.AddDevice(); + var scene = SceneManager.LoadScene("UITKTestScene", new LoadSceneParameters(LoadSceneMode.Additive)); + yield return null; + Assert.That(scene.isLoaded, Is.True, "UITKTestScene did not load as expected"); + + try + { + var objects = scene.GetRootGameObjects(); + var uiModule = objects.First(x => x.name == "EventSystem").GetComponent(); + var uiDocument = objects.First(x => x.name == "UIDocument").GetComponent(); + var uiRoot = uiDocument.rootVisualElement; + var uiButton = uiRoot.Query("Button").First(); + + uiModule.pointerBehavior = pointerBehavior; + + yield return null; + + var buttonCenter = new Vector2(uiButton.worldBound.center.x, Screen.height - uiButton.worldBound.center.y); + var buttonOutside = new Vector2(uiButton.worldBound.max.x + 10, Screen.height - uiButton.worldBound.center.y); + + // Finger #1 presses and holds on the button. + BeginTouch(1, buttonCenter, screen: touchscreen); + yield return null; + Assert.That(IsActiveVisualElement(uiButton), Is.True, "Expected uiButton to be active while touch #1 is still pressed."); + + // Finger #2 taps outside of the button while finger #1 is still held. + BeginTouch(2, buttonOutside, screen: touchscreen); + yield return null; + EndTouch(2, buttonOutside, screen: touchscreen); + yield return null; + + // Desired contract: + // - SingleUnifiedPointer: touch #2 can replace current pointer and clear active state. + // - Non-unified behaviors: touch #1 is still pressed and should keep the button visually active. + if (pointerBehavior == UIPointerBehavior.SingleUnifiedPointer) + Assert.That(IsActiveVisualElement(uiButton), Is.False, "Expected uiButton to no longer be active in SingleUnifiedPointer mode."); + else + Assert.That(IsActiveVisualElement(uiButton), Is.True, "Expected uiButton to remain active while touch #1 is still pressed."); + + EndTouch(1, buttonCenter, screen: touchscreen); + yield return null; + Assert.That(IsActiveVisualElement(uiButton), Is.False, "Expected uiButton to no longer be active after touch #1 is released."); } finally { + if (touchscreen.added) + InputSystem.RemoveDevice(touchscreen); SceneManager.UnloadSceneAsync(scene); } - // Wait for unload to complete. yield return null; } + private static bool IsActiveVisualElement(VisualElement visualElement) + { + return visualElement.Query().Active().ToList().Contains(visualElement); + } + private class UI_CanOperateUIToolkitInterface_UsingInputSystemUIInputModule_Setup : IPrebuildSetup { public void Setup() diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 47a24edf7e..2d0cd21b92 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fixed the input actions editor window entering a corrupt state when restarting Unity after the asset was edited. [UUM-134737](https://jira.unity3d.com/browse/UUM-134737) - Fixed `buttonSouth` returning the state of the east button (and so on for all the compass named buttons) when using a Nintendo Switch Pro Controller on iOS [ISXB-1632](issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1632) - Fixed `aButton` returning the state of the east button (and so on for all the letter named buttons) when using a Nintendo Switch Pro Controller on Standalone & iOS [ISXB-1632](issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1632) +- Fixed an issue where `UIToolkit` `ClickEvent` could be fired on Android after device rotation due to inactive touch state being replayed during action initial state checks [UUM-100125](https://jira.unity3d.com/browse/UUM-100125). ### Changed diff --git a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs index dd4cb7d6de..0557f71d1d 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs @@ -1321,6 +1321,9 @@ private void OnBeforeInitialUpdate() if (IsActiveControl(bindingIndex, controlIndex)) continue; + if (ShouldSkipInitialStateCheck(control)) + continue; + if (!control.CheckStateIsAtDefault()) { // Update press times. @@ -1348,6 +1351,22 @@ private void OnBeforeInitialUpdate() k_InputInitialActionStateCheckMarker.End(); } + private static bool ShouldSkipInitialStateCheck(InputControl control) + { + // UUM-100125 + // Touch controls intentionally preserve state such as position even when no touch is currently active. + // During binding re-resolution this can make inactive touches look actuated and cause invalid triggers. + for (var current = control; current != null; current = current.parent) + { + if (current is TouchControl touchControl) + { + return !touchControl.isInProgress; + } + } + + return false; + } + // Called from InputManager when one of our state change monitors has fired. // Tells us the time of the change *according to the state events coming in*. // Also tells us which control of the controls we are binding to triggered the