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

Conversation

jfreire-unity
Copy link
Collaborator

@jfreire-unity jfreire-unity commented Oct 18, 2024

Description

Fix for https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-845

A new pointer was not being created when we released and pressed a finger in the same frame, even though a new touchId was being produced by the native side.

This PR fixes that by looking into pointerIds instead of cached pointer controls. The cached pointer controls wouldn't reflect the control with the correct touchId when we release and press a touch control in the same frame.

Testing status & QA

Please test the bug project with and without this PR on a mobile device (Android or iOS)

For example, to reproduce the issue (more instructions in the bug report):

  1. Press and hold with one finger on the left panel (left panel is green)
  2. Quickly release the finger and press it with another on the right panel
  3. The right will not stay green. This means OnPointerDown wasn't called for this pointer ❌

With the PR, in this example, step 3 should show the right panel green ✅

Overall Product Risks

Low

  • Complexity: Low
  • Halo Effect: Low

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 jfreire-unity force-pushed the isxb-845-fix-pointer-down-same-frame-release-press branch 3 times, most recently from a5fec7f to c645468 Compare October 21, 2024 11:08
@jfreire-unity jfreire-unity changed the title FIX: OnPointerDown is not received when releasing and pressing with different fingers (ISXB-845) FIX: PointerEventData.pointerId is the same when simultaneously releasing and then pressing with another finger (ISXB-845) Oct 21, 2024
Copy link
Collaborator

@ritamerkl ritamerkl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't test it, looks good from what I see

Packages/com.unity.inputsystem/CHANGELOG.md Outdated Show resolved Hide resolved
Assets/Tests/InputSystem/Plugins/UITests.cs Show resolved Hide resolved
scene.leftChildReceiver.events.Clear();

Assert.That(scene.eventSystem.IsPointerOverGameObject(), Is.False);
Assert.That(scene.eventSystem.IsPointerOverGameObject(touchScreen.deviceId), Is.False);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure that this assert is valid touchScreen.deviceId doesn't represent a pointer id.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. It was copied from previous tests

@Pauliusd01 Pauliusd01 requested review from stefanunity and removed request for Pauliusd01 October 22, 2024 07:33
Copy link
Collaborator

@stefanunity stefanunity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked with 22LTS + Pixel 6

@jfreire-unity jfreire-unity requested review from bmalrat and removed request for bmalrat October 24, 2024 13:24
@jfreire-unity jfreire-unity force-pushed the isxb-845-fix-pointer-down-same-frame-release-press branch 2 times, most recently from 0781d9f to d41d05c Compare October 24, 2024 14:19
@jfreire-unity jfreire-unity force-pushed the isxb-845-fix-pointer-down-same-frame-release-press branch from d41d05c to dd3127a Compare October 24, 2024 14:22
@jfreire-unity jfreire-unity merged commit c52fff9 into develop Oct 24, 2024
77 checks passed
@jfreire-unity jfreire-unity deleted the isxb-845-fix-pointer-down-same-frame-release-press branch October 24, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants