This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Sort the Skia typefaces in a font style set into a consistent order #11056
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When Minikin searches for a font based on a font style, it will score the fonts in the family and choose the best match. However, multiple fonts may have equal scores (e.g. searching for a font with weight 600 when the set includes fonts with weights 500 and 700). In this case Minikin will select the first font in the list with the best score. However, the fonts in a font family's SkFontStyleSet may not always be provided in a consistent order by the SkFontMgr. So if the minikin::FontFamily list is populated based on the SkFontStyleSet order, then a query for a given style might not always return the same font. This change sorts the typefaces in the SkFontStyleSet before converting them into a Minikin font family. Fixes flutter/flutter#31212
GaryQian
reviewed
Aug 16, 2019
| SkFontStyle b_style = b->fontStyle(); | ||
| return (a_style.weight() != b_style.weight()) | ||
| ? a_style.weight() < b_style.weight() | ||
| : a_style.slant() < b_style.slant(); |
Contributor
There was a problem hiding this comment.
Is the slant always different? Is it possible to have a pair of fonts with identical weight and slant?
Member
Author
There was a problem hiding this comment.
Typically a font family would not include multiple fonts with the same weight and slant, but there is no guarantee against that
GaryQian
approved these changes
Aug 16, 2019
Contributor
GaryQian
left a comment
GaryQian
left a comment
There was a problem hiding this comment.
LGTM, up to you if you want to do further comparison for edge cases/adversarial font families
Contributor
|
Oh, could you add a test for this case? |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 16, 2019
engine-flutter-autoroll
added a commit
to flutter/flutter
that referenced
this pull request
Aug 17, 2019
[email protected]:flutter/engine.git/compare/e5f9132b347c...4d5c38e git log e5f9132..4d5c38e --no-merges --oneline 2019-08-16 [email protected] Roll src/third_party/dart a3b579d5c3..2a3b844b41 (5 commits) (flutter/engine#11060) 2019-08-16 [email protected] Roll src/third_party/skia df54f37a5dc1..237a95fe7b28 (2 commits) (flutter/engine#11058) 2019-08-16 [email protected] Roll buildroot to 5a33d6a to pickup changes to toolchain version tracking. (flutter/engine#11061) 2019-08-16 [email protected] Sort the Skia typefaces in a font style set into a consistent order (flutter/engine#11056) 2019-08-16 [email protected] Add ccls config files to .gitignore (flutter/engine#11046) 2019-08-16 [email protected] Pass Android Q insets.systemGestureInsets to Window (flutter/engine#10413) The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff ([email protected]), and stop the roller if necessary.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
When Minikin searches for a font based on a font style, it will score the fonts
in the family and choose the best match. However, multiple fonts may have
equal scores (e.g. searching for a font with weight 600 when the set includes
fonts with weights 500 and 700). In this case Minikin will select the first
font in the list with the best score.
However, the fonts in a font family's SkFontStyleSet may not always be provided
in a consistent order by the SkFontMgr. So if the minikin::FontFamily list is
populated based on the SkFontStyleSet order, then a query for a given style might
not always return the same font.
This change sorts the typefaces in the SkFontStyleSet before converting them
into a Minikin font family.
Fixes flutter/flutter#31212