-
Notifications
You must be signed in to change notification settings - Fork 329
FIX: Reset ButtonStates when re-using eventData with SingleUnifiedPointer #2139
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: Reset ButtonStates when re-using eventData with SingleUnifiedPointer #2139
Conversation
Pauliusd01
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried to reproduce the original issue for quite a while and wasn't able to with these changes. Also, checked our touch samples for any other potential issues and did not notice any. If there's concerns with this change maybe it would be a good idea to add someone from the UI team as well?
ekcoh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, am I correct this wasn't covered by any automated test? Would it be possible to add a test triggering the problem to avoid future regression in this area?
@ekcoh The original reported issue is so intermittent and difficult to test that I'm not quite sure what an automated test would look like. I believe there is a timing element to whether the button is being checked before/after being updated and this ensures that it's not considered 'pressed' before being fully updated. |
Description
When using SingleUnifiedPointer only a single pointer state is ever created and it is re-used for all incoming pointer input. As this is updated with the new event data we need to ensure that the ButtonState's FramePressedState is not left in any specific state of press as this is no longer relevant to the new input data.
Testing status & QA
Tested against repro project on Google Pixel 6. Original bug is stated as difficult to replicate - it doesn't happen often without the fix and I have yet see the issue again with the fix.
Run against All Tests for Input System as there is potential for issues with UI Tests.
Requesting further testing on UI particularly.
Overall Product Risks
Comments to reviewers
Please describe any additional information such as what to focus on, or historical info for the reviewers.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.After merge: