Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Reland "Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087" #16155

Merged
merged 5 commits into from
Jan 29, 2020
Merged

Reland "Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087" #16155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa8ae60
Revert "Revert breaking PRs (#16148)"
GaryQian Jan 28, 2020
d64b7b2
Remove extra copy of the string
GaryQian Jan 28, 2020
feb2d1d
Move dart:ui only tests directly into the engine.
GaryQian Jan 28, 2020
1c83e45
Formatter
GaryQian Jan 28, 2020
28ce923
Fix typos
GaryQian Jan 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 115 additions & 12 deletions lib/ui/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,94 @@ enum TextDecorationStyle {
wavy
}

/// {@template flutter.dart:ui.textHeightBehavior}
/// Defines how the paragraph will apply [TextStyle.height] to the ascent of the
/// first line and descent of the last line.
///
/// Each boolean value represents whether the [TextStyle.height] modifier will
/// be applied to the corresponding metric. By default, all properties are true,
/// and [TextStyle.height] is applied as normal. When set to false, the font's
/// default ascent will be used.
/// {@endtemplate}
class TextHeightBehavior {

/// Creates a new TextHeightBehavior object.
///
/// * applyHeightToFirstAscent: When true, the [TextStyle.height] modifier
/// will be applied to the ascent of the first line. When false, the font's
/// default ascent will be used.
/// * applyHeightToLastDescent: When true, the [TextStyle.height] modifier
/// will be applied to the descent of the last line. When false, the font's
/// default descent will be used.
///
/// All properties default to true (height modifications applied as normal).
const TextHeightBehavior({
this.applyHeightToFirstAscent = true,
this.applyHeightToLastDescent = true,
});

/// Creates a new TextHeightBehavior object from an encoded form.
///
/// See [encode] for the creation of the encoded form.
const TextHeightBehavior.fromEncoded(int encoded) : applyHeightToFirstAscent = (encoded & 0x1) == 0,
applyHeightToLastDescent = (encoded & 0x2) == 0;


/// Whether to apply the [TextStyle.height] modifier to the ascent of the first
/// line in the paragraph.
///
/// When true, the [TextStyle.height] modifier will be applied to to the ascent
/// of the first line. When false, the font's default ascent will be used and
/// the [TextStyle.height] will have no effect on the ascent of the first line.
///
/// This property only has effect if a non-null [TextStyle.height] is specified.
///
/// Defaults to true (height modifications applied as normal).
final bool applyHeightToFirstAscent;

/// Whether to apply the [TextStyle.height] modifier to the descent of the last
/// line in the paragraph.
///
/// When true, the [TextStyle.height] modifier will be applied to to the descent
/// of the last line. When false, the font's default descent will be used and
/// the [TextStyle.height] will have no effect on the descent of the last line.
///
/// This property only has effect if a non-null [TextStyle.height] is specified.
///
/// Defaults to true (height modifications applied as normal).
final bool applyHeightToLastDescent;

/// Returns an encoded int representation of this object.
int encode() {
return (applyHeightToFirstAscent ? 0 : 1 << 0) | (applyHeightToLastDescent ? 0 : 1 << 1);
}

@override
bool operator ==(dynamic other) {
if (other.runtimeType != runtimeType)
return false;
return other is TextHeightBehavior
&& other.applyHeightToFirstAscent == applyHeightToFirstAscent
&& other.applyHeightToLastDescent == applyHeightToLastDescent;
}

@override
int get hashCode {
return hashValues(
applyHeightToFirstAscent,
applyHeightToLastDescent,
);
}

@override
String toString() {
return 'TextHeightBehavior('
'applyHeightToFirstAscent: $applyHeightToFirstAscent, '
'applyHeightToLastDescent: $applyHeightToLastDescent'
')';
}
}

