-
Notifications
You must be signed in to change notification settings - Fork 6k
Optimize loading fallback font-family performance #13257
Optimize loading fallback font-family performance #13257
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
1 similar comment
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
1 similar comment
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
jason-simmons
left a comment
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.
Thank you for your contribution!
| uint32_t FontCollection::sNextId = 0; | ||
|
|
||
| const uint32_t kMinFallbackScore = 0x20000000; | ||
| std::vector<std::shared_ptr<FontFamily>> sFallbackFamilies; |
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.
Make this a member of the FontCollection class instead of a global. The member will need to be mutable because FontCollection::getFamilyForChar is const.
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.
It is a unclear to me exactly why this is called sFallbackFamilies, maybe consider a more descriptive name?
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.
It is a unclear to me exactly why this is called sFallbackFamilies, maybe consider a more descriptive name?
@GaryQian What's your suggested name?
|
|
||
| uint32_t FontCollection::sNextId = 0; | ||
|
|
||
| const uint32_t kMinFallbackScore = 0x20000000; |
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.
Replace the calcFamilyScore >= kMinFallbackScore checks with a call to calcCoverageScore.
calcFamilyScore is a combination of calcCoverageScore and some other scores. If calcCoverageScore returns a nonzero result, then that is equivalent to calcFamilyScore returning a score >= 0x20000000.
| uint32_t langListId, | ||
| int variant) const { | ||
| if (ch >= mMaxChar) { | ||
| for (size_t i = 0; i < sFallbackFamilies.size(); i++) { |
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.
(optional) Since this is identical to the snippet below (line 355), consider using helper function to help keep these two sections in sync.
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.
Also, since it would be helpful to have a short comment explaining why this is necessary.
|
@wangying3426 are you planning to follow-up on the changes requested by @jason-simmons and @GaryQian? |
OK,I will improve it soon. |
If a new fallback font is discovered during paragraph layout, the fallback font cache in txt::FontCollection will use that font in future layouts. However, that cache is not available if the new fallback font needs to be used for other characters within the current layout. This PR adds a cache to minikin::FontCollection and checks whether fonts in the cache can handle a character before calling the fallback font provider. See flutter#13257
If a new fallback font is discovered during paragraph layout, the fallback font cache in txt::FontCollection will use that font in future layouts. However, that cache is not available if the new fallback font needs to be used for other characters within the current layout. This PR adds a cache to minikin::FontCollection and checks whether fonts in the cache can handle a character before calling the fallback font provider. See flutter#13257
If a new fallback font is discovered during paragraph layout, the fallback font cache in txt::FontCollection will use that font in future layouts. However, that cache is not available if the new fallback font needs to be used for other characters within the current layout. This PR adds a cache to minikin::FontCollection and checks whether fonts in the cache can handle a character before calling the fallback font provider. See #13257
|
@jason-simmons has solved this problem, see #14482 , so I will close this PR. |
…ter#14482) If a new fallback font is discovered during paragraph layout, the fallback font cache in txt::FontCollection will use that font in future layouts. However, that cache is not available if the new fallback font needs to be used for other characters within the current layout. This PR adds a cache to minikin::FontCollection and checks whether fonts in the cache can handle a character before calling the fallback font provider. See flutter#13257
We found that it cost a very long time to measure a Chinese paragraph for Text Widget. As the picture shown, the Layout::measureText function takes more than 400ms.
The reason we discovered is that it will search the best-matched font-family in the Layout::measureText function for each Chinese character. If not matched, it will search the fallback font-family, however, this process is time consuming, especially for a very long paragraph.
Therefore, we cached the fallback font-family for reuse, and the same length text only cost 8ms.
At last, here is a case: