Skip to content

Commit 09c4206

Browse files
Harry Terkelsenharryterkelsen
Harry Terkelsen
authored and
harryterkelsen
committed
Use runes to get code units in CanvasKit. (flutter#24024)
1 parent 5d3477e commit 09c4206

5 files changed

Lines changed: 128 additions & 47 deletions

File tree

‎lib/web_ui/dev/goldens_lock.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
repository: https://github.com/flutter/goldens.git
2-
revision: b85f9093e6bc6d4e7cbb7f97491667c143c4a360
2+
revision: 44f00682eee2afd7042c02ce802199c1c4ff223e

‎lib/web_ui/dev/test_runner.dart‎

Lines changed: 43 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ class TestCommand extends Command<bool> with ArgUtils {
8888
'.dart_tool/goldens. Use this option to bulk-update all screenshots, '
8989
'for example, when a new browser version affects pixels.',
9090
)
91+
..addFlag(
92+
'fetch-goldens-repo',
93+
defaultsTo: true,
94+
negatable: true,
95+
help: 'Whether to fetch the goldens repo. Set this to false to iterate '
96+
'on golden tests without fearing that the fetcher will overwrite '
97+
'your local changes.',
98+
)
9199
..addOption(
92100
'browser',
93101
defaultsTo: 'chrome',
@@ -165,39 +173,41 @@ class TestCommand extends Command<bool> with ArgUtils {
165173
final FilePath dir = FilePath.fromWebUi('');
166174
print('');
167175
print('Initial test run is done!');
168-
print('Watching ${dir.relativeToCwd}/lib and ${dir.relativeToCwd}/test to re-run tests');
176+
print(
177+
'Watching ${dir.relativeToCwd}/lib and ${dir.relativeToCwd}/test to re-run tests');
169178
print('');
170179
PipelineWatcher(
171-
dir: dir.absolute,
172-
pipeline: testPipeline,
173-
ignore: (event) {
174-
// Ignore font files that are copied whenever tests run.
175-
if (event.path.endsWith('.ttf')) {
176-
return true;
177-
}
180+
dir: dir.absolute,
181+
pipeline: testPipeline,
182+
ignore: (event) {
183+
// Ignore font files that are copied whenever tests run.
184+
if (event.path.endsWith('.ttf')) {
185+
return true;
186+
}
178187

179-
// Ignore auto-generated JS files.
180-
// The reason we are using `.contains()` instead of `.endsWith()` is
181-
// because the auto-generated files could end with any of the
182-
// following:
183-
//
184-
// - browser_test.dart.js
185-
// - browser_test.dart.js.map
186-
// - browser_test.dart.js.deps
187-
if (event.path.contains('browser_test.dart.js')) {
188-
return true;
189-
}
188+
// Ignore auto-generated JS files.
189+
// The reason we are using `.contains()` instead of `.endsWith()` is
190+
// because the auto-generated files could end with any of the
191+
// following:
192+
//
193+
// - browser_test.dart.js
194+
// - browser_test.dart.js.map
195+
// - browser_test.dart.js.deps
196+
if (event.path.contains('browser_test.dart.js')) {
197+
return true;
198+
}
190199

191-
// React to changes in lib/ and test/ folders.
192-
final String relativePath = path.relative(event.path, from: dir.absolute);
193-
if (relativePath.startsWith('lib/') || relativePath.startsWith('test/')) {
194-
return false;
195-
}
200+
// React to changes in lib/ and test/ folders.
201+
final String relativePath =
202+
path.relative(event.path, from: dir.absolute);
203+
if (relativePath.startsWith('lib/') ||
204+
relativePath.startsWith('test/')) {
205+
return false;
206+
}
196207

197-
// Ignore anything else.
198-
return true;
199-
}
200-
).start();
208+
// Ignore anything else.
209+
return true;
210+
}).start();
201211
// Return a never-ending future.
202212
return Completer<bool>().future;
203213
} else {
@@ -217,15 +227,17 @@ class TestCommand extends Command<bool> with ArgUtils {
217227
bool unitTestResult = await runUnitTests();
218228
bool integrationTestResult = await runIntegrationTests();
219229
if (integrationTestResult != unitTestResult) {
220-
print('Tests run. Integration tests passed: $integrationTestResult '
230+
print(
231+
'Tests run. Integration tests passed: $integrationTestResult '
221232
'unit tests passed: $unitTestResult');
222233
}
223234
return integrationTestResult && unitTestResult;
224235
} else {
225236
return await runUnitTests();
226237
}
227238
}
228-
throw UnimplementedError('Unknown test type requested: $testTypesRequested');
239+
throw UnimplementedError(
240+
'Unknown test type requested: $testTypesRequested');
229241
} on TestFailureException {
230242
return true;
231243
}
@@ -774,6 +786,7 @@ const List<String> _kTestFonts = <String>[
774786
'ahem.ttf',
775787
'Roboto-Regular.ttf',
776788
'NotoNaskhArabic-Regular.ttf',
789+
'NotoColorEmoji.ttf',
777790
];
778791

779792
void _copyTestFontsIntoWebUi() {

‎lib/web_ui/lib/src/engine/canvaskit/font_fallbacks.dart‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,15 +211,19 @@ Future<void> _registerSymbolsAndEmoji() async {
211211
String? symbolsFontUrl = extractUrlFromCss(symbolsCss);
212212
String? emojiFontUrl = extractUrlFromCss(emojiCss);
213213

214-
if (symbolsFontUrl == null || emojiFontUrl == null) {
215-
html.window.console
216-
.warn('Error parsing CSS for Noto Emoji and Symbols font.');
214+
if (symbolsFontUrl != null) {
215+
notoDownloadQueue.add(_ResolvedNotoSubset(
216+
symbolsFontUrl, 'Noto Sans Symbols', const <CodeunitRange>[]));
217+
} else {
218+
html.window.console.warn('Error parsing CSS for Noto Symbols font.');
217219
}
218220

219-
notoDownloadQueue.add(_ResolvedNotoSubset(
220-
symbolsFontUrl!, 'Noto Sans Symbols', const <CodeunitRange>[]));
221-
notoDownloadQueue.add(_ResolvedNotoSubset(
222-
emojiFontUrl!, 'Noto Color Emoji Compat', const <CodeunitRange>[]));
221+
if (emojiFontUrl != null) {
222+
notoDownloadQueue.add(_ResolvedNotoSubset(
223+
emojiFontUrl, 'Noto Color Emoji Compat', const <CodeunitRange>[]));
224+
} else {
225+
html.window.console.warn('Error parsing CSS for Noto Emoji font.');
226+
}
223227
}
224228

225229
/// Finds the minimum set of fonts which covers all of the [codeunits].

‎lib/web_ui/lib/src/engine/canvaskit/text.dart‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -668,22 +668,22 @@ class CkParagraphBuilder implements ui.ParagraphBuilder {
668668
typefaces.addAll(typefacesForFamily);
669669
}
670670
}
671-
List<bool> codeUnitsSupported = List<bool>.filled(text.length, false);
671+
List<int> codeUnits = text.runes.toList();
672+
List<bool> codeUnitsSupported = List<bool>.filled(codeUnits.length, false);
672673
for (SkTypeface typeface in typefaces) {
673674
SkFont font = SkFont(typeface);
674675
Uint8List glyphs = font.getGlyphIDs(text);
675676
assert(glyphs.length == codeUnitsSupported.length);
676677
for (int i = 0; i < glyphs.length; i++) {
677-
codeUnitsSupported[i] |=
678-
glyphs[i] != 0 || _isControlCode(text.codeUnitAt(i));
678+
codeUnitsSupported[i] |= glyphs[i] != 0 || _isControlCode(codeUnits[i]);
679679
}
680680
}
681681

682682
if (codeUnitsSupported.any((x) => !x)) {
683683
List<int> missingCodeUnits = <int>[];
684684
for (int i = 0; i < codeUnitsSupported.length; i++) {
685685
if (!codeUnitsSupported[i]) {
686-
missingCodeUnits.add(text.codeUnitAt(i));
686+
missingCodeUnits.add(codeUnits[i]);
687687
}
688688
}
689689
_findFontsForMissingCodeunits(missingCodeUnits);

‎lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart‎

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ void main() {
1919
internalBootstrapBrowserTest(() => testMain);
2020
}
2121

22-
const ui.Rect kDefaultRegion = const ui.Rect.fromLTRB(0, 0, 500, 250);
22+
const ui.Rect kDefaultRegion = const ui.Rect.fromLTRB(0, 0, 100, 50);
2323

2424
Future<void> matchPictureGolden(String goldenFile, CkPicture picture,
2525
{ui.Rect region = kDefaultRegion, bool write = false}) async {
@@ -105,22 +105,86 @@ void testMain() {
105105
final CkCanvas canvas = recorder.beginRecording(kDefaultRegion);
106106

107107
pb = CkParagraphBuilder(
108-
CkParagraphStyle(
109-
fontSize: 32,
110-
),
108+
CkParagraphStyle(),
111109
);
110+
pb.pushStyle(ui.TextStyle(fontSize: 32));
112111
pb.addText('مرحبا');
112+
pb.pop();
113113
final CkParagraph paragraph = pb.build();
114114
paragraph.layout(ui.ParagraphConstraints(width: 1000));
115115

116-
canvas.drawParagraph(paragraph, ui.Offset(200, 120));
116+
canvas.drawParagraph(paragraph, ui.Offset(0, 0));
117117

118118
await matchPictureGolden(
119119
'canvaskit_font_fallback_arabic.png', recorder.endRecording());
120120
// TODO: https://github.com/flutter/flutter/issues/60040
121121
// TODO: https://github.com/flutter/flutter/issues/71520
122122
}, skip: isIosSafari || isFirefox);
123123

124+
test('will download Noto Emojis and Noto Symbols if no matching Noto Font',
125+
() async {
126+
final Completer<void> fontChangeCompleter = Completer<void>();
127+
// Intercept the system font change message.
128+
ui.window.onPlatformMessage = (String name, ByteData? data,
129+
ui.PlatformMessageResponseCallback? callback) {
130+
if (name == 'flutter/system') {
131+
const JSONMessageCodec codec = JSONMessageCodec();
132+
final dynamic message = codec.decodeMessage(data);
133+
if (message is Map) {
134+
if (message['type'] == 'fontsChange') {
135+
fontChangeCompleter.complete();
136+
}
137+
}
138+
}
139+
if (savedCallback != null) {
140+
savedCallback!(name, data, callback);
141+
}
142+
};
143+
144+
TestDownloader.mockDownloads[
145+
'https://fonts.googleapis.com/css2?family=Noto+Color+Emoji+Compat'] =
146+
'''
147+
/* arabic */
148+
@font-face {
149+
font-family: 'Noto Color Emoji';
150+
src: url(packages/ui/assets/NotoColorEmoji.ttf) format('ttf');
151+
}
152+
''';
153+
154+
expect(skiaFontCollection.globalFontFallbacks, ['Roboto']);
155+
156+
// Creating this paragraph should cause us to start to download the
157+
// fallback font.
158+
CkParagraphBuilder pb = CkParagraphBuilder(
159+
CkParagraphStyle(),
160+
);
161+
pb.addText('Hello 😊');
162+
163+
await fontChangeCompleter.future;
164+
165+
expect(skiaFontCollection.globalFontFallbacks,
166+
contains('Noto Color Emoji Compat 0'));
167+
168+
final CkPictureRecorder recorder = CkPictureRecorder();
169+
final CkCanvas canvas = recorder.beginRecording(kDefaultRegion);
170+
171+
pb = CkParagraphBuilder(
172+
CkParagraphStyle(),
173+
);
174+
pb.pushStyle(ui.TextStyle(fontSize: 26));
175+
pb.addText('Hello 😊');
176+
pb.pop();
177+
final CkParagraph paragraph = pb.build();
178+
paragraph.layout(ui.ParagraphConstraints(width: 1000));
179+
180+
canvas.drawParagraph(paragraph, ui.Offset(0, 0));
181+
182+
await matchPictureGolden(
183+
'canvaskit_font_fallback_emoji.png', recorder.endRecording());
184+
// TODO: https://github.com/flutter/flutter/issues/60040
185+
// TODO: https://github.com/flutter/flutter/issues/71520
186+
}, skip: isIosSafari || isFirefox);
187+
124188
test('will gracefully fail if we cannot parse the Google Fonts CSS',
125189
() async {
126190
TestDownloader.mockDownloads[

0 commit comments

Comments
 (0)