Start animator paused #29007
Start animator paused #29007
Conversation
dnfield
requested review from
chinmaygarde,
iskakaushik,
gw280 and
jason-simmons
| @@ -73,6 +73,9 @@ TEST_F(ShellTest, VSyncTargetTime) { | |||
| fml::TaskRunner::RunNowOrPostTask(shell->GetTaskRunners().GetUITaskRunner(), | |||
| [engine = shell->GetEngine()]() { | |||
| if (engine) { | |||
| // Engine needs a surface for frames to | |||
| // be scheduled. | |||
| engine->OnOutputSurfaceCreated(); | |||
There was a problem hiding this comment.
This is a slight change in behavior here that broke this test - before, the test never created a surface so it never actually scheduled frames and the call below this was a no-op. Now, there's an implicit call to RequestFrame(false) in this call. @iskakaushik
shell/common/animator.h
Outdated
| @@ -106,15 +106,15 @@ class Animator final { | |||
|
|
|||
| std::unique_ptr<FrameTimingsRecorder> frame_timings_recorder_; | |||
| uint64_t frame_request_number_ = 1; | |||
| int64_t dart_frame_deadline_; | |||
| int64_t dart_frame_deadline_ = 0; | |||
There was a problem hiding this comment.
Can we convert this to use time points in FML instead? I can't tell what unit the deadline is supposed to be in.
I know this wasn't the point of this patch but moving the initializers to the header seems to be a drive-by change. So, perhaps you are interested in cleaning this little but up as well. Fine if you'd rather not.
There was a problem hiding this comment.
done - would you mind taking a look to make sure what I did looks reasonable?
dnfield
added
waiting for tree to go green
|
This still has a problem - because of the logic in engine/shell/common/animator.cc Line 235 in e725a92 |
|
I am wrong. I was looking at a stale trace file. This patch substantially improves internal customer's startup times. |
|
There is still something not quite right about this patch. Going to mark it as a draft until I figure out exactly what. |
|
Ok. The problem is that we have multiple places calling I think this patch makes sense to land. I will look at separately landing a patch to make the engine avoid responding to |
dnfield
added
the
waiting for tree to go green
Do not call Animator::Delegate::OnAnimatorNotifyIdle until at least one frame has been rendered. (flutter#29015)
Do not call Animator::Delegate::OnAnimatorNotifyIdle until at least one frame has been rendered. (flutter#29015) flutter/flutter#91209
This reverts commit 666994f.
Starts the animator in a paused state, and only starts it once we get a request to schedule frame and we have a surface to render to.
This avoids the animator doing unnecessary work during application startup, and in particular avoids it potentially signaling that we have idle time for a GC when we haven't even rendered a frame yet.
Part of flutter/flutter#91209