From d89cf4fafe0de50af5ac539c6bcfb8b00934b2b9 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 2 Feb 2021 17:28:09 -0800 Subject: [PATCH 1/5] [web] Fix text rendering issue when paragraph style is ginormous --- .../lib/src/engine/text/canvas_paragraph.dart | 27 +++++++++++++++- .../engine/canvas_draw_image_golden_test.dart | 22 +++++++++---- .../engine/canvas_paragraph/general_test.dart | 32 +++++++++++++++++-- .../text/canvas_paragraph_builder_test.dart | 18 +++++------ 4 files changed, 80 insertions(+), 19 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart index 670a9b32cec50..5ef684206102c 100644 --- a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart @@ -133,7 +133,7 @@ class CanvasParagraph implements EngineParagraph { domRenderer.createElement('p') as html.HtmlElement; // 1. Set paragraph-level styles. - _applyParagraphStyleToElement(element: rootElement, style: paragraphStyle); + _applyNecessaryParagraphStyles(element: rootElement, style: paragraphStyle); final html.CssStyleDeclaration cssStyle = rootElement.style; cssStyle ..position = 'absolute' @@ -259,6 +259,31 @@ class CanvasParagraph implements EngineParagraph { } } +/// Applies a paragraph [style] to an [element], translating the properties to +/// their corresponding CSS equivalents. +/// +/// As opposed to [_applyParagraphStyleToElement], this method only applies +/// styles that are necessary at the paragraph level. Other styles (e.g. font +/// size) are always applied at the span level so they aren't needed at the +/// paragraph level. +void _applyNecessaryParagraphStyles({ + required html.HtmlElement element, + required EngineParagraphStyle style, +}) { + final html.CssStyleDeclaration cssStyle = element.style; + + if (style._textAlign != null) { + cssStyle.textAlign = textAlignToCssValue( + style._textAlign, style._textDirection ?? ui.TextDirection.ltr); + } + if (style._lineHeight != null) { + cssStyle.lineHeight = '${style._lineHeight}'; + } + if (style._textDirection != null) { + cssStyle.direction = _textDirectionToCss(style._textDirection); + } +} + /// A common interface for all types of spans that make up a paragraph. /// /// These spans are stored as a flat list in the paragraph object. diff --git a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart index 4aa6ef99cbb18..2137367a03dd5 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_draw_image_golden_test.dart @@ -334,15 +334,18 @@ void testMain() async { // Yellow text should be behind image and rectangle. // Cyan text should be above everything. test('Paints text above and below image', () async { + // Use a non-Ahem font so that text is visible. + debugEmulateFlutterTesterEnvironment = false; final RecordingCanvas rc = RecordingCanvas(const Rect.fromLTRB(0, 0, 400, 300)); rc.save(); Image testImage = createTestImage(); double testWidth = testImage.width.toDouble(); double testHeight = testImage.height.toDouble(); + final Color orange = Color(0xFFFF9800); final Paragraph paragraph1 = createTestParagraph( - 'should be below...............', - color: Color(0xFFFFFF40)); + 'Should be below below below below below', + color: orange); paragraph1.layout(const ParagraphConstraints(width: 400.0)); rc.drawParagraph(paragraph1, const Offset(20, 100)); rc.drawImageRect(testImage, Rect.fromLTRB(0, 0, testWidth, testHeight), @@ -352,14 +355,19 @@ void testMain() async { Paint() ..strokeWidth = 3 ..color = Color(0xA0000000)); + final Color cyan = Color(0xFF0097A7); final Paragraph paragraph2 = createTestParagraph( - 'Should be above...............', - color: Color(0xFF00FFFF)); + 'Should be above above above above above', + color: cyan); paragraph2.layout(const ParagraphConstraints(width: 400.0)); rc.drawParagraph(paragraph2, const Offset(20, 150)); rc.restore(); - await _checkScreenshot(rc, 'draw_text_composite_order_below', - maxDiffRatePercent: 1.0); + await _checkScreenshot( + rc, + 'draw_text_composite_order_below', + maxDiffRatePercent: 1.0, + region: Rect.fromLTWH(0, 0, 350, 300), + ); }); // Creates a picture @@ -723,7 +731,7 @@ HtmlImage createTestImage({int width = 100, int height = 50}) { Paragraph createTestParagraph(String text, {Color color = const Color(0xFF000000)}) { final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle( - fontFamily: 'Ahem', + fontFamily: 'Roboto', fontStyle: FontStyle.normal, fontWeight: FontWeight.normal, fontSize: 14.0, diff --git a/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart index 8c4538897ddb0..3f3a9bd94d56f 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart @@ -174,8 +174,7 @@ void testMain() async { canvas.save(); canvas.translate(offset.dx, offset.dy); canvas.rotate(math.pi / 4); - final Rect rect = - Rect.fromLTRB(0.0, 0.0, 150.0, paragraph.height); + final Rect rect = Rect.fromLTRB(0.0, 0.0, 150.0, paragraph.height); canvas.drawRect(rect, SurfacePaintData()..color = black); canvas.drawParagraph(paragraph, Offset.zero); canvas.restore(); @@ -198,6 +197,35 @@ void testMain() async { return takeScreenshot(canvas, bounds, 'canvas_paragraph_align_transform_dom'); }); + void testGiantParagraphStyles(EngineCanvas canvas) { + final CanvasParagraph paragraph = rich( + ParagraphStyle(fontFamily: 'monospace', fontSize: 80.0), + (CanvasParagraphBuilder builder) { + builder.pushStyle(EngineTextStyle.only(color: yellow, fontSize: 24.0)); + builder.addText('Lorem '); + builder.pushStyle(EngineTextStyle.only(color: red, fontSize: 32.0)); + builder.addText('ipsum'); + }, + )..layout(constrain(double.infinity)); + final Rect rect = Rect.fromLTRB(0.0, 0.0, paragraph.maxIntrinsicWidth, paragraph.height); + canvas.drawRect(rect, SurfacePaintData()..color = black); + canvas.drawParagraph(paragraph, Offset.zero); + } + + test('giant paragraph style', () { + const Rect bounds = Rect.fromLTWH(0, 0, 300, 200); + final canvas = BitmapCanvas(bounds, RenderStrategy()); + testGiantParagraphStyles(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_giant_paragraph_style'); + }); + + test('giant paragraph style (DOM)', () { + const Rect bounds = Rect.fromLTWH(0, 0, 300, 200); + final canvas = DomCanvas(domRenderer.createElement('flt-picture')); + testGiantParagraphStyles(canvas); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_giant_paragraph_style_dom'); + }); + test('paints spans with varying heights/baselines', () { final canvas = BitmapCanvas(bounds, RenderStrategy()); diff --git a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart index 8567abd18eeb9..9baad5a0f31f0 100644 --- a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart +++ b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart @@ -23,7 +23,7 @@ String get defaultFontFamily { const String defaultColor = 'color: rgb(255, 0, 0);'; const String defaultFontSize = 'font-size: 14px;'; final String paragraphStyle = - '$defaultFontFamily position: absolute; white-space: pre;'; + 'position: absolute; white-space: pre;'; void main() { internalBootstrapBrowserTest(() => testMain); @@ -50,7 +50,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: double.infinity)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'Hello' '' @@ -61,7 +61,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: 39.0)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'Hel
lo' '' @@ -168,7 +168,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: double.infinity)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'Hello' '' @@ -206,7 +206,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: double.infinity)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'Hello' '' @@ -220,7 +220,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: 75.0)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'Hello' '' @@ -271,7 +271,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: double.infinity)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'Hello' '' @@ -335,7 +335,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: double.infinity)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'First
Second ' '' @@ -349,7 +349,7 @@ void testMain() async { paragraph.layout(ParagraphConstraints(width: 180.0)); expect( paragraph.toDomElement().outerHtml, - '

' + '

' '' 'First
Second
' '' From 95e16b28464c9a63563d69fe4da5e7962992d2d7 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 3 Feb 2021 11:22:05 -0800 Subject: [PATCH 2/5] 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 4574022fa25e4..928d33c22e85d 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: 6839c709f859a1abe50e6322dfee17a3a3817c5c +revision: 85453fc8ed762fd0da5919267f1c94fa906737e6 From 23fedbeba34b01b219766ceb71a4fe9127890286 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 9 Feb 2021 15:47:00 -0800 Subject: [PATCH 3/5] 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 928d33c22e85d..f82ce8252aa3a 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: 85453fc8ed762fd0da5919267f1c94fa906737e6 +revision: 96e75232531e336309895fd20cea0cccafa20462 From a84a757fcaae6f2997dfde7854785591198f173b Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 10 Feb 2021 10:25:41 -0800 Subject: [PATCH 4/5] Use Roboto font --- lib/web_ui/dev/goldens_lock.yaml | 2 +- .../test/golden_tests/engine/canvas_paragraph/general_test.dart | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/dev/goldens_lock.yaml b/lib/web_ui/dev/goldens_lock.yaml index f82ce8252aa3a..c6d6a25dd35bc 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: 96e75232531e336309895fd20cea0cccafa20462 +revision: 92381b8ca48729e9b21c6b19dd39ed605a98d4ee diff --git a/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart index 3f3a9bd94d56f..f98cf1a8038dc 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart @@ -199,7 +199,7 @@ void testMain() async { void testGiantParagraphStyles(EngineCanvas canvas) { final CanvasParagraph paragraph = rich( - ParagraphStyle(fontFamily: 'monospace', fontSize: 80.0), + ParagraphStyle(fontFamily: 'Roboto', fontSize: 80.0), (CanvasParagraphBuilder builder) { builder.pushStyle(EngineTextStyle.only(color: yellow, fontSize: 24.0)); builder.addText('Lorem '); From 3cc1c098b12a5f514f2c04c6ed9685b1851b95ff Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 10 Feb 2021 11:04:59 -0800 Subject: [PATCH 5/5] slight increase in max diff rate to allow for minor screenshot discrepancies --- .../test/golden_tests/engine/canvas_paragraph/general_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart b/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart index f98cf1a8038dc..c67462816cff7 100644 --- a/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart +++ b/lib/web_ui/test/golden_tests/engine/canvas_paragraph/general_test.dart @@ -151,7 +151,7 @@ void testMain() async { canvas.drawParagraph(paragraph, offset); offset = offset.translate(0, paragraph.height + 10); - return takeScreenshot(canvas, bounds, 'canvas_paragraph_align_dom'); + return takeScreenshot(canvas, bounds, 'canvas_paragraph_align_dom', maxDiffRatePercent: 0.3); }); void testAlignAndTransform(EngineCanvas canvas) {