vsync_waiter_fallback should schedule firecallback for future #28817
Conversation
This makes the impl consistent with how other platforms implement vsyncs. This also ensures that we always pause the microtasks for the minimal time possible. Fixes: flutter/flutter#89786
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
504b876 to
83dfdbd
Compare
| @@ -95,6 +97,8 @@ void VsyncWaiter::ScheduleSecondaryCallback(uintptr_t id, | |||
| void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, | |||
There was a problem hiding this comment.
In the headerdocs for this protected method, can you document that this subclasses must invoke this at (or close to because of scheduling overhead), the frame start time? Also, can you drop a note in the PR commit message that says verification of all vsync waiter subclasses was done to account for the new assertion for the start time requirements in FireCallback.
There was a problem hiding this comment.
Done! will also add the comment when I merge.
| @@ -2,9 +2,14 @@ | |||
| // Use of this source code is governed by a BSD-style license that can be | |||
| // found in the LICENSE file. | |||
|
|
|||
| #define FML_USED_ON_EMBEDDER | |||
There was a problem hiding this comment.
Since this is not in the embedder, it seems odd to lift this gate here. This was done so that custom embedders with no custom event loop integration would continue to function. I think this may not be necessary anymore though.
Anyway, the base fml::TaskRunner has a protected task_runners_ field from you can grab a reference to the task runner you need. No need to use this define to access the current task runner.
…flutter#28817)" This reverts commit edf8fc8.
Prior to flutter#28817 super class vsync waiter would post the tast to the future if the frame start time is at a later poit. This change made it so that the sub classes are responsible for posting the task at the right time. This change makes it so that vsync waiter embedder respects that contract. Fixes flutter/flutter#92603
This makes the impl consistent with how other platforms implement
vsyncs. This also ensures that we always pause the microtasks for the
minimal time possible.
Fixes: flutter/flutter#89786