Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Feb 18, 2021

  • The missing CJK glyphs happened because we failed to download the font as the Noto tree is a superset of glyphs supported by actual fonts. This PR filters out fonts based on actual glyphs they support.
  • The missing symbol glyphs were due to a simple typo in one of the code unit ranges.
  • Unpredictable font precedence issue was due to us registering font fallbacks in order they are downloaded, which is usually random. This PR sorts fonts by URL. It is not clear if prioritizing by URL is the best strategy, but it should be better than random.
  • Fallback reset wasn't resetting everything it should. This PR fixes that too.

Partially fixes flutter/flutter#73628. This still needs @hterkelsen's CanvasKit 0.23 PR to reland.

@google-cla google-cla bot added the cla: yes label Feb 18, 2021
yjbanov added a commit to yjbanov/goldens that referenced this pull request Feb 18, 2021
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits

// TODO(yjbanov): instead of mutating the font tree during reset, it's
// better to construct an immutable tree of resolved fonts
// pointing back to the original NotoFont objects. Then
// resetting the tree would be a matter of reconstrucint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: reconstructing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

await font.ensureResolved();
}

final Set<int> unmatchedCodeUnits = Set<int>.from(coveredCodeUnits);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was confusing initially. maybe comment that it doesn't actually become "unmatchedCodeUnits" until it is passed as an input to findMInimumFontsForCodeUnits

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

if (rangesMap.isEmpty) {
html.window.console.warn('Parses Google Fonts CSS was empty: $css');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Parsed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

yjbanov added a commit to yjbanov/goldens that referenced this pull request Feb 19, 2021
yjbanov added a commit to flutter/goldens that referenced this pull request Feb 19, 2021
* golden updates for flutter/engine#24470

* add internation text samples
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yjbanov
Copy link
Contributor Author

yjbanov commented Feb 19, 2021

LUCI failure is a network flake. Submitting and will keep an eye on LUCI.

@yjbanov yjbanov merged commit 06f0fd9 into flutter:master Feb 19, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 22, 2021
christopherfujino added a commit that referenced this pull request Feb 23, 2021
…24557)

* Update Dart to 2.12.0-259.15.beta

* Propagate image decode errors to the future returned by Codec.getNextFrame (#24336)

* fix infinite loop in findMinimumFontsForCodeunits (#24441)

* Roll CanvasKit to 0.24. (#24498)

* Update tests for new API

* Update goldens and respond to comment

* fix missing CJK and symbol glyphs, font precedence, fallback reset (#24470)

* fix missing CJK and symbol glyphs
* Cache known covered code units

Co-authored-by: Harry Terkelsen <[email protected]>

* Update licenses_golden

Co-authored-by: Jason Simmons <[email protected]>
Co-authored-by: Yegor <[email protected]>
Co-authored-by: Harry Terkelsen <[email protected]>
Co-authored-by: Harry Terkelsen <[email protected]>
hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021
…lutter#24470)

* fix missing CJK and symbol glyphs
* Cache known covered code units

Co-authored-by: Harry Terkelsen <[email protected]>
chriscraws pushed a commit to chriscraws/engine that referenced this pull request Mar 23, 2021
…lutter#24470)

* fix missing CJK and symbol glyphs
* Cache known covered code units

Co-authored-by: Harry Terkelsen <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web : Special characters (ex: Chinese) not rendering on latest master using flutter run -d chrome

3 participants