From 40092d2f59f99ebc877909b07810894b4a7bc696 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Mon, 25 Mar 2024 20:15:28 -0700 Subject: [PATCH 1/4] Pass the flutter test font by default. --- .../engine/skwasm/skwasm_impl/paragraph.dart | 20 +++++++++-------- lib/web_ui/test/ui/line_metrics_test.dart | 22 ++++++++++++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart index 7b036779059b5..7fcc25df5c5fc 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart @@ -15,10 +15,12 @@ const int _kSoftLineBreak = 0; const int _kHardLineBreak = 100; final List _testFonts = ['FlutterTest', 'Ahem']; -String _computeEffectiveFontFamily(String fontFamily) { - return ui_web.debugEmulateFlutterTesterEnvironment && !_testFonts.contains(fontFamily) - ? _testFonts.first - : fontFamily; +List _computeEffectiveFontFamilies(List fontFamilies) { + if (!ui_web.debugEmulateFlutterTesterEnvironment) { + return fontFamilies; + } + final Iterable filteredFonts = fontFamilies.where(_testFonts.contains); + return filteredFonts.isEmpty ? _testFonts : filteredFonts.toList(); } class SkwasmLineMetrics extends SkwasmObjectWrapper implements ui.LineMetrics { @@ -359,7 +361,7 @@ class SkwasmTextStyle implements ui.TextStyle { textStyleSetTextBaseline(handle, textBaseline!.index); } - final List effectiveFontFamilies = fontFamilies; + final List effectiveFontFamilies = _computeEffectiveFontFamilies(fontFamilies); if (effectiveFontFamilies.isNotEmpty) { withScopedFontList(effectiveFontFamilies, (Pointer families, int count) => @@ -438,8 +440,8 @@ class SkwasmTextStyle implements ui.TextStyle { } List get fontFamilies => [ - if (fontFamily != null) _computeEffectiveFontFamily(fontFamily!), - if (fontFamilyFallback != null) ...fontFamilyFallback!.map(_computeEffectiveFontFamily), + if (fontFamily != null) fontFamily!, + if (fontFamilyFallback != null) ...fontFamilyFallback!, ]; final ui.Color? color; @@ -582,7 +584,7 @@ final class SkwasmStrutStyle extends SkwasmObjectWrapper implemen if (fontFamilyFallback != null) ...fontFamilyFallback, ]; if (fontFamilies.isNotEmpty) { - withScopedFontList(fontFamilies, + withScopedFontList(_computeEffectiveFontFamilies(fontFamilies), (Pointer families, int count) => strutStyleSetFontFamilies(handle, families, count)); } @@ -729,7 +731,7 @@ class SkwasmParagraphStyle extends SkwasmObjectWrapper implem final TextStyleHandle textStyleHandle = textStyle.handle; if (fontFamily != null) { - withScopedFontList([fontFamily], + withScopedFontList(_computeEffectiveFontFamilies([fontFamily]), (Pointer families, int count) => textStyleAddFontFamilies(textStyleHandle, families, count)); } diff --git a/lib/web_ui/test/ui/line_metrics_test.dart b/lib/web_ui/test/ui/line_metrics_test.dart index b356c4c8d48fd..bffc8fa084ad9 100644 --- a/lib/web_ui/test/ui/line_metrics_test.dart +++ b/lib/web_ui/test/ui/line_metrics_test.dart @@ -117,7 +117,7 @@ Future testMain() async { } }, skip: isHtml); // The rounding hack doesn't apply to the html renderer - test('uses flutter test fonts when debugEmulateFlutterTesterEnvironment is enabled', () { + test('overrides with flutter test font when debugEmulateFlutterTesterEnvironment is enabled', () { final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); builder.pushStyle(ui.TextStyle( fontSize: 10.0, @@ -128,6 +128,26 @@ Future testMain() async { paragraph.layout(const ui.ParagraphConstraints(width: 400)); expect(paragraph.numberOfLines, 1); + expect(paragraph.height, 10); + + final ui.LineMetrics? metrics = paragraph.getLineMetricsAt(0); + expect(metrics, isNotNull); + + // FlutterTest font's 'X' character is a square, so it's the font size (10.0) * 4 characters. + expect(metrics!.width, 40.0); + }); + + test('uses flutter test font by default when debugEmulateFlutterTesterEnvironment is enabled', () { + final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); + builder.pushStyle(ui.TextStyle( + fontSize: 10.0, + )); + builder.addText('XXXX'); + final ui.Paragraph paragraph = builder.build(); + paragraph.layout(const ui.ParagraphConstraints(width: 400)); + + expect(paragraph.numberOfLines, 1); + expect(paragraph.height, 10); final ui.LineMetrics? metrics = paragraph.getLineMetricsAt(0); expect(metrics, isNotNull); From de82ef8055323e975fc5f3ae9147e69c1d3c14c2 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Mon, 25 Mar 2024 20:34:11 -0700 Subject: [PATCH 2/4] Be more consistent with how we pass the fonts. --- .../engine/skwasm/skwasm_impl/paragraph.dart | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart index 7fcc25df5c5fc..6f6d724121136 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paragraph.dart @@ -578,16 +578,13 @@ final class SkwasmStrutStyle extends SkwasmObjectWrapper implemen ui.TextLeadingDistribution? leadingDistribution, }) { final StrutStyleHandle handle = strutStyleCreate(); - if (fontFamily != null || fontFamilyFallback != null) { - final List fontFamilies = [ - if (fontFamily != null) fontFamily, - if (fontFamilyFallback != null) ...fontFamilyFallback, - ]; - if (fontFamilies.isNotEmpty) { - withScopedFontList(_computeEffectiveFontFamilies(fontFamilies), - (Pointer families, int count) => - strutStyleSetFontFamilies(handle, families, count)); - } + final List effectiveFontFamilies = _computeEffectiveFontFamilies([ + if (fontFamily != null) fontFamily, + if (fontFamilyFallback != null) ...fontFamilyFallback, + ]); + if (effectiveFontFamilies.isNotEmpty) { + withScopedFontList(effectiveFontFamilies, (Pointer families, int count) => + strutStyleSetFontFamilies(handle, families, count)); } if (fontSize != null) { strutStyleSetFontSize(handle, fontSize); @@ -730,9 +727,11 @@ class SkwasmParagraphStyle extends SkwasmObjectWrapper implem (renderer.fontCollection as SkwasmFontCollection).defaultTextStyle.copy(); final TextStyleHandle textStyleHandle = textStyle.handle; - if (fontFamily != null) { - withScopedFontList(_computeEffectiveFontFamilies([fontFamily]), - (Pointer families, int count) => + final List effectiveFontFamilies = _computeEffectiveFontFamilies([ + if (fontFamily != null) fontFamily + ]); + if (effectiveFontFamilies.isNotEmpty) { + withScopedFontList(effectiveFontFamilies, (Pointer families, int count) => textStyleAddFontFamilies(textStyleHandle, families, count)); } if (fontSize != null) { From d9a2ef998eef8ee7a274d33075e2beb7c64d722c Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Tue, 26 Mar 2024 09:33:21 -0700 Subject: [PATCH 3/4] Added one more test when debugEmulateFlutterTestEnvironment is disabled. --- lib/web_ui/test/ui/line_metrics_test.dart | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/web_ui/test/ui/line_metrics_test.dart b/lib/web_ui/test/ui/line_metrics_test.dart index bffc8fa084ad9..4bd9cbdc8bd4f 100644 --- a/lib/web_ui/test/ui/line_metrics_test.dart +++ b/lib/web_ui/test/ui/line_metrics_test.dart @@ -5,6 +5,7 @@ import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/ui.dart' as ui; +import 'package:ui/ui_web/src/ui_web/testing.dart'; import '../common/test_initialization.dart'; import 'utils.dart'; @@ -121,7 +122,7 @@ Future testMain() async { final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); builder.pushStyle(ui.TextStyle( fontSize: 10.0, - fontFamily: 'SomeOtherFontFamily', + fontFamily: 'Roboto', )); builder.addText('XXXX'); final ui.Paragraph paragraph = builder.build(); @@ -155,4 +156,22 @@ Future testMain() async { // FlutterTest font's 'X' character is a square, so it's the font size (10.0) * 4 characters. expect(metrics!.width, 40.0); }); + + test('uses specified font when debugEmulateFlutterTesterEnvironment is disabled', () { + debugEmulateFlutterTesterEnvironment = false; + + final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); + builder.pushStyle(ui.TextStyle( + fontSize: 14.0, + fontFamily: 'Roboto', + )); + builder.addText('XXXX'); + final ui.Paragraph paragraph = builder.build(); + paragraph.layout(const ui.ParagraphConstraints(width: 400000)); + + expect(paragraph.numberOfLines, 1); + + // Roboto has 2 points of leading around the font, whereas the test font does not. + expect(paragraph.height, 16); + }); } From d757bb7d760d8e8ba7f60df692b32f1881bf9972 Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Tue, 26 Mar 2024 10:15:18 -0700 Subject: [PATCH 4/4] Use width instead of height, which is more consistent across platforms. --- lib/web_ui/test/ui/line_metrics_test.dart | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/test/ui/line_metrics_test.dart b/lib/web_ui/test/ui/line_metrics_test.dart index 4bd9cbdc8bd4f..e287cb6d79cd6 100644 --- a/lib/web_ui/test/ui/line_metrics_test.dart +++ b/lib/web_ui/test/ui/line_metrics_test.dart @@ -162,16 +162,19 @@ Future testMain() async { final ui.ParagraphBuilder builder = ui.ParagraphBuilder(ui.ParagraphStyle()); builder.pushStyle(ui.TextStyle( - fontSize: 14.0, + fontSize: 16.0, fontFamily: 'Roboto', )); - builder.addText('XXXX'); + builder.addText('O'); final ui.Paragraph paragraph = builder.build(); - paragraph.layout(const ui.ParagraphConstraints(width: 400000)); + paragraph.layout(const ui.ParagraphConstraints(width: 400)); expect(paragraph.numberOfLines, 1); - // Roboto has 2 points of leading around the font, whereas the test font does not. - expect(paragraph.height, 16); - }); + final ui.LineMetrics? metrics = paragraph.getLineMetricsAt(0); + expect(metrics, isNotNull); + + // In Roboto, the width should be 11 here. In the test font, it would be square (16 points) + expect(metrics!.width, 11); + }, solo: true); }