Skip to content
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 AChoreographer_registerRefreshRateCallback to get actual refresh rate #93688

Closed
dnfield opened this issue Nov 15, 2021 · 3 comments · Fixed by flutter/engine#29800
Closed
Assignees
Labels
engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list platform-android Android applications specifically waiting for PR to land (fixed) A fix is in flight

Comments

@dnfield
Copy link
Contributor

dnfield commented Nov 15, 2021

Today, the Android embedding calls Display#getRefreshRate once at initialization to get the refresh rate, and then assumes that stays stable when talking to the VSync waiter.

This is actually incorrect, since modern devices can dynamically change refresh rates. However, repeatedly calling getRefreshRate can be slow on some phones (2.5-5ms measured on a Huawei Nova 3), and calling it every frame is expensive.

We should use the AChoreographer_registerRefreshRateCallback where it's available to more efficiently get the FPS rate. This will unblock using the vsync target time for flutter/engine#29727

@blasten @ds84182 FYI

@dnfield dnfield added platform-android Android applications specifically engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list labels Nov 15, 2021
@dnfield dnfield self-assigned this Nov 15, 2021
@blasten
Copy link

blasten commented Nov 15, 2021

makes sense. Do we know how fast AChoreographer_registerRefreshRateCallback dispatches the first callback?

@dnfield
Copy link
Contributor Author

dnfield commented Nov 16, 2021

It's documented as dispatching as soon as you register, unless you manage to unregister before it dispatches :)

I'm playing with an implementation of this on a Pixel 4 and it seems like (for Flutter Gallery at least), the Pixel 4 basically runs it at 90hz ... constantly. I'd like to see a case where this actually varies to make sure this even matters...

chinmaygarde pushed a commit to flutter/engine that referenced this issue Nov 18, 2021
This makes sure the frame timings recorder and vsync waiter implementations get
the correct refresh rate if or when it adjusts at runtime. This should be OK
because we'll only query the refresh rate when the display metrics actually
change, which is much less frequent than every frame. I experimented with an NDK
implementation in the previous patch, but that vastly restricts the API levels
we can support, and currently on API 30 and 31 it just calls Java methods
through JNI anyway.

I've refactored the way Display updates are reported so that AndroidDisplay can
just dynamically get the refresh rate correctly. These values are used by a
service protocol extension and by some Stopwatches on the CompositorContext.

See also #29765

Fixes flutter/flutter#93688
Fixes flutter/flutter#93698
@github-actions
Copy link

github-actions bot commented Dec 2, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
engine flutter/engine repository. See also e: labels. P1 High-priority issues at the top of the work list platform-android Android applications specifically waiting for PR to land (fixed) A fix is in flight
Projects
None yet
3 participants