-
Notifications
You must be signed in to change notification settings - Fork 510
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
Use proper units when setting preferredFramesPerSecond #572
Conversation
preferredFramesPerSecond expects frames-per-second, rather than the fractional interval expected by frameInterval. For example, frameInterval=2 is the equivalent (on most devices) of preferredFramesPerSecond=30. This issue was introduced in pinterest#552
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing!
Can you address:
And I'll merge? |
@garrettmoon Sure thing. I would like to move the
Rather than doing the calculation up above when it might not be used, but I was concerned about this comment:
I didn't know if I risked creating a deadlock by doing it after referencing the display link. Do you know if that would be safe? |
Ah! Please remove that comment, I believe it copy-pasta'd it from the version I wrote in Texture. It's not relevant here since this class is designed to access the displayLink only on main. |
Perfect! I'll address that now. |
Also remove an obsolete comment warning about a lock (does not apply)
I'm not sure why the |
Seems to be flakey, I'll rerun. |
preferredFramesPerSecond expects frames-per-second, rather than the
fractional interval expected by frameInterval.
For example, frameInterval=2 is the equivalent (on most devices) of
preferredFramesPerSecond=30.
This issue was introduced in #552
It's likely this fixes #570