Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[web] Split all 1MB+ fallback fonts (including CJK) #56388

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+43,843 −23,087
Open

[web] Split all 1MB+ fallback fonts (including CJK) #56388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4792ba
[web] Split all 1MB+ fallback fonts (including CJK)
mdebbar Nov 5, 2024
370df40
roll fonts
mdebbar Nov 5, 2024
32d2ca7
address review comments
mdebbar Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ deps = {
'packages': [
{
'package': 'flutter/flutter_font_fallbacks',
'version': '10da6a95fedad127634500aa854466fe9e3fa760220a2a1c7c20df84073fce76'
'version': '8a753fd2150c398a5777a7fdb24fc9d4d5fe8015088c5237b61cf0ff26653fd0'
}
],
'dep_type': 'cipd',
Expand Down
84 changes: 50 additions & 34 deletions lib/web_ui/dev/roll_fallback_fonts.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ class RollFallbackFontsCommand extends Command<bool>
...await _processSplitFallbackFonts(client, splitFallbackFonts),
...await _processFallbackFonts(client, apiFallbackFonts),
];

final String? failedUrl = await _checkForLicenseAttributions(client, fallbackFontInfo);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe return a List<String> or Set<String> with all the fonts that might have failed, and then do if (failedUrls.isNotEmpty) to throw? That way we can report all the fonts that have a problem at once.

if (failedUrl != null) {
throw ToolExit('Could not find license attribution at: $failedUrl');
}

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: unnecessary empty line

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

final List<_Font> fallbackFontData = <_Font>[];

final Map<String, String> charsetForFamily = <String, String>{};
Expand All @@ -79,19 +85,11 @@ class RollFallbackFontsCommand extends Command<bool>
if (fontResponse.statusCode != 200) {
throw ToolExit('Failed to download font for $family');
}
final String urlString = uri.toString();
if (!urlString.startsWith(expectedUrlPrefix)) {
throw ToolExit('Unexpected url format received from Google Fonts API: $urlString.');
}
final String urlSuffix = urlString.substring(expectedUrlPrefix.length);
final String urlSuffix = getUrlSuffix(uri);
final io.File fontFile =
io.File(path.join(fontDir.path, urlSuffix));

final Uint8List bodyBytes = fontResponse.bodyBytes;
if (!await _checkForLicenseAttribution(client, urlSuffix, bodyBytes)) {
throw ToolExit(
'Expected license attribution not found in file: $urlString');
}
hasher.add(utf8.encode(urlSuffix));
hasher.add(bodyBytes);

Expand Down Expand Up @@ -144,12 +142,7 @@ class RollFallbackFontsCommand extends Command<bool>