/// Determines if lists [a] and [b] are deep equivalent.
///
/// Returns true if the lists are both null, or if they are both non-null, have
Expand Down Expand Up @@ -746,20 +834,23 @@ class TextStyle {
//
// - Element 5: The value of |maxLines|.
//
// - Element 6: The encoded value of |textHeightBehavior|.
//
Int32List _encodeParagraphStyle(
TextAlign textAlign,
TextDirection textDirection,
int maxLines,
String fontFamily,
double fontSize,
double height,
TextHeightBehavior textHeightBehavior,
FontWeight fontWeight,
FontStyle fontStyle,
StrutStyle strutStyle,
String ellipsis,
Locale locale,
) {
final Int32List result = Int32List(6); // also update paragraph_builder.cc
final Int32List result = Int32List(7); // also update paragraph_builder.cc
if (textAlign != null) {
result[0] |= 1 << 1;
result[1] = textAlign.index;
Expand All @@ -780,28 +871,32 @@ Int32List _encodeParagraphStyle(
result[0] |= 1 << 5;
result[5] = maxLines;
}
if (fontFamily != null) {
if (textHeightBehavior != null) {
result[0] |= 1 << 6;
result[6] = textHeightBehavior.encode();
}
if (fontFamily != null) {
result[0] |= 1 << 7;
// Passed separately to native.
}
if (fontSize != null) {
result[0] |= 1 << 7;
result[0] |= 1 << 8;
// Passed separately to native.
}
if (height != null) {
result[0] |= 1 << 8;
result[0] |= 1 << 9;
// Passed separately to native.
}
if (strutStyle != null) {
result[0] |= 1 << 9;
result[0] |= 1 << 10;
// Passed separately to native.
}
if (ellipsis != null) {
result[0] |= 1 << 10;
result[0] |= 1 << 11;
// Passed separately to native.
}
if (locale != null) {
result[0] |= 1 << 11;
result[0] |= 1 << 12;
// Passed separately to native.
}
return result;
Expand Down Expand Up @@ -842,6 +937,9 @@ class ParagraphStyle {
/// the line height to take the height as defined by the font, which may not
/// be exactly the height of the `fontSize`.
///
/// * `textHeightBehavior`: Specifies how the `height` multiplier is
/// applied to ascent of the first line and the descent of the last line.
///
/// * `fontWeight`: The typeface thickness to use when painting the text
/// (e.g., bold).
///
Expand Down Expand Up @@ -869,6 +967,7 @@ class ParagraphStyle {
String fontFamily,
double fontSize,
double height,
TextHeightBehavior textHeightBehavior,
FontWeight fontWeight,
FontStyle fontStyle,
StrutStyle strutStyle,
Expand All @@ -881,6 +980,7 @@ class ParagraphStyle {
fontFamily,
fontSize,
height,
textHeightBehavior,
fontWeight,
fontStyle,
strutStyle,
Expand Down Expand Up @@ -929,11 +1029,14 @@ class ParagraphStyle {
'fontWeight: ${ _encoded[0] & 0x008 == 0x008 ? FontWeight.values[_encoded[3]] : "unspecified"}, '
'fontStyle: ${ _encoded[0] & 0x010 == 0x010 ? FontStyle.values[_encoded[4]] : "unspecified"}, '
'maxLines: ${ _encoded[0] & 0x020 == 0x020 ? _encoded[5] : "unspecified"}, '
'fontFamily: ${ _encoded[0] & 0x040 == 0x040 ? _fontFamily : "unspecified"}, '
'fontSize: ${ _encoded[0] & 0x080 == 0x080 ? _fontSize : "unspecified"}, '
'height: ${ _encoded[0] & 0x100 == 0x100 ? "${_height}x" : "unspecified"}, '
'ellipsis: ${ _encoded[0] & 0x200 == 0x200 ? "\"$_ellipsis\"" : "unspecified"}, '
'locale: ${ _encoded[0] & 0x400 == 0x400 ? _locale : "unspecified"}'
'textHeightBehavior: ${
_encoded[0] & 0x040 == 0x040 ?
TextHeightBehavior.fromEncoded(_encoded[6]).toString() : "unspecified"}, '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update framework side tests this would break before landing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make the framework tests fail until this can roll, which seems to result in more tree-redness than just having autoroller be stopped.

Copy link
Contributor Author

@GaryQian GaryQian Jan 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also still block skia->engine since engine and framework tests are circularly dependent now.

*actually that may not be true, not sure what happens if framework is red, but engine framework tests can pass

'fontFamily: ${ _encoded[0] & 0x080 == 0x080 ? _fontFamily : "unspecified"}, '
'fontSize: ${ _encoded[0] & 0x100 == 0x100 ? _fontSize : "unspecified"}, '
'height: ${ _encoded[0] & 0x200 == 0x200 ? "${_height}x" : "unspecified"}, '
'ellipsis: ${ _encoded[0] & 0x400 == 0x400 ? "\"$_ellipsis\"" : "unspecified"}, '
'locale: ${ _encoded[0] & 0x800 == 0x800 ? _locale : "unspecified"}'
')';
}
}
Expand Down
18 changes: 12 additions & 6 deletions lib/ui/text/paragraph_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ const int psTextDirectionIndex = 2;
const int psFontWeightIndex = 3;
const int psFontStyleIndex = 4;
const int psMaxLinesIndex = 5;
const int psFontFamilyIndex = 6;
const int psFontSizeIndex = 7;
const int psHeightIndex = 8;
const int psStrutStyleIndex = 9;
const int psEllipsisIndex = 10;
const int psLocaleIndex = 11;
const int psTextHeightBehaviorIndex = 6;
const int psFontFamilyIndex = 7;
const int psFontSizeIndex = 8;
const int psHeightIndex = 9;
const int psStrutStyleIndex = 10;
const int psEllipsisIndex = 11;
const int psLocaleIndex = 12;

const int psTextAlignMask = 1 << psTextAlignIndex;
const int psTextDirectionMask = 1 << psTextDirectionIndex;
Expand All @@ -90,6 +91,7 @@ const int psMaxLinesMask = 1 << psMaxLinesIndex;
const int psFontFamilyMask = 1 << psFontFamilyIndex;
const int psFontSizeMask = 1 << psFontSizeIndex;
const int psHeightMask = 1 << psHeightIndex;
const int psTextHeightBehaviorMask = 1 << psTextHeightBehaviorIndex;
const int psStrutStyleMask = 1 << psStrutStyleIndex;
const int psEllipsisMask = 1 << psEllipsisIndex;
const int psLocaleMask = 1 << psLocaleIndex;
Expand Down Expand Up @@ -265,6 +267,10 @@ ParagraphBuilder::ParagraphBuilder(
style.has_height_override = true;
}

if (mask & psTextHeightBehaviorMask) {
style.text_height_behavior = encoded[psTextHeightBehaviorIndex];
}

if (mask & psStrutStyleMask) {
decodeStrut(strutData, strutFontFamilies, style);
}
Expand Down
7 changes: 7 additions & 0 deletions lib/web_ui/lib/src/engine/compositor/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class SkParagraphStyle implements ui.ParagraphStyle {
String fontFamily,
double fontSize,
double height,
ui.TextHeightBehavior textHeightBehavior,
ui.FontWeight fontWeight,
ui.FontStyle fontStyle,
ui.StrutStyle strutStyle,
Expand All @@ -38,6 +39,7 @@ class SkParagraphStyle implements ui.ParagraphStyle {
fontFamily,
fontSize,
height,
textHeightBehavior,
fontWeight,
fontStyle,
ellipsis,
Expand Down Expand Up @@ -85,6 +87,7 @@ class SkParagraphStyle implements ui.ParagraphStyle {
String fontFamily,
double fontSize,
double height,
ui.TextHeightBehavior textHeightBehavior,
ui.FontWeight fontWeight,
ui.FontStyle fontStyle,
String ellipsis,
Expand Down Expand Up @@ -129,6 +132,10 @@ class SkParagraphStyle implements ui.ParagraphStyle {
skParagraphStyle['heightMultiplier'] = height;
}

if (textHeightBehavior != null) {
skParagraphStyle['textHeightBehavior'] = textHeightBehavior.encode();
}

if (maxLines != null) {
skParagraphStyle['maxLines'] = maxLines;
}
Expand Down
22 changes: 20 additions & 2 deletions lib/web_ui/lib/src/engine/text/paragraph.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle {
String fontFamily,
double fontSize,
double height,
ui.TextHeightBehavior textHeightBehavior,
ui.FontWeight fontWeight,
ui.FontStyle fontStyle,
ui.StrutStyle strutStyle,
Expand All @@ -470,6 +471,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle {
_fontFamily = fontFamily,
_fontSize = fontSize,
_height = height,
_textHeightBehavior = textHeightBehavior,
// TODO(b/128317744): add support for strut style.
_strutStyle = strutStyle,
_ellipsis = ellipsis,
Expand All @@ -483,6 +485,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle {
final String _fontFamily;
final double _fontSize;
final double _height;
final ui.TextHeightBehavior _textHeightBehavior;
final EngineStrutStyle _strutStyle;
final String _ellipsis;
final ui.Locale _locale;
Expand Down Expand Up @@ -536,13 +539,27 @@ class EngineParagraphStyle implements ui.ParagraphStyle {
_fontFamily == typedOther._fontFamily ||
_fontSize == typedOther._fontSize ||
_height == typedOther._height ||
_textHeightBehavior == typedOther._textHeightBehavior ||
_ellipsis == typedOther._ellipsis ||
_locale == typedOther._locale;
}

@override
int get hashCode =>
ui.hashValues(_fontFamily, _fontSize, _height, _ellipsis, _locale);
int get hashCode {
return ui.hashValues(
_textAlign,
_textDirection,
_fontWeight,
_fontStyle,
_maxLines,
_fontFamily,
_fontSize,
_height,
_textHeightBehavior,
_ellipsis,
_locale
);
}

@override
String toString() {
Expand All @@ -553,6 +570,7 @@ class EngineParagraphStyle implements ui.ParagraphStyle {
'fontWeight: ${_fontWeight ?? "unspecified"}, '
'fontStyle: ${_fontStyle ?? "unspecified"}, '
'maxLines: ${_maxLines ?? "unspecified"}, '
'textHeightBehavior: ${_textHeightBehavior ?? "unspecified"}, '
'fontFamily: ${_fontFamily ?? "unspecified"}, '
'fontSize: ${_fontSize != null ? _fontSize.toStringAsFixed(1) : "unspecified"}, '
'height: ${_height != null ? "${_height.toStringAsFixed(1)}x" : "unspecified"}, '
Expand Down
Loading