From beed86c9ba43b7231f7eabe6ced6db86fbf471d6 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 11:50:24 -0800 Subject: [PATCH 1/7] Use `runes` to get code units in CanvasKit. --- lib/web_ui/dev/test_runner.dart | 68 ++++++++++--------- .../src/engine/canvaskit/font_fallbacks.dart | 30 ++++---- lib/web_ui/lib/src/engine/canvaskit/text.dart | 31 +++++---- .../canvaskit/fallback_fonts_golden_test.dart | 64 +++++++++++++++++ 4 files changed, 135 insertions(+), 58 deletions(-) diff --git a/lib/web_ui/dev/test_runner.dart b/lib/web_ui/dev/test_runner.dart index f5d9ed67433ec..e9a555fcf9e4a 100644 --- a/lib/web_ui/dev/test_runner.dart +++ b/lib/web_ui/dev/test_runner.dart @@ -92,8 +92,7 @@ class TestCommand extends Command with ArgUtils { 'fetch-goldens-repo', defaultsTo: true, negatable: true, - help: - 'Whether to fetch the goldens repo. Set this to false to iterate ' + help: 'Whether to fetch the goldens repo. Set this to false to iterate ' 'on golden tests without fearing that the fetcher will overwrite ' 'your local changes.', ) @@ -174,39 +173,41 @@ class TestCommand extends Command with ArgUtils { final FilePath dir = FilePath.fromWebUi(''); print(''); print('Initial test run is done!'); - print('Watching ${dir.relativeToCwd}/lib and ${dir.relativeToCwd}/test to re-run tests'); + print( + 'Watching ${dir.relativeToCwd}/lib and ${dir.relativeToCwd}/test to re-run tests'); print(''); PipelineWatcher( - dir: dir.absolute, - pipeline: testPipeline, - ignore: (event) { - // Ignore font files that are copied whenever tests run. - if (event.path.endsWith('.ttf')) { - return true; - } + dir: dir.absolute, + pipeline: testPipeline, + ignore: (event) { + // Ignore font files that are copied whenever tests run. + if (event.path.endsWith('.ttf')) { + return true; + } - // Ignore auto-generated JS files. - // The reason we are using `.contains()` instead of `.endsWith()` is - // because the auto-generated files could end with any of the - // following: - // - // - browser_test.dart.js - // - browser_test.dart.js.map - // - browser_test.dart.js.deps - if (event.path.contains('browser_test.dart.js')) { - return true; - } + // Ignore auto-generated JS files. + // The reason we are using `.contains()` instead of `.endsWith()` is + // because the auto-generated files could end with any of the + // following: + // + // - browser_test.dart.js + // - browser_test.dart.js.map + // - browser_test.dart.js.deps + if (event.path.contains('browser_test.dart.js')) { + return true; + } - // React to changes in lib/ and test/ folders. - final String relativePath = path.relative(event.path, from: dir.absolute); - if (relativePath.startsWith('lib/') || relativePath.startsWith('test/')) { - return false; - } + // React to changes in lib/ and test/ folders. + final String relativePath = + path.relative(event.path, from: dir.absolute); + if (relativePath.startsWith('lib/') || + relativePath.startsWith('test/')) { + return false; + } - // Ignore anything else. - return true; - } - ).start(); + // Ignore anything else. + return true; + }).start(); // Return a never-ending future. return Completer().future; } else { @@ -226,7 +227,8 @@ class TestCommand extends Command with ArgUtils { bool unitTestResult = await runUnitTests(); bool integrationTestResult = await runIntegrationTests(); if (integrationTestResult != unitTestResult) { - print('Tests run. Integration tests passed: $integrationTestResult ' + print( + 'Tests run. Integration tests passed: $integrationTestResult ' 'unit tests passed: $unitTestResult'); } return integrationTestResult && unitTestResult; @@ -234,7 +236,8 @@ class TestCommand extends Command with ArgUtils { return await runUnitTests(); } } - throw UnimplementedError('Unknown test type requested: $testTypesRequested'); + throw UnimplementedError( + 'Unknown test type requested: $testTypesRequested'); } on TestFailureException { return true; } @@ -786,6 +789,7 @@ const List _kTestFonts = [ 'ahem.ttf', 'Roboto-Regular.ttf', 'NotoNaskhArabic-Regular.ttf', + 'NotoColorEmoji.ttf', ]; void _copyTestFontsIntoWebUi() { diff --git a/lib/web_ui/lib/src/engine/canvaskit/font_fallbacks.dart b/lib/web_ui/lib/src/engine/canvaskit/font_fallbacks.dart index b395ec706af73..44654e05b35d0 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/font_fallbacks.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/font_fallbacks.dart @@ -211,15 +211,19 @@ Future _registerSymbolsAndEmoji() async { String? symbolsFontUrl = extractUrlFromCss(symbolsCss); String? emojiFontUrl = extractUrlFromCss(emojiCss); - if (symbolsFontUrl == null || emojiFontUrl == null) { - html.window.console - .warn('Error parsing CSS for Noto Emoji and Symbols font.'); + if (symbolsFontUrl != null) { + notoDownloadQueue.add(_ResolvedNotoSubset( + symbolsFontUrl, 'Noto Sans Symbols', const [])); + } else { + html.window.console.warn('Error parsing CSS for Noto Symbols font.'); } - notoDownloadQueue.add(_ResolvedNotoSubset( - symbolsFontUrl!, 'Noto Sans Symbols', const [])); - notoDownloadQueue.add(_ResolvedNotoSubset( - emojiFontUrl!, 'Noto Color Emoji Compat', const [])); + if (emojiFontUrl != null) { + notoDownloadQueue.add(_ResolvedNotoSubset( + emojiFontUrl, 'Noto Color Emoji Compat', const [])); + } else { + html.window.console.warn('Error parsing CSS for Noto Emoji font.'); + } } /// Finds the minimum set of fonts which covers all of the [codeunits]. @@ -695,9 +699,10 @@ class NotoDownloader { if (assertionsEnabled) { _debugActiveDownloadCount += 1; } - final Future result = html.window.fetch(url).then((dynamic fetchResult) => fetchResult - .arrayBuffer() - .then((dynamic x) => x as ByteBuffer)); + final Future result = html.window.fetch(url).then( + (dynamic fetchResult) => fetchResult + .arrayBuffer() + .then((dynamic x) => x as ByteBuffer)); if (assertionsEnabled) { result.whenComplete(() { _debugActiveDownloadCount -= 1; @@ -713,8 +718,9 @@ class NotoDownloader { if (assertionsEnabled) { _debugActiveDownloadCount += 1; } - final Future result = html.window.fetch(url).then((dynamic response) => - response.text().then((dynamic x) => x as String)); + final Future result = html.window.fetch(url).then( + (dynamic response) => + response.text().then((dynamic x) => x as String)); if (assertionsEnabled) { result.whenComplete(() { _debugActiveDownloadCount -= 1; diff --git a/lib/web_ui/lib/src/engine/canvaskit/text.dart b/lib/web_ui/lib/src/engine/canvaskit/text.dart index ed79932ff999f..7f7e12df9b103 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -20,7 +20,7 @@ class CkParagraphStyle implements ui.ParagraphStyle { ui.StrutStyle? strutStyle, String? ellipsis, ui.Locale? locale, - }) : skParagraphStyle = toSkParagraphStyle( + }) : skParagraphStyle = toSkParagraphStyle( textAlign, textDirection, maxLines, @@ -34,11 +34,11 @@ class CkParagraphStyle implements ui.ParagraphStyle { ellipsis, locale, ), - _textDirection = textDirection ?? ui.TextDirection.ltr, - _fontFamily = fontFamily, - _fontSize = fontSize, - _fontWeight = fontWeight, - _fontStyle = fontStyle; + _textDirection = textDirection ?? ui.TextDirection.ltr, + _fontFamily = fontFamily, + _fontSize = fontSize, + _fontWeight = fontWeight, + _fontStyle = fontStyle; final SkParagraphStyle skParagraphStyle; final ui.TextDirection? _textDirection; @@ -276,13 +276,14 @@ class CkTextStyle implements ui.TextStyle { } /// Lazy-initialized list of font families sent to Skia. - late final List effectiveFontFamilies = _getEffectiveFontFamilies(fontFamily, fontFamilyFallback); + late final List effectiveFontFamilies = + _getEffectiveFontFamilies(fontFamily, fontFamilyFallback); /// Lazy-initialized Skia style used to pass the style to Skia. /// /// This is lazy because not every style ends up being passed to Skia, so the /// conversion would be wasteful. - late final SkTextStyle skTextStyle = () { + late final SkTextStyle skTextStyle = () { // Write field values to locals so null checks promote types to non-null. final ui.Color? color = this.color; final ui.TextDecoration? decoration = this.decoration; @@ -695,14 +696,14 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { typefaces.addAll(typefacesForFamily); } } - List codeUnitsSupported = List.filled(text.length, false); + List codeUnits = text.runes.toList(); + List codeUnitsSupported = List.filled(codeUnits.length, false); for (SkTypeface typeface in typefaces) { SkFont font = SkFont(typeface); Uint8List glyphs = font.getGlyphIDs(text); assert(glyphs.length == codeUnitsSupported.length); for (int i = 0; i < glyphs.length; i++) { - codeUnitsSupported[i] |= - glyphs[i] != 0 || _isControlCode(text.codeUnitAt(i)); + codeUnitsSupported[i] |= glyphs[i] != 0 || _isControlCode(codeUnits[i]); } } @@ -710,7 +711,7 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { List missingCodeUnits = []; for (int i = 0; i < codeUnitsSupported.length; i++) { if (!codeUnitsSupported[i]) { - missingCodeUnits.add(text.codeUnitAt(i)); + missingCodeUnits.add(codeUnits[i]); } } _findFontsForMissingCodeunits(missingCodeUnits); @@ -778,7 +779,8 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { // This object is never deleted. It is effectively a static global constant. // Therefore it doesn't need to be wrapped in CkPaint. static final SkPaint _defaultTextForeground = SkPaint(); - static final SkPaint _defaultTextBackground = SkPaint()..setColorInt(0x00000000); + static final SkPaint _defaultTextBackground = SkPaint() + ..setColorInt(0x00000000); @override void pushStyle(ui.TextStyle style) { @@ -796,7 +798,8 @@ class CkParagraphBuilder implements ui.ParagraphBuilder { foreground = _defaultTextForeground; } - final SkPaint background = skStyle.background?.skiaObject ?? _defaultTextBackground; + final SkPaint background = + skStyle.background?.skiaObject ?? _defaultTextBackground; _paragraphBuilder.pushPaintStyle( skStyle.skTextStyle, foreground, background); } else { diff --git a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart index 955ea4c54e48a..a8cf2c015dfb2 100644 --- a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart +++ b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart @@ -121,6 +121,70 @@ void testMain() { // TODO: https://github.com/flutter/flutter/issues/71520 }, skip: isIosSafari || isFirefox); + test('will download Noto Emojis and Noto Symbols if no matching Noto Font', + () async { + final Completer fontChangeCompleter = Completer(); + // Intercept the system font change message. + ui.window.onPlatformMessage = (String name, ByteData? data, + ui.PlatformMessageResponseCallback? callback) { + if (name == 'flutter/system') { + const JSONMessageCodec codec = JSONMessageCodec(); + final dynamic message = codec.decodeMessage(data); + if (message is Map) { + if (message['type'] == 'fontsChange') { + fontChangeCompleter.complete(); + } + } + } + if (savedCallback != null) { + savedCallback!(name, data, callback); + } + }; + + TestDownloader.mockDownloads[ + 'https://fonts.googleapis.com/css2?family=Noto+Color+Emoji+Compat'] = + ''' +/* arabic */ +@font-face { + font-family: 'Noto Color Emoji'; + src: url(packages/ui/assets/NotoColorEmoji.ttf) format('ttf'); +} +'''; + + expect(skiaFontCollection.globalFontFallbacks, ['Roboto']); + + // Creating this paragraph should cause us to start to download the + // fallback font. + CkParagraphBuilder pb = CkParagraphBuilder( + CkParagraphStyle(), + ); + pb.addText('Hello 😊'); + + await fontChangeCompleter.future; + + expect(skiaFontCollection.globalFontFallbacks, + contains('Noto Color Emoji Compat 0')); + + final CkPictureRecorder recorder = CkPictureRecorder(); + final CkCanvas canvas = recorder.beginRecording(kDefaultRegion); + + pb = CkParagraphBuilder( + CkParagraphStyle( + fontSize: 32, + ), + ); + pb.addText('Hello 😊'); + final CkParagraph paragraph = pb.build(); + paragraph.layout(ui.ParagraphConstraints(width: 1000)); + + canvas.drawParagraph(paragraph, ui.Offset(200, 120)); + + await matchPictureGolden( + 'canvaskit_font_fallback_emoji.png', recorder.endRecording()); + // TODO: https://github.com/flutter/flutter/issues/60040 + // TODO: https://github.com/flutter/flutter/issues/71520 + }, skip: isIosSafari || isFirefox); + test('will gracefully fail if we cannot parse the Google Fonts CSS', () async { TestDownloader.mockDownloads[ From 419a12dc72a22e21dcafdbcc422b4ea655e7df4c Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 12:48:57 -0800 Subject: [PATCH 2/7] Reduced whitespace in golden tests --- .../canvaskit/fallback_fonts_golden_test.dart | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart index a8cf2c015dfb2..ae5dbb5099920 100644 --- a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart +++ b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart @@ -19,7 +19,7 @@ void main() { internalBootstrapBrowserTest(() => testMain); } -const ui.Rect kDefaultRegion = const ui.Rect.fromLTRB(0, 0, 500, 250); +const ui.Rect kDefaultRegion = const ui.Rect.fromLTRB(0, 0, 100, 50); Future matchPictureGolden(String goldenFile, CkPicture picture, {ui.Rect region = kDefaultRegion, bool write = false}) async { @@ -30,7 +30,7 @@ Future matchPictureGolden(String goldenFile, CkPicture picture, sb.addPicture(ui.Offset.zero, picture); dispatcher.rasterizer!.draw(sb.build().layerTree); await matchGoldenFile(goldenFile, - region: region, maxDiffRatePercent: 0.0, write: write); + region: region, maxDiffRatePercent: 0.0, write: true); } void testMain() { @@ -105,15 +105,15 @@ void testMain() { final CkCanvas canvas = recorder.beginRecording(kDefaultRegion); pb = CkParagraphBuilder( - CkParagraphStyle( - fontSize: 32, - ), + CkParagraphStyle(), ); + pb.pushStyle(ui.TextStyle(fontSize: 60)); pb.addText('مرحبا'); + pb.pop(); final CkParagraph paragraph = pb.build(); paragraph.layout(ui.ParagraphConstraints(width: 1000)); - canvas.drawParagraph(paragraph, ui.Offset(200, 120)); + canvas.drawParagraph(paragraph, ui.Offset(0, 0)); await matchPictureGolden( 'canvaskit_font_fallback_arabic.png', recorder.endRecording()); @@ -169,15 +169,15 @@ void testMain() { final CkCanvas canvas = recorder.beginRecording(kDefaultRegion); pb = CkParagraphBuilder( - CkParagraphStyle( - fontSize: 32, - ), + CkParagraphStyle(), ); + pb.pushStyle(ui.TextStyle(fontSize: 48)); pb.addText('Hello 😊'); + pb.pop(); final CkParagraph paragraph = pb.build(); paragraph.layout(ui.ParagraphConstraints(width: 1000)); - canvas.drawParagraph(paragraph, ui.Offset(200, 120)); + canvas.drawParagraph(paragraph, ui.Offset(0, 0)); await matchPictureGolden( 'canvaskit_font_fallback_emoji.png', recorder.endRecording()); From 83f37bb7722df13f09ec245cce237e7baf7bd019 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 12:50:58 -0800 Subject: [PATCH 3/7] Update goldens lock --- lib/web_ui/dev/goldens_lock.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index 682f836bbe1b5..7c48812199af9 100644 --- a/lib/web_ui/dev/goldens_lock.yaml +++ b/lib/web_ui/dev/goldens_lock.yaml @@ -1,2 +1,2 @@ repository: https://github.com/flutter/goldens.git -revision: d70eca62b254302b293973573d3b16ffd05b19db +revision: 42c0e0436bd282064402bd982a406fa0670bdae6 From ab6bb039c98bc464eecc265f1677ef6513b811f2 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 13:22:26 -0800 Subject: [PATCH 4/7] Don't force overwrite the goldens --- lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart index ae5dbb5099920..a47222be35872 100644 --- a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart +++ b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart @@ -30,7 +30,7 @@ Future matchPictureGolden(String goldenFile, CkPicture picture, sb.addPicture(ui.Offset.zero, picture); dispatcher.rasterizer!.draw(sb.build().layerTree); await matchGoldenFile(goldenFile, - region: region, maxDiffRatePercent: 0.0, write: true); + region: region, maxDiffRatePercent: 0.0, write: write); } void testMain() { From cf89ebd770dd7875f992d1ca21c99a6ca3a33056 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 15:32:00 -0800 Subject: [PATCH 5/7] Fix golden tests for 1 DPR machine --- lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart index a47222be35872..001d1c1007d13 100644 --- a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart +++ b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart @@ -30,7 +30,7 @@ Future matchPictureGolden(String goldenFile, CkPicture picture, sb.addPicture(ui.Offset.zero, picture); dispatcher.rasterizer!.draw(sb.build().layerTree); await matchGoldenFile(goldenFile, - region: region, maxDiffRatePercent: 0.0, write: write); + region: region, maxDiffRatePercent: 0.0, write: true); } void testMain() { @@ -107,7 +107,7 @@ void testMain() { pb = CkParagraphBuilder( CkParagraphStyle(), ); - pb.pushStyle(ui.TextStyle(fontSize: 60)); + pb.pushStyle(ui.TextStyle(fontSize: 32)); pb.addText('مرحبا'); pb.pop(); final CkParagraph paragraph = pb.build(); @@ -171,7 +171,7 @@ void testMain() { pb = CkParagraphBuilder( CkParagraphStyle(), ); - pb.pushStyle(ui.TextStyle(fontSize: 48)); + pb.pushStyle(ui.TextStyle(fontSize: 26)); pb.addText('Hello 😊'); pb.pop(); final CkParagraph paragraph = pb.build(); From fc66bb06ec75ed0707d03af351afb000e2e732f9 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 15:46:43 -0800 Subject: [PATCH 6/7] Update golden lock again --- lib/web_ui/dev/goldens_lock.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index 7c48812199af9..8bb0d377fa78e 100644 --- a/lib/web_ui/dev/goldens_lock.yaml +++ b/lib/web_ui/dev/goldens_lock.yaml @@ -1,2 +1,2 @@ repository: https://github.com/flutter/goldens.git -revision: 42c0e0436bd282064402bd982a406fa0670bdae6 +revision: 44f00682eee2afd7042c02ce802199c1c4ff223e From ac9cc3e92c82ed6a64bc7a3fa3ee4a39ec9fc95e Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Thu, 28 Jan 2021 16:19:43 -0800 Subject: [PATCH 7/7] Don't overwrite goldens --- lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart index 001d1c1007d13..cc2fd0d1cff55 100644 --- a/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart +++ b/lib/web_ui/test/canvaskit/fallback_fonts_golden_test.dart @@ -30,7 +30,7 @@ Future matchPictureGolden(String goldenFile, CkPicture picture, sb.addPicture(ui.Offset.zero, picture); dispatcher.rasterizer!.draw(sb.build().layerTree); await matchGoldenFile(goldenFile, - region: region, maxDiffRatePercent: 0.0, write: true); + region: region, maxDiffRatePercent: 0.0, write: write); } void testMain() {