FIX: Add test and fix for multiple touchscreen input bug.#2434
Open
Darren-Kelly-Unity wants to merge 2 commits into
Open
FIX: Add test and fix for multiple touchscreen input bug.#2434Darren-Kelly-Unity wants to merge 2 commits into
Darren-Kelly-Unity wants to merge 2 commits into
Conversation
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #2434 +/- ##
============================================
+ Coverage 58.58% 79.01% +20.43%
============================================
Files 738 766 +28
Lines 135831 140369 +4538
============================================
+ Hits 79570 110917 +31347
+ Misses 56261 29452 -26809 Flags with carried forward coverage won't be shown. Click here to find out more.
|
Darren-Kelly-Unity
changed the title
Collaborator
|
/test_plan |
Contributor
Test Plan
Summary of Changes & Risk AssessmentSummary of ChangesThis PR fixes a bug (IN-108611) where touch events from different physical touchscreens could conflict if they shared the same Risk Assessment
Test ScenariosFunctional Testing
Regression Testing
🔍 Regression Deep Dive (additional risks identified)
Edge Cases
💡 This test plan updates automatically when 🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr |
Collaborator
|
I have no access to two pc touchscreens so I will look for someone who could help. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of this PR
Fixes a bug (IN-108611) where an ongoing touch on one physical touchscreen could be incorrectly updated by touch events originating from a second physical touchscreen, when both screens reported the same
touchId. The root cause was that three touch-matching code paths inTouchscreen.csidentified touches bytouchIdalone, ignoringdisplayIndex.Release Notes
[IN-108611] Input System — Bug Fixes
touchId.Functional Testing status
Devices_TouchMoveOnSecondDisplay_DoesNotUpdateTouchOnFirstDisplayadded toTouchscreenMultiDisplayTests.cs, covering the exact failure sequence (twoBeganevents with the sametouchIdon differentdisplayIndexvalues, followed by aMovedfrom one screen verifying the other is untouched).Performance Testing Status
No performance impact. The three changed comparisons each add a single byte comparison (
displayIndex) to an existing conditional — no additional allocations, iterations, or data structure changes.Overall Product Risks
Technical Risk: 1 — Self-contained fix to three closely related conditions in one file. Behaviour change is additive (stricter matching); single-screen setups are entirely unaffected since
displayIndexwill always match itself.Halo Effect: 1 — Could affect any code path relying on
Touchscreenwith multi-display touch input, but the existing behaviour in those paths was already incorrect.Class diagram
Changes analysed from
origin/develop...HEADClass diagram
classDiagram class Touchscreen { «modified» OnStateEvent() «modified» GetCurrentValueAsObject() «modified» MergeForward() }