Skip to content

fix: hard coding frame rate #573

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

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

SamuelBellomo
Copy link
Contributor

Description (*)

setting to 120 so it doesn't influence RTT too much, but still shaves off about 1/3 of CPU usage when running 2 builds on the same machine

We'll need to test this on Android, make sure this doesn't mess things up. @fernando-cortez @jilfranco-unity if one of you guys could test it on your devices, that'd be great :)

… off about 1/3 of CPU usage when running 2 builds on the same machine
@SamuelBellomo SamuelBellomo added 0-URGENT Blocker for a release and needs to be merged ASAP 2-Easy This PR is trivial and can be reviewed quickly 1-Needs Review PR needs attention from the assignee and reviewers GDC-cherrypick labels Mar 17, 2022
Copy link
Collaborator

@fernando-cortez fernando-cortez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on a pixel 3, which has a refresh rate of 60 Hz, and this was playing nicely, mostly since it gets capped at 60.

Per the docs on targetFrameRate, it is usually defaulted to 30 Hz on mobile devices. Pixel 6 will have a 90 Hz refresh rate, but I feel like you'll see diminishing returns with that so high. Capping this to 60 on mobile devices I think should be enough.

@SamuelBellomo SamuelBellomo merged commit 0a3a88c into release/GDC2022 Mar 18, 2022
@SamuelBellomo SamuelBellomo deleted the sam/fix/hard-coding-frame-rate branch March 18, 2022 18:14
SamuelBellomo added a commit that referenced this pull request Mar 18, 2022
…ch, but still shaves off about 1/3 of CPU usage when running 2 builds on the same machine (#573)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0-URGENT Blocker for a release and needs to be merged ASAP 1-Needs Review PR needs attention from the assignee and reviewers 2-Easy This PR is trivial and can be reviewed quickly GDC-cherrypick
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants