search

Unity-Technologies / InputSystem

An efficient and versatile input system for Unity.
Other
1.42k stars 306 forks source link

FIX: Add workaround for cases where button presses can be missed (ISXB-491) #1935

Closed graham-huws closed 2 months ago

graham-huws commented 3 months ago

Description

Fix for ISXB-491. The current design of the input system package relies on the following method for checking if a button has been pressed on the current frame: Check the current value of the button, and compare to the final value of the previous frame. The issue with this approach is that presses can be missed if they happened very quickly, or there was some form of lag making the update take longer than intended. Whether a button is pressed is only properly evaluated when asked, so if we don't check every input coming in, some can be missed.

Changes made

This PR seeks to solve this in a very simplistic way - check every value that affects ButtonControls and record their pressed state at the time the change comes in. Current belief is that a "clever" solution to this issue would require a fairly extensive rewrite of how the input system works. Performance comparison data can be viewed on Sheet 2 here.

Checklist

Before review:

During merge:

lyndon-unity commented 3 months ago

Added Paulius to help testing performance on Android

Pauliusd01 commented 3 months ago

The original bug project crashes when built on my only Android testing phone (Xiaomi MI 8 Lite) so I used the QA mobile sensor testing project instead when idling and when interacting with the UI buttons. Not sure what to make of the results so I'm just gonna post the screenshot with analyzer data as well as profiler captures. Capture data

analyzer screenshots

(In the screenshot blue highlighted area is for simply idling the app and the right green area is me pressing UI buttons every 0.5~ sec)

lyndon-unity commented 3 months ago

These captures don't show any significant slow down so I think this is good to land (pending reacting to feedback above in PR)

lyndon-unity commented 3 months ago
Screenshot 2024-06-03 at 16 39 28

There is a slowdown on some frames for the PreUpdate.NewInputUpdate. 0.20 to 0.34 on one frame Only 2 frames show slow down more and 0.1

Screenshot 2024-06-03 at 16 41 14

InputProcess has some slow down but its minimal 0.03 to 0.04

How many taps were made in this sample ?

Pauliusd01 commented 3 months ago

For the past tapping captures I just profiled for 15~ secs and tapped twice every second, the number of taps could be different from both versions. Here's a brand new capture of me idling for 5 secs, tapping once -> idling for 5 secs more and tapping again. Both of these should now just be 2 taps each

image ExtraCaptures.zip

I used this project: https://github.cds.internal.unity3d.com/unity/InputSystem-TestProject

Pauliusd01 commented 3 months ago

Here's one of me just tapping non stop for around 15 secs

image ExtraCaptures2.zip

lyndon-unity commented 3 months ago

Here's one of me just tapping non stop for around 15 secs

Screenshot 2024-06-04 at 13 30 08

Sorting by time and looking at the slowest 100 frames (when there is active tap interation) there is a small slowdown in InputUpdate 0.01ms (0.06->0.07ms). But this seems reasonable for the reliability improvement of the edge detection from the fix.

graham-huws commented 3 months ago

Have updated to address above comments, plus some extra issues that cropped up. Have added new perf results to the second sheet of the gdoc.

ekcoh commented 2 months ago

@lyndon-unity Could you help summarise what additional updates are needed to transition this from "Request Changes"?

graham-huws commented 2 months ago

Now updated to a new version which only does processing for ButtonControls which have had wasPressedThisFrame/wasReleasedThisFrame called, saving a lot of performance when not many are being utilised.

Added V2 benchmarks to the spreadsheet, and new tests which try varying amounts of buttons to test to see the impact.