CHANGE: Touch OnPointerExit now deferred one frame (case 1347048).#1365
Merged
CHANGE: Touch OnPointerExit now deferred one frame (case 1347048).#1365
Conversation
ekcoh
approved these changes
Jul 8, 2021
Collaborator
There was a problem hiding this comment.
Code looks good to me based on what I was able to grasp. The only things that makes me wonder is whether the introduced delay will introduce inconsistencies or might break UI logic and/or tests where touch pointer UI is involved? It sounds like you do not think that would be a problem.
...realize now that its only the exit event that is affected if I understand you correctly and then its likely less critical. E.g. it would be a bit exotic to have logic based on touch exit that is dependent on 1 frame diff I guess.
Contributor
Author
Definitely can't say with certainty that it won't but yup, does seem unlikely. We just space out the event sequence from the same Main risk I see is us picking up the same |
Contributor
Author
Good point. Fixed. |
bryandube
added a commit
that referenced
this pull request
Jul 19, 2021
* Switch default controller for unmatched layouts to XRControllerWithRumble and support '_' in sanitized strings * reverted rumble change * reverted changelog message since change was reverted * FIX: Action reference destroyed in player prefab (case 1319756, #1316). * FIX: Fixing compilation of tests on 2021.1 and short-term remediating docfx issues (#1322) * Added input action asset validation message to PlayerInputManager and editor (#1297) * Fix for tests * Pin windows CI image to an exact version for now (#1323) Authored-by: Dmytro Ivanov <dmytro@unity3d.com> * NEW: Device Simulator plugin for Input System (#1264) * FIX: Events queued during OnUpdate being lost (case 1297339). (#1321) Co-authored-by: Rene Damm <rene@unity3d.com> * CHANGE: InputSystemUIInputModule defaults are set via Reset instead of default script references (fixes 1323566) (#1317) * FIX: Correctly selecting dropdown items when going up through hierarchy (#1309) * NEW: Adding UNITY_INCLUDE_TESTS define constraint to package test assemblies (#1294) * FIX: Fixing Keyboard.current becoming null after OnScreenButton is disabled/destroyed (#1310) * FIX: clickCount not incremented correctly (case 1317239, #1315). * CHANGE: Add event processing limits (#1325) * DOCS: Update LICENSE and CONTRIBUTIONS. * RELEASE: 1.1-pre.4. * FIX: Package validation failure. * FIX: Tests failing from change to -pre. * FIX: InputUser.pairedDevices getting corrupted (case 1327628, #1326). * FIX: Misaligned binding paths when in text mode (case 1200107, #1328). * FIX: Calling InputSystem.Update from inside action callback causes stack overflow (case 1316000) (#1327) * Added InputSystem.runUpdatesInEditMode for XR support (#1319) * Fixing CI (#1357) * FIX: InputTestFixture leaving .current at null (case 1329015, #1345). * FIX: GetBindingDisplyString() doubling up output for composite bindings (case 1321175, #1330). * FIX: Console errors for PlayerInput with empty action maps (case 1317735, #1341). * FIX: Broken script references in Touch Samples (case 1190598, #1339). * FIX: PointerInput in TouchSamples registered too late (case 1215048, #1342). * FIX: MultiTapInteraction not respecting multiTapDelayTime setting (case 1292754, #1337). * MERGE: stable => develop (#1343). * FIX: Looking for wrong control types when rebinding parts of composites (case 1272563, #1338). * FIX: No RMB menu when clicking empty tree views (case 1336426, #1351). * FIX: EnumerateChangedControls skipping PS4 dpad left/right/down (case 1315107, #1336). * FIX: AxisComposite not respecting min/maxValue (case 1335838, #1331). * FIX: Undo not working in input settings (case 1291709, #1332). * FIX: Remove error message when PlayerInput can't find action (case 1259577, #1334). * CHANGE: InputDevice.OnConfigurationChanged now virtual (#1329). * FIX: ArgumentOutOfRangeException in IsPointerOverGameObject (case 1337354, #1356). * FIX: Stuck HoldInteraction when held&released in same event (case 1346786, #1361). * FIX: Incorrect array indexing in InputUser. (#1362) * FIX: Incorrect indexing in InputUser.OnDeviceChanged that could result in incorrect pairing of devices. * FIX: Incorrect indexing when sorting in InputActionRebindingExtensions.RebindingOperation (#1364) * FIX: Incorrect indexing when sorting magnitude based on score in InputActionRebindingExtensions.RebindingOperation. * FIX: 'Add Action' not resetting interactions and processors (#1369). * CHANGE: Touch OnPointerExit now deferred one frame (case 1347048, #1365). * FIX: Setting motor speeds and light bar color in quick succession on DualShock 4 (#1346) * FIX: Setting motor speeds and light bar color in quick succession on DualShock 4 This commit fixes the issue with calling SetMotorSpeeds followed by SetLightBarColor in quick succession on a DualShock 4 controller. The IOCTL interface currently only allows a single command at a time to be processed on the input device, so the second of these calls will usually fail. The fix contained here is simply to expose an extra method on the DualShock 4 interface called SetMotorSpeedsAndLightBarColor that combines both instructions into a single IOCTL command, but a more robust fix will be added later that deals with this issue universally at a lower level in the InputDevice hierarchy. * Add documentation and tweaked changelog * FIX: Callback ordering issues when adding/removing during execution (case 1322530, #1335). * FIX: PlayerInputManager join action not triggering (#1344) * FIX: Submit and cancel UI actions now trigger on press instead of release (#1363) * FIX: InputControlPathDrawer showing duplicate entries (#1366) * FIX: Broken link in CONTRIBUTIONS.md (#1367) * FIX: WebGL gamepad up/down controls were not being inverted (#1370) * Add XboxOne/DualShock layouts for Android (#1255) * ADDED: DualShock4GamepadAndroid and XboxOneGamepadAndroid layout for Android * FIX: Right stick to use AXIS.Z and AXIS.RZ for Android gamepads. * FIX: Fixed triggers to always use Axis.Gas and Axis.Brake for Android gamepads. Co-authored-by: Rene Damm <rene@unity3d.com> Co-authored-by: andrew-oc <78356434+andrew-oc@users.noreply.github.com> Co-authored-by: Andrew O'Connor <andrew.oconnor@unity3d.com> * Updated changelog to match after merge * Revert "Updated changelog to match after merge" This reverts commit f83bc75. * Revert "Merge branch 'develop' into xr-layout-builder-openxr-fixes" This reverts commit 3037847, reversing changes made to b87964c. Co-authored-by: Rene Damm <rene@unity3d.com> Co-authored-by: Dmytro Ivanov <jimon.j1m0n@gmail.com> Co-authored-by: andrew-oc <78356434+andrew-oc@users.noreply.github.com> Co-authored-by: Roman Osipov <31279599+RomanO-Unity@users.noreply.github.com> Co-authored-by: Tom Baird <tomb@unity3d.com> Co-authored-by: David Tetlow <65656964+DTetlow@users.noreply.github.com> Co-authored-by: Håkan Sidenvall <hakan.sidenvall@unity3d.com> Co-authored-by: Brendan Duncan <brendanduncan@gmail.com> Co-authored-by: rytis-buz <52201119+rytis-buz@users.noreply.github.com> Co-authored-by: Andrew O'Connor <andrew.oconnor@unity3d.com>
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
3 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.
Fixes 1347048 (FogBugz).
Description
InputSystemUIInputModulecreates and removes pointers for touch input on the fly. So, finger goes down, pointer is created; finger moves around, pointer is updated; finger is lifted, pointer is removed. This is different from mouse and pen input where a pointer is created for the whole device and just sticks around.This creates a problem with
IsPointerOverGameObject()that relies on a) a pointer being there in the first place and b) it having a state of "over GameObject".Changes made
Pointer removal is now simply deferred one frame. So, the
OnPointerUpevent does happen in the same frame as before but theOnPointerExitnow happens one frame later.Notes
Potential for Regression
We now have pointer states that may "leak" into the next frame. If a platform reuses touch IDs, we may end up picking up a pointer record that before did not exist.
I believe this is not an issue. Even if a pointer record gets reused this way, the event sequence that we surface from the record should be unaffected.
Old Input System
I have not verified how this behaves in the old input system. I believe that in most but not all situations it would work as
StandaloneInputModuleusesUnityEngine.Input.toucheswhich implicitly delays some touch endings by one frame. IIRC this only happens, however, if there's also aMovedin the same frame. In practice, that is likely to almost always be the case but in practice, it seems the same problem may occur with the old input system depending on frame vs input timing.Checklist
Before review:
Changed,Fixed,Addedsections.([case %number%](https://issuetracker.unity3d.com/issues/...)).Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.