Defer setup of the default font manager if the embedding prefetched the font manager #28987
Conversation
|
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. |
| engine->SetupDefaultFontManager(); | ||
| } | ||
| }); | ||
| if (!settings_.prefetched_default_font_manager) { |
There was a problem hiding this comment.
nit: consider updating the comment above to explain the guard, and where we do a last minute check to make sure it's set up.
Also consider documenting that we do it this way because it's guarded by SkOnce, so we can be confident a second call will be very quick as long as the first call has finished.
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
Change looks good, needs a test.
Perhaps something like a ShellTest that passes in the new setting and verifies that the SkFontMgr has not been set up after creating the shell, but that it is created after running the engine?
It'd be interesting to see whether this helps our existing time to first frame benchmark in the framework too. I'm not sure if it will because the moto g4 has more cores than the internal device I'm testing on.
…he font manager SkFontMgr_Android can take a long time to initialize. On Android, the embedding will call FlutterJNI.nativePrefetchDefaultFontManager early during application startup, which will construct the global default SkFontMgr on a background thread. SetupDefaultFontManager will need to wait until the prefetch has completed in order to obtain the default SkFontMgr. If the embedding has done a prefetch, then this PR moves SetupDefaultFontManager to a later phase of engine setup to make waiting less likely. See flutter/flutter#91045
|
Added a test - PTAL |
jason-simmons
force-pushed
the
bug_91045_b
branch
from
437772c to
8340bfb
Compare
jason-simmons
added
the
waiting for tree to go green
…etched the font manager (flutter/engine#28987)
…etched the font manager (flutter/engine#28987)
…etched the font manager (flutter/engine#28987)
…etched the font manager (flutter/engine#28987)
…etched the font manager (flutter/engine#28987)
…etched the font manager (flutter/engine#28987)
3 participants
SkFontMgr_Android can take a long time to initialize. On Android,
the embedding will call FlutterJNI.nativePrefetchDefaultFontManager
early during application startup, which will construct the global
default SkFontMgr on a background thread.
SetupDefaultFontManager will need to wait until the prefetch has
completed in order to obtain the default SkFontMgr. If the embedding
has done a prefetch, then this PR moves SetupDefaultFontManager to a
later phase of engine setup to make waiting less likely.
See flutter/flutter#91045