Reschedule engine frame if it arrives in the middle of warm-up #72115
Conversation
|
Isn't that what _ignoreNextEngineDrawFrame is trying to do? cc @goderbauer |
|
|
cc @goderbauer |
There was a problem hiding this comment.
Why do we need to schedule another frame if one is already scheduled?
There was a problem hiding this comment.
(In other words, why do we ignore _hasScheduledFrame here?)
There was a problem hiding this comment.
From GVC: We should say that we need to reset it here because it was never reseted in _handleBeginFrame when we dropped the frame.
There was a problem hiding this comment.
Actually, after discussing this a little more, this is fine. We have no idea what state _hasScheduledFrame is in, and we need it to be false so that scheduleFrame actually does something.
There was a problem hiding this comment.
Added more explanation.
5684241 to
9e89622
Compare
9e89622 to
d7395ab
Compare
| _rescheduleAfterWarmUpFrame = false; | ||
| // Reschedule in a timer to allow the draw-frame phase of the warm-up | ||
| // frame to finish. | ||
| Timer.run(() { |
There was a problem hiding this comment.
should this be a post-frame callback rather than a timer?
There was a problem hiding this comment.
I was wary of using any method that relies on the frame lifecycle, given that the issue we're dealing here is precisely a violation of said lifecycle 😃 Having said that, I just tried, and a post-frame callback seems to work fine. LMK what your preference is, and whether it has some advantages that I could capture in a test.
There was a problem hiding this comment.
A post-frame callback would make the semantics a little clearer since we want to run this at the end of the currently executing frame (which is the warm-up frame).
| _rescheduleAfterWarmUpFrame = false; | ||
| // Reschedule in a timer to allow the draw-frame phase of the warm-up | ||
| // frame to finish. | ||
| Timer.run(() { |
There was a problem hiding this comment.
A post-frame callback would make the semantics a little clearer since we want to run this at the end of the currently executing frame (which is the warm-up frame).
|
LGTM too. Please keep an eye out for regressions in the days after landing this (e.g. trawl through the recently-filed issues for unexplained weirdness, look out for failures in google's tests), even more so than usual, because this is a subtle and core part of the system. |
|
(e.g. look for regressions around when the splash screen comes down on android apps) |
Rgr. Hopefully this is a no-op on mobile, since on the VM we never get a frame in the middle of a warm-up frame and therefore the new codepath never executes. |
"Hopefully", as in "I'm counting on it". If it's otherwise, I'll revert immediately as it would mean I probably messed it up big time. |
|
Actually, now that I think about it, if in a rare occasion the mobile does get a frame in the middle of the warm-up frame it may be a source of flakiness in apps/tests. So this, theoretically, could fix the flakiness. |
Description
Reschedule engine frame if it arrives in the middle of warm-up.
Related Issues
#69313
Tests
I added the following tests: added a new test in
scheduler_test.dart.