Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add scheduleWarmUpFrame #50570
Add scheduleWarmUpFrame #50570
Changes from 12 commits
1950e72
f55dfdf
7f76c5f
31a5ea3
590287d
40b269b
2d70a0f
4af9f06
bfe5570
21439c4
1bd21db
4f41658
eeac064
9c5bce4
cdcf71a
7ab7d8e
afbcfae
7381087
05d3d2d
06957bb
68953c0
a9b7511
8731e47
cdeffd9
5d9f48b
1589c59
556984f
2b6d3b2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also want to use this when rendering the first frame in a new
FlutterView
? I imagine we'd want the new view to be rendered as fast as possible too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point, although I imagine this would require more work. Can I open an issue to leave the optimization for later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this optimization works for individual views. Because of the unified widget tree we can only produce frames for all views at once and cannot target individual views. This optimization only works for the app as a whole.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, I was just hypothesizing. If we don't need it, then we don't need it. My gut feeling is that we don't need any of this on the web. So I'm actually a little sad that this API even exists.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is meant for first frame only, why not call it
scheduleFirstFrame
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The biggest reason, in my opinion, is that this frame is not guaranteed to be rendered, for example if
Animator::BeginFrame
is not called before, or on Web (if you agree with my other comment). Therefore this might not be the first frame.Besides, I don't want to give developers the impression that scheduling the first frame using
scheduleFrame
is wrong. ThescheduleWarmUpFrame
really is for warming up. An app doesn't need warm-ups to perform correctly, but will perform better. It's probably the same reason whySchedulerBinding.scheduleWarmUpFrame
is not calledscheduleFirstFrame
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to what Tong said: The warmup frame is also used in hot reload scenarios where it isn't the first frame of the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The semantics of this method sound pretty complicated. I guess warm up is fine then. Should we have some assertions in the engine for this method? For example, should we make sure it's only called for the first frame, and is never called after a
scheduleFrame
is called? Simply exposing this as a sibling toscheduleFrame
seems to contain footguns.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's possible, since
SchedulerBinding.scheduleWarmUpFrame
is also called right after hot reloading. I think it's better leave it to the app, i.e. the framework.