for (final _Font font in fallbackFontData) {
final String family = font.info.family;
final String urlString = font.info.uri.toString();
if (!urlString.startsWith(expectedUrlPrefix)) {
throw ToolExit(
'Unexpected url format received from Google Fonts API: $urlString.');
}
final String urlSuffix = urlString.substring(expectedUrlPrefix.length);
final String urlSuffix = getUrlSuffix(font.info.uri);
sb.writeln(" NotoFont('$family', '$urlSuffix'),");
}
sb.writeln('];');
Expand Down Expand Up @@ -385,7 +378,6 @@ const List<String> apiFallbackFonts = <String>[
'Noto Sans',
'Noto Music',
'Noto Sans Symbols',
'Noto Sans Symbols 2',
'Noto Sans Adlam',
'Noto Sans Anatolian Hieroglyphs',
'Noto Sans Arabic',
Expand All @@ -407,12 +399,9 @@ const List<String> apiFallbackFonts = <String>[
'Noto Sans Cham',
'Noto Sans Cherokee',
'Noto Sans Coptic',
'Noto Sans Cuneiform',
'Noto Sans Cypriot',
'Noto Sans Deseret',
'Noto Sans Devanagari',
'Noto Sans Duployan',
'Noto Sans Egyptian Hieroglyphs',
'Noto Sans Elbasan',
'Noto Sans Elymaic',
'Noto Sans Ethiopic',
Expand All @@ -423,17 +412,14 @@ const List<String> apiFallbackFonts = <String>[
'Noto Sans Gujarati',
'Noto Sans Gunjala Gondi',
'Noto Sans Gurmukhi',
'Noto Sans HK',
'Noto Sans Hanunoo',
'Noto Sans Hatran',
'Noto Sans Hebrew',
'Noto Sans Imperial Aramaic',
'Noto Sans Indic Siyaq Numbers',
'Noto Sans Inscriptional Pahlavi',
'Noto Sans Inscriptional Parthian',
'Noto Sans JP',
'Noto Sans Javanese',
'Noto Sans KR',
'Noto Sans Kaithi',
'Noto Sans Kannada',
'Noto Sans Kayah Li',
Expand Down Expand Up @@ -492,7 +478,6 @@ const List<String> apiFallbackFonts = <String>[
'Noto Sans Psalter Pahlavi',
'Noto Sans Rejang',
'Noto Sans Runic',
'Noto Sans SC',
'Noto Sans Saurashtra',
'Noto Sans Sharada',
'Noto Sans Shavian',
Expand All @@ -504,7 +489,6 @@ const List<String> apiFallbackFonts = <String>[
'Noto Sans Sundanese',
'Noto Sans Syloti Nagri',
'Noto Sans Syriac',
'Noto Sans TC',
'Noto Sans Tagalog',
'Noto Sans Tagbanwa',
'Noto Sans Tai Le',
Expand All @@ -531,27 +515,59 @@ const List<String> apiFallbackFonts = <String>[
/// handling.
const List<String> splitFallbackFonts = <String>[
'Noto Color Emoji',
'Noto Sans Symbols 2',
'Noto Sans Cuneiform',
'Noto Sans Duployan',
'Noto Sans Egyptian Hieroglyphs',
Comment on lines +519 to +521
Copy link
Member

Choose a reason for hiding this comment

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

If we don't want to bloat the main file in the general case... Have we thought of trimming the list of fallback fonts to "scripts currently in use today", with an opt-in to the full set? (Maybe vending 2 different versions of the font metadata file?)

'Noto Sans HK',
'Noto Sans JP',
'Noto Sans KR',
'Noto Sans SC',
'Noto Sans TC',
];

Future<bool> _checkForLicenseAttribution(
String getUrlSuffix(Uri fontUri) {
final String urlString = fontUri.toString();
if (!urlString.startsWith(expectedUrlPrefix)) {
throw ToolExit('Unexpected url format received from Google Fonts API: $urlString.');
}
return urlString.substring(expectedUrlPrefix.length);
Comment on lines +530 to +534
Copy link
Member

@ditman ditman Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This should could be a regex :P (now we have two problems!)

}

/// Checks the license attribution for unique fonts in the [fallbackFontInfo]
/// list.
///
/// When the license check fails, it returns the URL of the license file that
/// failed the check. When the check passes, it returns `null`.
Future<String?> _checkForLicenseAttributions(
Copy link
Contributor

Choose a reason for hiding this comment

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

document that this method returns a non-null String with the non-conforming license URL if the license fails the check

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

http.Client client,
String urlSuffix,
Uint8List fontBytes,
List<_FontInfo> fallbackFontInfo,
) async {
const String googleFontsUpstream =
'https://github.com/google/fonts/tree/main/ofl';
const String attributionString =
'This Font Software is licensed under the SIL Open Font License, Version 1.1.';

final String fontPackageName = urlSuffix.split('/').first;
final String fontLicenseUrl = '$googleFontsUpstream/$fontPackageName/OFL.txt';
final http.Response response = await client.get(Uri.parse(fontLicenseUrl));
if (response.statusCode != 200) {
throw ToolExit('Failed to download `$fontPackageName` license at $fontLicenseUrl');
final uniqueFontPackageNames = <String>{};
for (final (family: _, :uri) in fallbackFontInfo) {
final String urlSuffix = getUrlSuffix(uri);
final String fontPackageName = urlSuffix.split('/').first;
uniqueFontPackageNames.add(fontPackageName);
}

for (final String fontPackageName in uniqueFontPackageNames) {
final String fontLicenseUrl = '$googleFontsUpstream/$fontPackageName/OFL.txt';
final http.Response response = await client.get(Uri.parse(fontLicenseUrl));
if (response.statusCode != 200) {
return fontLicenseUrl;
}
final String licenseString = response.body;
if (!licenseString.contains(attributionString)) {
return fontLicenseUrl;
}
}
final String licenseString = response.body;

return licenseString.contains(attributionString);
return null;
}

// The basic information of a font: its [family] (name) and [uri].
Expand Down
Loading