From ff3b25abd041dc546855ef5ee4303dd9cdfffdea Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 6 Nov 2020 12:24:59 -0800 Subject: [PATCH] [web] Implement style inheritance during paragraph construction --- .../lib/src/engine/text/canvas_paragraph.dart | 162 +++++++++++++++++- lib/web_ui/lib/src/engine/text/paragraph.dart | 70 +++++++- .../text/canvas_paragraph_builder_test.dart | 96 +++++------ 3 files changed, 266 insertions(+), 62 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 a0c8eaefd862d..de266ad59dbe3 100644 --- a/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart @@ -300,6 +300,26 @@ abstract class StyleNode { /// The resolved text style is equivalent to the entire ascendent chain of /// parent style nodes. EngineTextStyle resolveStyle(); + + ui.Color? get _color; + ui.TextDecoration? get _decoration; + ui.Color? get _decorationColor; + ui.TextDecorationStyle? get _decorationStyle; + double? get _decorationThickness; + ui.FontWeight? get _fontWeight; + ui.FontStyle? get _fontStyle; + ui.TextBaseline? get _textBaseline; + String? get _fontFamily; + List? get _fontFamilyFallback; + List? get _fontFeatures; + double? get _fontSize; + double? get _letterSpacing; + double? get _wordSpacing; + double? get _height; + ui.Locale? get _locale; + ui.Paint? get _background; + ui.Paint? get _foreground; + List? get _shadows; } /// Represents a non-root [StyleNode]. @@ -315,9 +335,91 @@ class ChildStyleNode extends StyleNode { @override EngineTextStyle resolveStyle() { - // TODO(mdebbar): combine all styles from the parent hierarchy. - return style; + return EngineTextStyle( + color: _color, + decoration: _decoration, + decorationColor: _decorationColor, + decorationStyle: _decorationStyle, + decorationThickness: _decorationThickness, + fontWeight: _fontWeight, + fontStyle: _fontStyle, + textBaseline: _textBaseline, + fontFamily: _fontFamily, + fontFamilyFallback: _fontFamilyFallback, + fontFeatures: _fontFeatures, + fontSize: _fontSize, + letterSpacing: _letterSpacing, + wordSpacing: _wordSpacing, + height: _height, + locale: _locale, + background: _background, + foreground: _foreground, + shadows: _shadows, + ); } + + // Read these properties from the TextStyle associated with this node. If the + // property isn't defined, go to the parent node. + + @override + ui.Color? get _color => style._color ?? parent._color; + + @override + ui.TextDecoration? get _decoration => style._decoration ?? parent._decoration; + + @override + ui.Color? get _decorationColor => style._decorationColor ?? parent._decorationColor; + + @override + ui.TextDecorationStyle? get _decorationStyle => style._decorationStyle ?? parent._decorationStyle; + + @override + double? get _decorationThickness => style._decorationThickness ?? parent._decorationThickness; + + @override + ui.FontWeight? get _fontWeight => style._fontWeight ?? parent._fontWeight; + + @override + ui.FontStyle? get _fontStyle => style._fontStyle ?? parent._fontStyle; + + @override + ui.TextBaseline? get _textBaseline => style._textBaseline ?? parent._textBaseline; + + @override + List? get _fontFamilyFallback => style._fontFamilyFallback ?? parent._fontFamilyFallback; + + @override + List? get _fontFeatures => style._fontFeatures ?? parent._fontFeatures; + + @override + double? get _fontSize => style._fontSize ?? parent._fontSize; + + @override + double? get _letterSpacing => style._letterSpacing ?? parent._letterSpacing; + + @override + double? get _wordSpacing => style._wordSpacing ?? parent._wordSpacing; + + @override + double? get _height => style._height ?? parent._height; + + @override + ui.Locale? get _locale => style._locale ?? parent._locale; + + @override + ui.Paint? get _background => style._background ?? parent._background; + + @override + ui.Paint? get _foreground => style._foreground ?? parent._foreground; + + @override + List? get _shadows => style._shadows ?? parent._shadows; + + // Font family is slightly different from the other properties above. It's + // never null on the TextStyle object, so we use `_isFontFamilyProvided` to + // check if font family is defined or not. + @override + String? get _fontFamily => style._isFontFamilyProvided ? style._fontFamily : parent._fontFamily; } /// The root style node for the paragraph. @@ -342,6 +444,62 @@ class RootStyleNode extends StyleNode { } return style; } + + @override + ui.Color? get _color => null; + + @override + ui.TextDecoration? get _decoration => null; + + @override + ui.Color? get _decorationColor => null; + + @override + ui.TextDecorationStyle? get _decorationStyle => null; + + @override + double? get _decorationThickness => null; + + @override + ui.FontWeight? get _fontWeight => paragraphStyle._fontWeight; + @override + ui.FontStyle? get _fontStyle => paragraphStyle._fontStyle; + + @override + ui.TextBaseline? get _textBaseline => null; + + @override + String? get _fontFamily => paragraphStyle._fontFamily; + + @override + List? get _fontFamilyFallback => null; + + @override + List? get _fontFeatures => null; + + @override + double? get _fontSize => paragraphStyle._fontSize; + + @override + double? get _letterSpacing => null; + + @override + double? get _wordSpacing => null; + + @override + double? get _height => paragraphStyle._height; + + @override + ui.Locale? get _locale => paragraphStyle._locale; + + @override + ui.Paint? get _background => null; + + @override + ui.Paint? get _foreground => null; + + @override + List? get _shadows => null; } /// Builds a [CanvasParagraph] containing text with the given styling diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index ec3a59bcfb106..d0d16528867ab 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -894,7 +894,38 @@ class EngineParagraphStyle implements ui.ParagraphStyle { /// The web implementation of [ui.TextStyle]. class EngineTextStyle implements ui.TextStyle { - EngineTextStyle({ + /// Constructs an [EngineTextStyle] with all properties being required. + /// + /// This is good for call sites that need to be updated whenever a new + /// property is added to [EngineTextStyle]. Non-updated call sites will fail + /// the build otherwise. + factory EngineTextStyle({ + required ui.Color? color, + required ui.TextDecoration? decoration, + required ui.Color? decorationColor, + required ui.TextDecorationStyle? decorationStyle, + required double? decorationThickness, + required ui.FontWeight? fontWeight, + required ui.FontStyle? fontStyle, + required ui.TextBaseline? textBaseline, + required String? fontFamily, + required List? fontFamilyFallback, + required double? fontSize, + required double? letterSpacing, + required double? wordSpacing, + required double? height, + required ui.Locale? locale, + required ui.Paint? background, + required ui.Paint? foreground, + required List? shadows, + required List? fontFeatures, + }) = EngineTextStyle.only; + + /// Constructs an [EngineTextStyle] with only the given properties. + /// + /// This constructor should be used sparingly in tests, for example. Or when + /// we know for sure that not all properties are needed. + EngineTextStyle.only({ ui.Color? color, ui.TextDecoration? decoration, ui.Color? decorationColor, @@ -940,14 +971,20 @@ class EngineTextStyle implements ui.TextStyle { _foreground = foreground, _shadows = shadows; - factory EngineTextStyle.fromParagraphStyle(EngineParagraphStyle paragraphStyle) => EngineTextStyle( - fontWeight: paragraphStyle._fontWeight, - fontStyle: paragraphStyle._fontStyle, - fontFamily: paragraphStyle._fontFamily, - fontSize: paragraphStyle._fontSize, - height: paragraphStyle._height, - locale: paragraphStyle._locale, - ); + /// Constructs an [EngineTextStyle] by reading properties from an + /// [EngineParagraphStyle]. + factory EngineTextStyle.fromParagraphStyle( + EngineParagraphStyle paragraphStyle, + ) { + return EngineTextStyle.only( + fontWeight: paragraphStyle._fontWeight, + fontStyle: paragraphStyle._fontStyle, + fontFamily: paragraphStyle._fontFamily, + fontSize: paragraphStyle._fontSize, + height: paragraphStyle._height, + locale: paragraphStyle._locale, + ); + } final ui.Color? _color; final ui.TextDecoration? _decoration; @@ -1281,10 +1318,13 @@ class DomParagraphBuilder implements ui.ParagraphBuilder { ui.TextDecoration? decoration; ui.Color? decorationColor; ui.TextDecorationStyle? decorationStyle; + double? decorationThickness; ui.FontWeight? fontWeight = _paragraphStyle._fontWeight; ui.FontStyle? fontStyle = _paragraphStyle._fontStyle; ui.TextBaseline? textBaseline; String fontFamily = _paragraphStyle._fontFamily ?? DomRenderer.defaultFontFamily; + List? fontFamilyFallback; + List? fontFeatures; double fontSize = _paragraphStyle._fontSize ?? DomRenderer.defaultFontSize; final ui.TextAlign textAlign = _paragraphStyle._effectiveTextAlign; final ui.TextDirection textDirection = _paragraphStyle._effectiveTextDirection; @@ -1316,6 +1356,9 @@ class DomParagraphBuilder implements ui.ParagraphBuilder { if (style._decorationStyle != null) { decorationStyle = style._decorationStyle; } + if (style._decorationThickness != null) { + decorationThickness = style._decorationThickness; + } if (style._fontWeight != null) { fontWeight = style._fontWeight; } @@ -1326,6 +1369,12 @@ class DomParagraphBuilder implements ui.ParagraphBuilder { textBaseline = style._textBaseline; } fontFamily = style._fontFamily; + if (style._fontFamilyFallback != null) { + fontFamilyFallback = style._fontFamilyFallback; + } + if (style._fontFeatures != null) { + fontFeatures = style._fontFeatures; + } if (style._fontSize != null) { fontSize = style._fontSize!; } @@ -1358,10 +1407,13 @@ class DomParagraphBuilder implements ui.ParagraphBuilder { decoration: decoration, decorationColor: decorationColor, decorationStyle: decorationStyle, + decorationThickness: decorationThickness, fontWeight: fontWeight, fontStyle: fontStyle, textBaseline: textBaseline, fontFamily: fontFamily, + fontFamilyFallback: fontFamilyFallback, + fontFeatures: fontFeatures, fontSize: fontSize, letterSpacing: letterSpacing, wordSpacing: wordSpacing, 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 c61ae96fcb89f..2f354a6b35c63 100644 --- a/lib/web_ui/test/text/canvas_paragraph_builder_test.dart +++ b/lib/web_ui/test/text/canvas_paragraph_builder_test.dart @@ -35,7 +35,7 @@ void testMain() { expect(span, isA()); final FlatTextSpan textSpan = span as FlatTextSpan; expect(textSpan.text, 'Hello'); - expect(textSpan.style, EngineTextStyle(fontSize: 13.0)); + expect(textSpan.style, TextStyle(fontSize: 13.0)); }); test('Builds a single-span paragraph with complex styles', () { @@ -57,17 +57,16 @@ void testMain() { final FlatTextSpan span = paragraph.spans.single as FlatTextSpan; expect(span.text, 'Hello'); - // TODO(mdebbar): Uncomment this once style resolution is implemented. - // expect( - // span.style, - // TextStyle( - // height: 1.5, - // fontSize: 9.0, - // fontWeight: FontWeight.bold, - // fontStyle: FontStyle.italic, - // letterSpacing: 2.0, - // ), - // ); + expect( + span.style, + TextStyle( + height: 1.5, + fontSize: 9.0, + fontWeight: FontWeight.bold, + fontStyle: FontStyle.italic, + letterSpacing: 2.0, + ), + ); }); test('Builds a multi-span paragraph', () { @@ -86,25 +85,23 @@ void testMain() { final FlatTextSpan hello = paragraph.spans.first as FlatTextSpan; expect(hello.text, 'Hello'); - // TODO(mdebbar): Uncomment this once style resolution is implemented. - // expect( - // hello.style, - // TextStyle( - // fontSize: 13.0, - // fontWeight: FontWeight.bold, - // ), - // ); + expect( + hello.style, + TextStyle( + fontSize: 13.0, + fontWeight: FontWeight.bold, + ), + ); final FlatTextSpan world = paragraph.spans.last as FlatTextSpan; expect(world.text, ' world'); - // TODO(mdebbar): Uncomment this once style resolution is implemented. - // expect( - // world.style, - // TextStyle( - // fontSize: 13.0, - // fontStyle: FontStyle.italic, - // ), - // ); + expect( + world.style, + TextStyle( + fontSize: 13.0, + fontStyle: FontStyle.italic, + ), + ); }); test('Builds a multi-span paragraph with complex styles', () { @@ -126,34 +123,31 @@ void testMain() { final FlatTextSpan hello = paragraph.spans[0] as FlatTextSpan; expect(hello.text, 'Hello'); - // TODO(mdebbar): Uncomment this once style resolution is implemented. - // expect( - // hello.style, - // TextStyle(fontSize: 13.0, fontWeight: FontWeight.bold, height: 2.0), - // ); + expect( + hello.style, + TextStyle(fontSize: 13.0, fontWeight: FontWeight.bold, height: 2.0), + ); final FlatTextSpan world = paragraph.spans[1] as FlatTextSpan; expect(world.text, ' world'); - // TODO(mdebbar): Uncomment this once style resolution is implemented. - // expect( - // world.style, - // TextStyle( - // fontSize: 13.0, - // fontWeight: FontWeight.bold, - // fontStyle: FontStyle.italic, - // ), - // ); + expect( + world.style, + TextStyle( + fontSize: 13.0, + fontWeight: FontWeight.bold, + fontStyle: FontStyle.italic, + ), + ); final FlatTextSpan bang = paragraph.spans[2] as FlatTextSpan; expect(bang.text, '!'); - // TODO(mdebbar): Uncomment this once style resolution is implemented. - // expect( - // bang.style, - // TextStyle( - // fontSize: 13.0, - // fontWeight: FontWeight.normal, - // fontStyle: FontStyle.italic, - // ), - // ); + expect( + bang.style, + TextStyle( + fontSize: 13.0, + fontWeight: FontWeight.normal, + fontStyle: FontStyle.italic, + ), + ); }); }