Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: PointerEventData.pointerId is the same when simultaneously releasing and then pressing with another finger (ISXB-845) #2033

Merged
200 changes: 195 additions & 5 deletions Assets/Tests/InputSystem/Plugins/UITests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1693,6 +1693,201 @@ public IEnumerator UI_CanDriveUIFromMultipleTouches()
Assert.That(scene.leftChildReceiver.events, Is.Empty);
}

// https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845
// This tests that we can release and press touches on the same frame with the expected events and touchIds.
[UnityTest]
[Category("UI")]
public IEnumerator UI_CanReleaseAndPressTouchesOnSameFrame()
jfreire-unity marked this conversation as resolved.
Show resolved Hide resolved
{
var touchScreen = InputSystem.AddDevice<Touchscreen>();

// Prevent default selection of left object. This means that we will not have to contend with selections at all
// in this test as they are driven from UI objects and not by the input module itself.
var scene = CreateTestUI(noFirstSelected: true);

var asset = ScriptableObject.CreateInstance<InputActionAsset>();
var map = asset.AddActionMap("map");
var pointAction = map.AddAction("point", type: InputActionType.PassThrough, binding: "<Touchscreen>/touch*/position");
var leftClickAction = map.AddAction("leftClick", type: InputActionType.PassThrough, binding: "<Touchscreen>/touch*/press");

scene.uiModule.point = InputActionReference.Create(pointAction);
scene.uiModule.leftClick = InputActionReference.Create(leftClickAction);

map.Enable();

yield return null;

scene.leftChildReceiver.events.Clear();

Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.False);
Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.False);

// Touch left object.
var firstPosition = scene.From640x480ToScreen(100, 100);
BeginTouch(1, firstPosition);
yield return null;

var pointerIdTouch1 = ExtendedPointerEventData.MakePointerIdForTouch(touchScreen.deviceId, 1);
var pointerIdTouch2 = ExtendedPointerEventData.MakePointerIdForTouch(touchScreen.deviceId, 2);
var pointerIdTouch3 = ExtendedPointerEventData.MakePointerIdForTouch(touchScreen.deviceId, 3);

Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.True);
Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.False);

Assert.That(scene.leftChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerEnter).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));
Assert.That(scene.leftChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerDown).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));

Assert.That(scene.rightChildReceiver.events, Is.Empty);
Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(1));

scene.leftChildReceiver.events.Clear();
scene.rightChildReceiver.events.Clear();

// Release left object and Touch right object on the same frame.
var secondPosition = scene.From640x480ToScreen(350, 200);
EndTouch(1, firstPosition);
BeginTouch(2, secondPosition);
BeginTouch(3, secondPosition);
MoveTouch(2, secondPosition);
yield return null;

Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.True);
Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.True);
Assert.That(scene.eventSystem.IsPointerOverGameObject(3), Is.True);

// Pointer 1
Assert.That(scene.leftChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerUp).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));

// Pointer 2
Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerEnter).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerDown).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerMove).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

// Pointer 3
Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerEnter).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerDown).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(3));

scene.leftChildReceiver.events.Clear();
scene.rightChildReceiver.events.Clear();

// End second touch.
EndTouch(2, secondPosition);
EndTouch(3, secondPosition);
yield return null;

Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.True);
Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.True);
Assert.That(scene.eventSystem.IsPointerOverGameObject(3), Is.True);

Assert.That(scene.leftChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerExit).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch1).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == firstPosition));

Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerUp).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerUp).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.uiModule.m_PointerStates.length, Is.EqualTo(2));

scene.leftChildReceiver.events.Clear();
scene.rightChildReceiver.events.Clear();

// Next frame
yield return null;

Assert.That(scene.eventSystem.IsPointerOverGameObject(1), Is.False);
Assert.That(scene.eventSystem.IsPointerOverGameObject(2), Is.False);
Assert.That(scene.eventSystem.IsPointerOverGameObject(3), Is.False);

Assert.That(scene.leftChildReceiver.events, Is.Empty);
Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerExit).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch2).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.rightChildReceiver.events,
Has.Exactly(1).With.Property("type").EqualTo(EventType.PointerExit).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.device == touchScreen).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.touchId == 3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerId == pointerIdTouch3).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.pointerType == UIPointerType.Touch).And
.Matches((UICallbackReceiver.Event e) => e.pointerData.position == secondPosition));

Assert.That(scene.uiModule.m_PointerStates.length, Is.Zero);
}

// https://fogbugz.unity3d.com/f/cases/1190150/
[UnityTest]
[Category("UI")]
Expand Down Expand Up @@ -1724,8 +1919,6 @@ public IEnumerator UI_CanUseTouchSimulationWithUI()

