Perf: Don't do raycasts for the GazeProvider if it's behavior is set …#10340
Perf: Don't do raycasts for the GazeProvider if it's behavior is set …#10340RogPodge merged 10 commits into
Conversation
jverral
requested review from
david-c-kline,
julenka and
keveleigh
as code owners
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| @@ -578,7 +578,7 @@ private void UpdateGazeProvider() | |||
| { | |||
| // The gaze hit result may be populated from the UpdatePointers call. If it has not, then perform | |||
There was a problem hiding this comment.
This is an interesting one. This second pass was introduced to ensure an app could always query what the head/eyes were targeting regardless of if the MRTK input system would send events over it. This change would be a behavioral breaking change if apps are relying on this info while setting the gaze pointer to always be off.
On the other hand...setting the gaze pointer to be always off could imply that it will never be used for anything. Maybe we can take this change and heavily document the meaning.
There was a problem hiding this comment.
Building on this comment: we specifically have tests that ensure the assertion I made above is true, which this PR is failing:
There was a problem hiding this comment.
Seems like maybe the gaze target should be something lazy evaluated, especially if the behavior is set to AlwaysOff.
|
Perhaps add an optional "dont raycast if gaze pointer is off" option. Also, @jverral, how much time does this save? |
…vider, also included in-editor ways to more obviously do this
|
Made some changes to better indicate the way we intend users to disable gaze provider raycasts, i.e. directly disabling the gaze provider component. The inspector changes + updated doc comments should be sufficient for addressing this issue |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| { | ||
| using (UpdateGazeProviderPerfMarker.Auto()) | ||
| { | ||
| bool gazeProviderEnabled = (CoreServices.InputSystem.GazeProvider?.Enabled).GetValueOrDefault(false); |
There was a problem hiding this comment.
Do you think there's anywhere reasonable to cache the input system or the gaze provider in this class? And maybe update the cache if something goes null, but otherwise I'm not sure we need to re-query every frame. Could get expensive 😬
There was a problem hiding this comment.
Cached it as much as possible without storing it at top level, since it doesn't look like we follow that pattern anywhere else in code.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
4 participants
…to off.
Overview
Was noting some perf hits in the profiler in our project w/ Gaze...when we set the behavior to AlwaysOff.
Changes
Verification