Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087

GaryQian · 2019-12-30T19:40:39Z

Part of a series of changes to resolve flutter/flutter#47175

Paired with the Framework-side changes: flutter/flutter#48346

Adds HeightBehavior to allow for adjustment of how vertical line height is applied for the first and last lines.

GaryQian · 2020-01-02T22:43:09Z

This may be landed independently from the framework side changes as it is backwards compatible and non-breaking.

GaryQian · 2020-01-03T03:05:43Z

Bugfix portion of this is extracted out in standalone PR: #15106

GaryQian · 2020-01-07T18:14:38Z

cc @Hixie for API review. The framework side PR would add boundaryLineHeightBehavior property to Text and RichText

GaryQian · 2020-01-07T23:29:54Z

Framework side changes: flutter/flutter#48346

GaryQian · 2020-01-08T00:49:01Z

Will require a manual roll to pass build_and_test_linux_unopt_debug.

ParagraphStyle toString tests need to be updated with new property.

Hixie · 2020-01-08T19:55:30Z

lib/ui/text.dart

@@ -443,6 +443,8 @@ class BoundaryLineHeightBehavior {
  ///  * last: 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.
+  ///
+  /// All properties default to true (height modificaitons applied as normal).


(also the cases below)

Hixie · 2020-01-08T19:56:59Z

lib/ui/text.dart

+  /// the [TextStyle.height] will have no effect on the ascent of the first line.
+  ///
+  /// Defaults to true (height modificaitons applied as normal).
+  final bool first;


"first" is not a yes/no question. It would be more intuitive with a name like "padFirstLine" or something.

or "applyAscentToFirstLine" and "applyDescentToLastLine" maybe

Changed to applyHeightToFirstLineAscent A bit long but very descriptive.

Hixie · 2020-01-08T19:57:06Z

lib/ui/text.dart

+  /// the [TextStyle.height] will have no effect on the descent of the last line.
+  ///
+  /// Defaults to true (height modificaitons applied as normal).
+  final bool last;


similar comment here

Hixie · 2020-01-08T19:57:25Z

lib/ui/text.dart

+
+  @override
+  bool operator ==(dynamic other) {
+    if (identical(this, other))


identical check shouldn't be necessary here

Hixie · 2020-01-08T19:58:00Z

lib/ui/text.dart

+  String toString() {
+    return 'BoundaryLineHeightBehavior('
+             'first: $first, '
+             'last: $last, '


when you write a test for the toString, you'll see you have a trailing comma. :-)

Hixie · 2020-01-08T20:00:11Z

lib/ui/text.dart

@@ -765,13 +844,14 @@ Int32List _encodeParagraphStyle(
  String fontFamily,


the comment on this method is out of date

Hixie · 2020-01-08T20:01:36Z

lib/ui/text.dart

@@ -881,6 +965,7 @@ class ParagraphStyle {
    String fontFamily,


docs on this constructor are out of date

GaryQian · 2020-01-27T21:15:04Z

Landing this will break engine->framework roller until flutter/flutter#48346 is also landed/manually rolled.

build_and_test_linux_unopt_debug will be red until framework PR is landed as well.

…)" This reverts commit cbf4536.

* Revert "Web PargraphStyle TextHeightBehavior integration (#16075)" This reverts commit 86682a2. * Revert "Engine/LibTxt/dart:ui impl of TextHeightBehavior (#15087)" This reverts commit cbf4536.

…ine#15087)

flutter/engine@c4229bf...ec32966 git log c4229bf..ec32966 --first-parent --oneline 2020-01-28 [email protected] Revert "Always make gpu thread different from platform thread regardless of platform view (#16068)" (flutter/engine#16161) 2020-01-28 [email protected] Roll fuchsia/sdk/core/mac-amd64 from gNitp... to 5fMtM... (flutter/engine#16153) 2020-01-28 [email protected] Revert "Disable setting a library tag handler." (flutter/engine#16157) 2020-01-28 [email protected] Always make gpu thread different from platform thread regardless of platform view (flutter/engine#16068) 2020-01-28 [email protected] Add test to ensure that concurrent message loops have at least one workers. (flutter/engine#16074) 2020-01-28 [email protected] Revert breaking PRs (flutter/engine#16148) 2020-01-28 [email protected] Roll fuchsia/sdk/core/linux-amd64 from 8Ns10... to 2rLoq... (flutter/engine#15971) 2020-01-28 [email protected] Gets the DPI for all awareness mode and older Windows versions (flutter/engine#15951) 2020-01-28 [email protected] Hold a mutex when updating all CanPostTaskToAllNativeThreads::Captures members. (flutter/engine#16085) 2020-01-28 [email protected] Disable setting a library tag handler. (flutter/engine#16086) 2020-01-28 [email protected] Web PargraphStyle TextHeightBehavior integration (flutter/engine#16075) 2020-01-28 [email protected] Fix flake by making thread ID tracking in CanPostTaskToAllNativeThreads thread safe. (flutter/engine#16081) 2020-01-28 [email protected] Remove buggy assertion in EmbedderTest::CanPostTaskToAllNativeThreads. (flutter/engine#16071) 2020-01-27 [email protected] Remove tonic/platform. (flutter/engine#16062) 2020-01-27 [email protected] Allow embedders to schedule a callback on all engine managed threads. (flutter/engine#15980) 2020-01-27 [email protected] Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/engine#15087) 2020-01-27 [email protected] Roll src/third_party/skia f1b2b42613cb..9c1d30dd163e (8 commits) (flutter/engine#16060) 2020-01-27 [email protected] Remove stale recipe changelog. (flutter/engine#15985) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

)

… (flutter#16155)

* Revert "Web PargraphStyle TextHeightBehavior integration (flutter#16075)" This reverts commit 86682a2. * Revert "Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter#15087)" This reverts commit cbf4536.

… (flutter#16155)

…ter#15087" (flutter#16155)" This reverts commit b7ccc90.

…)" This reverts commit 5938439.

GaryQian added brand new feature affects: engine affects: text input Work in progress (WIP) Not ready (yet) for review! labels Dec 30, 2019

GaryQian self-assigned this Dec 30, 2019

auto-assign bot requested a review from gw280 December 30, 2019 19:40

googlebot added the cla: yes label Dec 30, 2019

GaryQian removed the request for review from gw280 December 30, 2019 19:48

GaryQian mentioned this pull request Dec 30, 2019

[Text] Allow for line height *between* lines on text. flutter/flutter#47175

Closed

GaryQian removed the Work in progress (WIP) Not ready (yet) for review! label Jan 3, 2020

GaryQian changed the title ~~[WIP] Engine/LibTxt/dart:ui support for boundary line height behavior~~ Engine/LibTxt/dart:ui support for boundary line height behavior Jan 3, 2020

cbracken requested a review from jason-simmons January 6, 2020 19:16

GaryQian force-pushed the lineheight branch from dfc6029 to d911ece Compare January 6, 2020 23:18

GaryQian requested a review from johnsonmh January 6, 2020 23:20

johnsonmh approved these changes Jan 7, 2020

View reviewed changes

GaryQian requested a review from Hixie January 7, 2020 18:13

GaryQian force-pushed the lineheight branch from 5a0eda4 to 565e2b2 Compare January 7, 2020 20:29

GaryQian mentioned this pull request Jan 7, 2020

Expose TextHeightBehavior in Text, RichText, and DefaultTextStyle. flutter/flutter#48346

Merged

9 tasks

Hixie reviewed Jan 8, 2020

View reviewed changes

lib/ui/text.dart

@@ -881,6 +965,7 @@ class ParagraphStyle {

String fontFamily,

Copy link

Contributor

Hixie Jan 8, 2020

Choose a reason for hiding this comment

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

docs on this constructor are out of date

Fix dart unittests

abbaafb

GaryQian requested a review from jason-simmons January 24, 2020 00:28

jason-simmons approved these changes Jan 24, 2020

View reviewed changes

GaryQian merged commit cbf4536 into flutter:master Jan 27, 2020

GaryQian mentioned this pull request Jan 28, 2020

Web PargraphStyle TextHeightBehavior integration #16075

Merged

dnfield added a commit to dnfield/engine that referenced this pull request Jan 28, 2020

Revert "Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter#15087…

5f8e75a

…)" This reverts commit cbf4536.

dnfield mentioned this pull request Jan 28, 2020

Revert breaking PRs #16148

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2020

cbf4536 Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/eng…

6b6a1ed

…ine#15087)

This was referenced Jan 28, 2020

Roll engine c4229bfbbae4..b4fc4b5ab343 (12 commits) flutter/flutter#49630

Closed

Roll engine c4229bfbbae4..b4fc4b5ab343 (12 commits) flutter/flutter#49632

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2020

cbf4536 Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/eng…

59bd7b5

…ine#15087)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2020

cbf4536 Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/eng…

0062d7f

…ine#15087)

This was referenced Jan 28, 2020

Roll engine c4229bfbbae4..b4fc4b5ab343 (12 commits) flutter/flutter#49636

Closed

Roll engine c4229bfbbae4..53baa7aee535 (13 commits) flutter/flutter#49653

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 29, 2020

cbf4536 Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/eng…

21c918c

…ine#15087)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 29, 2020

cbf4536 Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/eng…

9f08159

…ine#15087)

engine-flutter-autoroll mentioned this pull request Jan 29, 2020

Roll engine c4229bfbbae4..53baa7aee535 (13 commits) flutter/flutter#49655

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 29, 2020

cbf4536 Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/eng…

f9fa647

…ine#15087)

engine-flutter-autoroll mentioned this pull request Jan 29, 2020

Roll engine c4229bfbbae4..ec32966e030a (18 commits) flutter/flutter#49656

Merged

GaryQian added a commit that referenced this pull request Jan 29, 2020

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

ee297aa

)

GaryQian mentioned this pull request Jan 29, 2020

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

Merged

stuartmorgan-g pushed a commit to stuartmorgan-g/engine that referenced this pull request Jan 30, 2020

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

ebd7905

… (flutter#16155)

NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter#15087)

5938439

NoamDev pushed a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

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

b7ccc90

… (flutter#16155)

NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Revert "Reland "Engine/LibTxt/dart:ui impl of TextHeightBehavior flut…

e134761

…ter#15087" (flutter#16155)" This reverts commit b7ccc90.

NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020

Revert "Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter#15087…

be2f2da

…)" This reverts commit 5938439.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087

Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087

GaryQian commented Dec 30, 2019 •

edited

Loading

GaryQian commented Jan 2, 2020

GaryQian commented Jan 3, 2020

GaryQian commented Jan 7, 2020

GaryQian commented Jan 7, 2020

GaryQian commented Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

GaryQian Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

Hixie Jan 8, 2020

GaryQian commented Jan 27, 2020 •

edited

Loading

		@@ -765,13 +844,14 @@ Int32List _encodeParagraphStyle(
		String fontFamily,

		@@ -881,6 +965,7 @@ class ParagraphStyle {
		String fontFamily,

Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087

Engine/LibTxt/dart:ui impl of TextHeightBehavior #15087

Conversation

GaryQian commented Dec 30, 2019 • edited Loading

GaryQian commented Jan 2, 2020

GaryQian commented Jan 3, 2020

GaryQian commented Jan 7, 2020

GaryQian commented Jan 7, 2020

GaryQian commented Jan 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GaryQian commented Jan 27, 2020 • edited Loading

GaryQian commented Dec 30, 2019 •

edited

Loading

GaryQian commented Jan 27, 2020 •

edited

Loading