Assert.That(scene.uiModule.m_CurrentPointerType, Is.EqualTo(UIPointerType.Touch));
Assert.That(scene.uiModule.m_PointerIds.length, Is.EqualTo(1));
Assert.That(scene.uiModule.m_PointerTouchControls.length, Is.EqualTo(1));
Assert.That(scene.uiModule.m_PointerTouchControls[0], Is.SameAs(Touchscreen.current.touches[0]));
Assert.That(scene.leftChildReceiver.events,
EventSequence(
AllEvents("pointerType", UIPointerType.Touch),
Expand All @@ -1749,8 +1942,6 @@ public IEnumerator UI_CanUseTouchSimulationWithUI()

Assert.That(scene.uiModule.m_CurrentPointerType, Is.EqualTo(UIPointerType.Touch));
Assert.That(scene.uiModule.m_PointerIds.length, Is.EqualTo(1));
Assert.That(scene.uiModule.m_PointerTouchControls.length, Is.EqualTo(1));
Assert.That(scene.uiModule.m_PointerTouchControls[0], Is.SameAs(Touchscreen.current.touches[0]));
Assert.That(scene.leftChildReceiver.events,
EventSequence(
AllEvents("pointerType", UIPointerType.Touch),
Expand All @@ -1767,7 +1958,6 @@ public IEnumerator UI_CanUseTouchSimulationWithUI()

Assert.That(scene.uiModule.m_CurrentPointerType, Is.EqualTo(UIPointerType.None));
Assert.That(scene.uiModule.m_PointerIds.length, Is.Zero);
Assert.That(scene.uiModule.m_PointerTouchControls.length, Is.Zero);
Assert.That(scene.leftChildReceiver.events,
EventSequence(
AllEvents("pointerType", UIPointerType.Touch),
Expand Down
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ however, it has to be formatted properly to pass verification tests.
### Fixed
- Fixed an issue causing the Action context menu to not show on right click when right clicking an action in the Input Action Editor [ISXB-1134](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1134).
- Fixed `ArgumentNullException: Value cannot be null.` during the migration of Project-wide Input Actions from `InputManager.asset` to `InputSystem_Actions.inputactions` asset which lead do the lost of the configuration [ISXB-1105](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1105)
- Fixed pointerId staying the same when simultaneously releasing and then pressing in the same frame on mobile using touch. [ISXB-1006](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845)

### Changed
- Added back the InputManager to InputSystem project-wide asset migration code with performance improvement (ISX-2086)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1721,26 +1721,9 @@ private int GetPointerStateIndexFor(InputControl control, bool createIfNotExists
////REVIEW: Any way we can cut down on the hops all over memory that we're doing here?
var device = control.device;

////TODO: We're repeatedly inspecting the control setup here. Do this once and only redo it if the control setup changes.

////REVIEW: It seems wrong that we are picking up an input here that is *NOT* reflected in our actions. We just end
//// up reading a touchId control implicitly instead of allowing actions to deliver IDs to us. On the other hand,
//// making that setup explicit in actions may be quite awkward and not nearly as robust.
// Determine the pointer (and touch) ID. We default the pointer ID to the device
// ID of the InputDevice.
var controlParent = control.parent;
var touchControlIndex = m_PointerTouchControls.IndexOfReference(controlParent);
if (touchControlIndex != -1)
{
// For touches, we cache a reference to the control of a pointer so that we don't
// have to continuously do ReadValue() on the touch ID control.
m_CurrentPointerId = m_PointerIds[touchControlIndex];
m_CurrentPointerIndex = touchControlIndex;
m_CurrentPointerType = UIPointerType.Touch;

return touchControlIndex;
}

var pointerId = device.deviceId;
var touchId = 0;
var touchPosition = Vector2.zero;
Expand Down Expand Up @@ -1773,18 +1756,15 @@ private int GetPointerStateIndexFor(InputControl control, bool createIfNotExists
// NOTE: This is a linear search but m_PointerIds is only IDs and the number of concurrent pointers
// should be very low at any one point (in fact, we don't generally expect to have more than one
// which is why we are using InlinedArrays).
if (touchId == 0) // Not necessary for touches; see above.
for (var i = 0; i < m_PointerIds.length; i++)
{
for (var i = 0; i < m_PointerIds.length; i++)
if (m_PointerIds[i] == pointerId)
{
if (m_PointerIds[i] == pointerId)
{
// Existing entry found. Make it the current pointer.
m_CurrentPointerId = pointerId;
m_CurrentPointerIndex = i;
m_CurrentPointerType = m_PointerStates[i].pointerType;
return i;
}
// Existing entry found. Make it the current pointer.
m_CurrentPointerId = pointerId;
m_CurrentPointerIndex = i;
m_CurrentPointerType = m_PointerStates[i].pointerType;
return i;
}
}

Expand Down Expand Up @@ -1930,7 +1910,6 @@ private int AllocatePointer(int pointerId, int displayIndex, int touchId, UIPoin

// Allocate state.
m_PointerIds.AppendWithCapacity(pointerId);
m_PointerTouchControls.AppendWithCapacity(touchControl);
return m_PointerStates.AppendWithCapacity(new PointerModel(eventData));
}

Expand Down Expand Up @@ -1975,7 +1954,6 @@ private void RemovePointerAtIndex(int index)
// Remove. Note that we may change the order of pointers here. This can save us needless copying
// and m_CurrentPointerIndex should be the only index we get around for longer.
m_PointerIds.RemoveAtByMovingTailWithCapacity(index);
m_PointerTouchControls.RemoveAtByMovingTailWithCapacity(index);
m_PointerStates.RemoveAtByMovingTailWithCapacity(index);
Debug.Assert(m_PointerIds.length == m_PointerStates.length, "Pointer ID array should match state array in length");

Expand Down Expand Up @@ -2475,7 +2453,6 @@ private struct InputActionReferenceState
[NonSerialized] private int m_CurrentPointerIndex = -1;
[NonSerialized] internal UIPointerType m_CurrentPointerType = UIPointerType.None;
internal InlinedArray<int> m_PointerIds; // Index in this array maps to index in m_PointerStates. Separated out to make searching more efficient (we do a linear search).
internal InlinedArray<InputControl> m_PointerTouchControls;
internal InlinedArray<PointerModel> m_PointerStates;

// Navigation-type input.
Expand Down