FIX: An infinite loop in InputSystemUIInputModule was causing an editor / player hang

[Secticide]

Description

Fixes ISXB-1258

This was caused by the assumption that RemovePointerAtIndex would always successfully remove the pointer, which is not the case with touch based pointers.

The following code highlights the issue:

private void PurgeStalePointers()
{
    for (var i = 0; i < m_PointerStates.length; ++i)
    {
        ref var state = ref GetPointerStateForIndex(i);
        var device = state.eventData.device;
        if (!device.added || // Check if device was removed altogether.
            (!HaveControlForDevice(device, point) &&
             !HaveControlForDevice(device, trackedDevicePosition) &&
             !HaveControlForDevice(device, trackedDeviceOrientation)))
        {
            SendPointerExitEventsAndRemovePointer(i);
            --i;
        }
    }

If SendPointerExitEventsAndRemovePointer fails to remove the pointer from the list; decrementing i is incorrect and will lead to attempting to remove the pointer at the same index again, which will fail and we'll loop ad infinitum.

Testing status & QA

I've tested this locally on a Windows 11 touch-enabled machine (laptop), but it would be great to get some testing across mobile devices (including Switch) to ensure no additional issues were introduced.

Overall Product Risks

• Complexity: 0
• Halo Effect: 1

Comments to reviewers

The only remaining concern I have with this solution is with the OnDisable code. The first thing we call in OnDisable for the Input system UI module is ResetPointers - how important is it that the pointers are all 100% reset here? If it is an absolute must, we could always add a force flag as a parameter to the SendPointerExitEventsAndRemovePointer method.

Checklist

Before review:

• Changelog entry added.
  • Explains the change in Changed, Fixed, Added sections.
  • For API change contains an example snippet and/or migration example.
  • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
  • Jira port for the next release set as "Resolved".
• Tests added/changed, if applicable.
  • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
  • Performance tests.
  • Integration tests.
• Docs for new/changed API's.
  • Xmldoc cross references are set correctly.
  • Added explanation how the API works.
  • Usage code examples added.
  • The manual is updated, if needed.

During merge:

• Commit message for squash-merge is prefixed with one of the list:
  • NEW: ___.
  • FIX: ___.
  • DOCS: ___.
  • CHANGE: ___.
  • RELEASE: 1.1.0-preview.3.

After merge:

• Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

[jfreire-unity]

Good catch! I can have a look today or Monday 👍

[jfreire-unity]

Looks ok, good find!
Just added a question before approving. Also added QA to test this and make sure Touch functionality has no regression.

[Pauliusd01]

Hi @MahumKhan , could you help with checking the switch platform?

[stefanunity]

Helped with some testing:
Can reproduce the bug and it looks fixed with this PR.
TouchSamples project also still works.

Tested on:

• Win11 touchscreen
• Pixel 6a
• Unity 22.3 + 6.0

[Pauliusd01]

LGTM, checked touch + PS5/Xbox input on the user repro project as well as the internal Touch Event/Sensor project on Mi 8 Lite, iPhone 8 and did not notice any unexpected issues or crashes

[cristian-murarescu]

Tested this on Switch with and without the fix, can confirm that without installing this fixed package, the project freezes. With it installed, it doesn't freeze.

Tested by spamming A and touching the white square on the screen repeatedly.