Use AChoreographer methods to await vsync when available #31859
Conversation
ColdPaleLight
force-pushed
the
android_vsync_ndk
branch
from
e94f9c9 to
6fde4c4
Compare
ColdPaleLight
changed the title
|
|
||
| //------------------------------------------------------------------------------ | ||
| /// The Android Choreographer is used by `VsyncWaiterAndroid` to await vsync | ||
| /// signal. It's only avialalbe on API 29+ or API 24+ and architecture is 64-bit |
There was a problem hiding this comment.
| /// signal. It's only avialalbe on API 29+ or API 24+ and architecture is 64-bit | |
| /// signal. It's only available on API 29+ or API 24+ if the architecture is 64-bit |
API 29 added support for an explicit 64-bit int (int64_t).
| "AChoreographer_postFrameCallback"); | ||
| } | ||
| #endif | ||
| if (get_instance_fn && post_frame_callback_fn) { |
There was a problem hiding this comment.
is this right?
if !FML_ARCH_CPU_64_BITS and Choreographer_postFrameCallback64 == nullptr, then should_use_ndk_choreographer returns false.
There was a problem hiding this comment.
Yes, we can not use AChoreographer_postFrameCallback in 32-bits app, because in the callback function AChoreographer_frameCallback(long frameTimeNanos, void *data), the type of frameTimeNanos is long, and in 32-bits architecture long is 32bit. And actually the correct frameTimeNanos should be 64 bit. So in 32-bits architecture we can't get the correct frameTimeNanos.
I have tested on two devices and the conclusion is that the 32-bits flutter app does not work correctly if using AChoreographer_postFrameCallback
There was a problem hiding this comment.
I see. I didn't realize this was going to be this complicated.
There was a problem hiding this comment.
I see. I didn't realize this was going to be this complicated.
We can also simplify it, for example , API 29+ use NDK AChoreographer_postFrameCallback64, others use Java fallback. So that we don't need to consider whether the app is 32-bits or 64-bits.
There was a problem hiding this comment.
I think if we are doing this, then making it work on API level >=24 for x64 is better than only >= 29.
| VsyncWaiterAndroid::VsyncWaiterAndroid(flutter::TaskRunners task_runners) | ||
| : VsyncWaiter(std::move(task_runners)) {} | ||
| : VsyncWaiter(std::move(task_runners)), | ||
| should_use_ndk_choreographer_( |
There was a problem hiding this comment.
nit: here and elsewhere, drop should.
| AndroidChoreographer::PostFrameCallback(&OnVsyncFromNDK, weak_this); | ||
| }); | ||
| } else { | ||
| // TODO: Remove the JNI fallback when we drop support for API level < 29. |
There was a problem hiding this comment.
Please file and add a bug here.
ColdPaleLight
added
the
waiting for tree to go green
)" This reverts commit 266d336.
4 participants
Fix flutter/flutter#62166
The main goal of this PR is to make await vsync happen on the UI thread instead of the Platform thread.
We have tried to do it using JNI but got reverted, which lets us know that we cannot use
Choreographerclass in java in non-main thread. #31750However,
AChoreographeris only available at API 24+ and has bugs on 32-bit architecture (can't get the correct timestamp), so we can't make this new feature available to all Android devices. See the table below for detailscc @blasten @chinmaygarde @dnfield @jason-simmons
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.