Repository navigation
[web_paragraph] Compute empty paragraph line metrics with height, TextHeightBehavior, and StrutStyle - #193403
Draft
Rusino wants to merge 2 commits into
Draft
[web_paragraph] Compute empty paragraph line metrics with height, TextHeightBehavior, and StrutStyle#193403Rusino wants to merge 2 commits into
Rusino wants to merge 2 commits into
Conversation
Rusino
force-pushed
the
webparagraph_height_and_placeholders
branch
from
October 2, 2026 21:14
2b328a6 to
fed29b9
Compare
This was referenced Oct 2, 2026
Rusino
force-pushed
the
webparagraph_height_and_placeholders
branch
from
October 3, 2026 03:01
fed29b9 to
6590cdd
Compare
Rusino
force-pushed
the
webparagraph_height_and_placeholders
branch
from
October 3, 2026 21:27
6590cdd to
0a88953
Compare
This was referenced Oct 3, 2026
Rusino
force-pushed
the
webparagraph_height_and_placeholders
branch
from
October 3, 2026 21:45
0a88953 to
b45d36f
Compare
Hzj-jie
pushed a commit
to zijiehe-google-com/flutter
that referenced
this pull request
Oct 12, 2026
…flutter#193733) Part of flutter#193731. Split out of flutter#193403 (first of four independent PRs; the others are flutter#193734 and flutter#193736, and flutter#193403 itself will be rebased on top once these land). ### Problem - `ChildStyleNode._height` converted `style.height == ui.kTextHeightNone` to `null` *before* falling back to `parent._height`, so a child `TextStyle(height: ui.kTextHeightNone)` could never override a non-null parent height (`ParagraphStyle(height: 3.0)` + `pushStyle(TextStyle(height: kTextHeightNone))` still laid out the line at 3x). - `LineBlock` only checked `span.style.height == null`, so `ParagraphStyle(height: ui.kTextHeightNone)` (`0.0`) multiplied the line ascent/descent by `0.0` instead of disabling the height multiplier. ### Fix - `ChildStyleNode._height` keeps `style.height` as-is: `style.height ?? parent._height`. `kTextHeightNone` now reaches `LineBlock`, which is the one place that interprets it. - `LineBlock`: `height == null || height == ui.kTextHeightNone` → raw font ascent/descent, no scaling (same as the native path, where `paragraph_builder.cc` sets `has_height_override = style.height != kTextHeightNone`). - `LineBlock` uses `StyleManager.defaultFontSize` instead of a hardcoded `14.0`. - `WebStrutStyle` already normalizes `kTextHeightNone` to `null` in its constructor, so struts need no change. ### Tests - `paragraph_get_boxes_test.dart`: `layout with kTextHeightNone disables parent height multiplier`, `layout with ParagraphStyle(kTextHeightNone)`. - **WebParagraph corpus (`preferWebParagraph` + `forceTestFonts` vs `origin/master` `b7315d76579e`)**: 20,012 tests (18,765 executed, 1,247 skipped) — **503 -> 503 failures (0 fixed standalone, +2 new tests passing, 0 regressions)**. - *Note:* Existing corpus tests for `kTextHeightNone` also depend on other issues (`test/ui/paragraph_builder_test.dart: kTextHeightNone StrutStyle` is unlocked in flutter#193774 once `StrutStyle` is applied; `test/painting/text_painter_test.dart: kTextHeightNone unsets the text height multiplier` still fails due to Chromium's integer-rounded `FlutterTest` font metrics at `10px` where `ascent + descent = 11 != 10`). ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
mdebbar
added a commit
to mdebbar/flutter
that referenced
this pull request
Oct 15, 2026
…ult to proportional (flutter#193736) Part of flutter#193731. Split out of flutter#193403 (third of four independent PRs; the others are flutter#193733 and flutter#193734, and flutter#193403 is stacked on top of this PR). ### Problem - **`TextHeightBehavior.leadingDistribution` was ignored.** `WebParagraphStyle` never passed `textHeightBehavior.leadingDistribution` to the root `WebTextStyle`, and `RootStyleNode._leadingDistribution` always returned `null`, so the value could not reach `LineBlock`. Only an explicit `TextStyle(leadingDistribution: …)` had any effect. - **The default leading distribution was `even`.** `LineBlock` treated `null` the same as `TextLeadingDistribution.even` (half-leading). SkParagraph (and therefore the native engine and CanvasKit) default to `proportional`: `TextStyle::fHalfLeading` is `false` unless `leadingDistribution == even` is set, and `dart:ui`'s `TextHeightBehavior.leadingDistribution` defaults to `TextLeadingDistribution.proportional`. - **Negative half-leading clamped `fontBoundingBoxDescent` to `0.0`.** `TextLine` initialized `fontBoundingBoxAscent` and `fontBoundingBoxDescent` to `0.0` and accumulated every block with `math.max(0.0, ...)`. With `TextLeadingDistribution.even` and a small `height` multiplier (where `height * fontSize < rawAscent + rawDescent`), `extraLeading / 2` is negative and `multipliedFontBoundingBoxDescent` can become negative; `math.max(0.0, ...)` clamped it to `0.0`, inflating the line height above `height * fontSize`. ### Fix - `WebParagraphStyle` propagates `textHeightBehavior?.leadingDistribution` into the root `WebTextStyle`; `RootStyleNode._leadingDistribution` returns `style.leadingDistribution`. Child `TextStyle`s still override it through `ChildStyleNode` as before. - `LineBlock`: `null` now falls through to the `proportional` branch (ascent and descent are scaled by `height * fontSize / (ascent + descent)`), `even` keeps the half-leading behavior. - `TextLine.updateBoundingBox`: track `_hasLineMetrics` so the first block on the line initializes `fontBoundingBoxAscent` and `fontBoundingBoxDescent` directly (preserving a negative `descent`), while subsequent blocks take `math.max`. ### Behavior change For any paragraph with a `height` multiplier and no explicit `leadingDistribution`, the **line height is unchanged** but the **baseline moves**: the extra leading is now split in proportion to the font's ascent/descent instead of 50/50. This brings WebParagraph in line with CanvasKit/Skwasm output for the same `ParagraphStyle`; WebParagraph is not the default renderer yet, so no app-facing change on stable. ### Tests - New `paragraph_line_height_test.dart`: - `Height multiplier scales ascent and descent proportionally by default` — checks the scaled line keeps `height * fontSize`, that ascent/descent scale by the same factor by default, that `TextHeightBehavior(leadingDistribution: even)` adds equal leading on both sides, and that the two differ. Fails on `master` (default was `even` and the THB value was ignored). - `Half-leading with a small height multiplier preserves negative descent` — checks that `height: 0.1` with `TextLeadingDistribution.even` keeps a negative `descent` and total height `0.1 * fontSize`. - Existing `webparagraph/*` and `ui/paragraph_builder_test.dart` suites pass. - **WebParagraph corpus (`preferWebParagraph` + `forceTestFonts` vs `origin/master` `b7315d76579e`)**: **0 fixed standalone, +2 new tests passing, 0 regressions** (base of the flutter#193736 -> flutter#193773 -> flutter#193774 -> flutter#193403 stack that fixes **34** corpus tests; framework `TextPainter line-height` tests in `test/painting/text_painter_test.dart` also call `getBoxesForSelection` with `BoxHeightStyle.tight` and are unlocked when combined with flutter#193734). ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Mouad Debbar <[email protected]>
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
Rusino
force-pushed
the
webparagraph_height_and_placeholders
branch
2 times, most recently
from
October 15, 2026 20:22
d80f172 to
5335d30
Compare
harryterkelsen
pushed a commit
to harryterkelsen/flutter
that referenced
this pull request
Oct 16, 2026
…boxes (flutter#193734) Part of flutter#193731. Split out of flutter#193403 (second of four independent PRs; the others are flutter#193733 and flutter#193736, and flutter#193403 itself will be rebased on top once these land). ### Problem - **Placeholder vertical position was computed too early.** `PlaceholderBlock.calculatePlaceholderTop` set `block.advance` from the line ascent *as it was at that moment* before folding `block.ascent` into the line. A placeholder taller than the preceding text (or a later, taller placeholder on the same line) grew `line.fontBoundingBoxAscent` afterwards in `line.updateBoundingBox(block)`, leaving placeholder `advance.top` positioned against a stale ascent. `getBoxesForPlaceholders` returned those stale rects, so a placeholder box could sit above the line's top (negative `top`) and disagree with `getBoxesForRange` for the same range. - **`BoxHeightStyle.tight` returned the height-scaled box for text.** `getBoxesForRange` used `multipliedFontBoundingBoxAscent` / `multipliedHeight`, i.e. the box included the extra leading added by `TextStyle.height`. SkParagraph's `TextLine::getRectsForRange` returns the unscaled glyph bounds for `tight` (`Rect::MakeXYWH(..., -run.ascent(), ..., run.descent())` relative to the baseline). The same code path was also applied to `PlaceholderBlock`, whose `advance` is already line-relative, producing a double-shifted box. ### Fix - `PlaceholderBlock` & `addLine`: split placeholder layout into two passes: - Pass 1 calls `block.calculatePlaceholderMetrics(line.fontBoundingBoxAscent, line.fontBoundingBoxDescent)` to compute each placeholder's `ascent`/`descent` and folds them into the line metrics via `line.updateBoundingBox(block)`. - Pass 2, after `line.fontBoundingBoxAscent` is final, calls `block.calculatePlaceholderTop(line.fontBoundingBoxAscent)` to compute `top = lineAscent - ascent` and set `block.advance = Rect.fromLTWH(spanShiftFromLineStart, top, span.width, span.height)`. - `getBoxesForRange`, `BoxHeightStyle.tight`: - `TextBlock`: `top = line.advance.top + line.fontBoundingBoxAscent - block.rawFontBoundingBoxAscent`, `height = rawFontBoundingBoxAscent + rawFontBoundingBoxDescent` (unscaled glyph bounds positioned at the baseline, matching SkParagraph). - `PlaceholderBlock`: `top = firstRect.top + line.advance.top`, `height = span.height` — `block.advance` is already relative to the line, so no extra ascent shift. - Removed the unused vertical `.translate(...)` on `firstRect` for `TextBlock`, since `firstRect` only provides `left` and `width` while `top` and `bottom` are computed per `BoxHeightStyle`. ### Tests - `paragraph_get_boxes_test.dart`: - `getBoxesForRange and getBoxesForPlaceholders return matching boxes for placeholders` — two placeholders on one line where the second is taller; checks both APIs agree, `top >= 0`, and the box sizes equal the placeholder sizes. On `master`: `getBoxesForPlaceholders` returns `(0, -12, 50, 18)` for the first placeholder while `getBoxesForRange(0, 1)` returns `(0, 14, 50, 44)`. - `BoxHeightStyle.tight uses unscaled glyph height and respects TextHeightBehavior` — `height: 3.0` with `even` leading; `tight` box height equals the unscaled line height and is centered in the 60px line. On `master` the `tight` box is 60px tall instead of 22px. - Existing `paragraph_get_boxes_test.dart` and `paragraph_placeholders_test.dart` pass. - **Full WebParagraph corpus (`--full`, `preferWebParagraph` + `forceTestFonts` vs `origin/master` `b7315d76579e`)**: - Part 1 (`web_ui`): `16/16` `webparagraph/*` (`+198 ~5 -1`, **+2** new tests passing) + `60/60` `test/ui/*` (`+1354 ~5 -41`). - Part 2 (`packages/flutter`, `894/894` suites across `8/8` shards): `+16712 ~1237 -459` vs baseline `+16710 ~1237 -461` (**+3** existing tests fixed; **1** apparent golden diff in `test/cupertino/text_field_test.dart: Overflow clipBehavior none golden` where on `origin/master` two bugs cancelled out — missing `StrutStyle(forceStrutHeight: true)` from flutter#193774 inflated the line ascent by 83px while the old single-pass `calculatePlaceholderTop` shifted the placeholder by `-83px` detached from its `'Hi'` text; resolved once flutter#193774 lands). - Jointly with flutter#193736 + flutter#193773 also unlocks `test/painting/text_painter_test.dart: TextPainter line-height half-leading with leading trim`. <details> <summary>Fixed corpus tests vs <code>origin/master</code> (3)</summary> - `packages/flutter` :: `test/painting/text_painter_test.dart: TextPainter line-height TextLeadingDistribution falls back to paragraph style` - `packages/flutter` :: `test/painting/text_painter_test.dart: TextPainter line-height half-leading` - `packages/flutter` :: `test/painting/text_painter_test.dart: TextPainter widget span` </details> ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
gaaclarke
pushed a commit
to gaaclarke/flutter
that referenced
this pull request
Oct 22, 2026
… applyHeightToLastDescent (flutter#193773) Honors `TextHeightBehavior.applyHeightToFirstAscent` and `applyHeightToLastDescent` when computing `WebParagraph` line metrics (for non-empty paragraphs). Part of flutter#193731. > [!NOTE] > Split out from flutter#193403 to keep each change small and focused. ### Problem Both `applyHeightToFirstAscent` and `applyHeightToLastDescent` were ignored: the first line always kept its height-scaled ascent and the last line always kept its height-scaled descent. ### Fix - `TextLine.rawFontBoundingBoxAscent` / `rawFontBoundingBoxDescent`: accumulate unscaled font metrics alongside height-scaled metrics in `TextLine.updateBoundingBox` (and copy them in `TextLine.inheritMetricsFrom`). - **Identifying the first and last lines**: - `isFirstLine` is determined in `TextLayout._finishLine` via `lines.isEmpty` before the line is appended. - `isLastLine` is passed from `_LineBuilder.build` (`wrapper.dart`) and stored on `line.lastLine` in `_finishLine`: a content line is the last line when it hits the `maxLines` limit or reaches the artificial EOF cluster without a trailing newline; when a trailing `\n` is present (and `maxLines` is not yet reached), the synthetic empty line added after `\n` is marked `isLastLine: true`. - **Single-pass metric adjustment (`TextLayout._finishLine` -> `TextLine.applyTextHeightBehavior`)**: before computing `line.advance`, `isFirstLine && !applyHeightToFirstAscent` sets `usesRawAscent = true` and reverts `fontBoundingBoxAscent` to `rawFontBoundingBoxAscent`, and `isLastLine && !applyHeightToLastDescent` sets `usesRawDescent = true` and reverts `fontBoundingBoxDescent` to `rawFontBoundingBoxDescent`. Because this happens before `line.advance` is returned to the wrapper, line tops and paragraph height are accumulated in a single pass. - **Placeholders & backgrounds**: `PlaceholderBlock.positionInLine` positions placeholders after `applyTextHeightBehavior` so first-line placeholders align against the reverted ascent, and `WebParagraphPainter._paintAllBlocks` bounds background rects by raw ascent/descent when `usesRawAscent`/`usesRawDescent` are set. - **Trailing newline**: the synthetic empty line created after a trailing `\n` inherits the preceding line's scaled and raw metrics via `inheritMetricsFrom`, so the synthetic last line drops its scaled descent while the preceding text line keeps its scaled descent — matching `SkParagraph`. ### Tests - `paragraph_line_height_test.dart` (+6 tests): - `TextHeightBehavior reverts a single line to the unscaled font metrics` - `TextHeightBehavior affects only the first line ascent and the last line descent` - `TextHeightBehavior treats the empty line after a trailing newline as the last line` - `TextHeightBehavior applies applyHeightToLastDescent to the maxLines limit line` - `TextHeightBehavior positions first-line placeholders against the reverted ascent` - `TextHeightBehavior bounds background rects by reverted first ascent and last descent` - **WebParagraph corpus (`preferWebParagraph` + `forceTestFonts` vs `origin/master` `2e53c3af41a`)**: - Part 1 (`web_ui` `test/webparagraph/*` + `test/ui/*`): **+6 new tests passing (`207` vs `201`), 0 regressions**. - Part 2 (`packages/flutter`): **+2 tests fixed (`test/painting/text_painter_test.dart: TextHeightBehavior with strut on empty paragraph` and `test/painting/text_painter_test.dart: TextPainter widget span`), 0 regressions**. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Mouad Debbar <[email protected]>
Compute StrutStyle metrics with the default font size, leading, and paragraph leadingDistribution fallback, initialize TextLine metrics from the strut, and lock both scaled and raw line metrics when forceStrutHeight is set. Assisted-by: Gemini 4 Argon (High)
…tHeightBehavior, and StrutStyle Mirror SkParagraph's computeEmptyMetrics order (font metrics -> height multiplier -> TextHeightBehavior -> StrutStyle) and empty-paragraph half-leading quirks so all web renderers produce matching empty paragraph heights and baselines.
Rusino
force-pushed
the
webparagraph_height_and_placeholders
branch
from
October 26, 2026 21:56
5335d30 to
9064150
Compare
suryaKommana2662
pushed a commit
to suryaKommana2662/flutter
that referenced
this pull request
Oct 29, 2026
…flutter#193733) Part of flutter#193731. Split out of flutter#193403 (first of four independent PRs; the others are flutter#193734 and flutter#193736, and flutter#193403 itself will be rebased on top once these land). ### Problem - `ChildStyleNode._height` converted `style.height == ui.kTextHeightNone` to `null` *before* falling back to `parent._height`, so a child `TextStyle(height: ui.kTextHeightNone)` could never override a non-null parent height (`ParagraphStyle(height: 3.0)` + `pushStyle(TextStyle(height: kTextHeightNone))` still laid out the line at 3x). - `LineBlock` only checked `span.style.height == null`, so `ParagraphStyle(height: ui.kTextHeightNone)` (`0.0`) multiplied the line ascent/descent by `0.0` instead of disabling the height multiplier. ### Fix - `ChildStyleNode._height` keeps `style.height` as-is: `style.height ?? parent._height`. `kTextHeightNone` now reaches `LineBlock`, which is the one place that interprets it. - `LineBlock`: `height == null || height == ui.kTextHeightNone` → raw font ascent/descent, no scaling (same as the native path, where `paragraph_builder.cc` sets `has_height_override = style.height != kTextHeightNone`). - `LineBlock` uses `StyleManager.defaultFontSize` instead of a hardcoded `14.0`. - `WebStrutStyle` already normalizes `kTextHeightNone` to `null` in its constructor, so struts need no change. ### Tests - `paragraph_get_boxes_test.dart`: `layout with kTextHeightNone disables parent height multiplier`, `layout with ParagraphStyle(kTextHeightNone)`. - **WebParagraph corpus (`preferWebParagraph` + `forceTestFonts` vs `origin/master` `b7315d76579e`)**: 20,012 tests (18,765 executed, 1,247 skipped) — **503 -> 503 failures (0 fixed standalone, +2 new tests passing, 0 regressions)**. - *Note:* Existing corpus tests for `kTextHeightNone` also depend on other issues (`test/ui/paragraph_builder_test.dart: kTextHeightNone StrutStyle` is unlocked in flutter#193774 once `StrutStyle` is applied; `test/painting/text_painter_test.dart: kTextHeightNone unsets the text height multiplier` still fails due to Chromium's integer-rounded `FlutterTest` font metrics at `10px` where `ascent + descent = 11 != 10`). ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
suryaKommana2662
pushed a commit
to suryaKommana2662/flutter
that referenced
this pull request
Oct 29, 2026
…ult to proportional (flutter#193736) Part of flutter#193731. Split out of flutter#193403 (third of four independent PRs; the others are flutter#193733 and flutter#193734, and flutter#193403 is stacked on top of this PR). ### Problem - **`TextHeightBehavior.leadingDistribution` was ignored.** `WebParagraphStyle` never passed `textHeightBehavior.leadingDistribution` to the root `WebTextStyle`, and `RootStyleNode._leadingDistribution` always returned `null`, so the value could not reach `LineBlock`. Only an explicit `TextStyle(leadingDistribution: …)` had any effect. - **The default leading distribution was `even`.** `LineBlock` treated `null` the same as `TextLeadingDistribution.even` (half-leading). SkParagraph (and therefore the native engine and CanvasKit) default to `proportional`: `TextStyle::fHalfLeading` is `false` unless `leadingDistribution == even` is set, and `dart:ui`'s `TextHeightBehavior.leadingDistribution` defaults to `TextLeadingDistribution.proportional`. - **Negative half-leading clamped `fontBoundingBoxDescent` to `0.0`.** `TextLine` initialized `fontBoundingBoxAscent` and `fontBoundingBoxDescent` to `0.0` and accumulated every block with `math.max(0.0, ...)`. With `TextLeadingDistribution.even` and a small `height` multiplier (where `height * fontSize < rawAscent + rawDescent`), `extraLeading / 2` is negative and `multipliedFontBoundingBoxDescent` can become negative; `math.max(0.0, ...)` clamped it to `0.0`, inflating the line height above `height * fontSize`. ### Fix - `WebParagraphStyle` propagates `textHeightBehavior?.leadingDistribution` into the root `WebTextStyle`; `RootStyleNode._leadingDistribution` returns `style.leadingDistribution`. Child `TextStyle`s still override it through `ChildStyleNode` as before. - `LineBlock`: `null` now falls through to the `proportional` branch (ascent and descent are scaled by `height * fontSize / (ascent + descent)`), `even` keeps the half-leading behavior. - `TextLine.updateBoundingBox`: track `_hasLineMetrics` so the first block on the line initializes `fontBoundingBoxAscent` and `fontBoundingBoxDescent` directly (preserving a negative `descent`), while subsequent blocks take `math.max`. ### Behavior change For any paragraph with a `height` multiplier and no explicit `leadingDistribution`, the **line height is unchanged** but the **baseline moves**: the extra leading is now split in proportion to the font's ascent/descent instead of 50/50. This brings WebParagraph in line with CanvasKit/Skwasm output for the same `ParagraphStyle`; WebParagraph is not the default renderer yet, so no app-facing change on stable. ### Tests - New `paragraph_line_height_test.dart`: - `Height multiplier scales ascent and descent proportionally by default` — checks the scaled line keeps `height * fontSize`, that ascent/descent scale by the same factor by default, that `TextHeightBehavior(leadingDistribution: even)` adds equal leading on both sides, and that the two differ. Fails on `master` (default was `even` and the THB value was ignored). - `Half-leading with a small height multiplier preserves negative descent` — checks that `height: 0.1` with `TextLeadingDistribution.even` keeps a negative `descent` and total height `0.1 * fontSize`. - Existing `webparagraph/*` and `ui/paragraph_builder_test.dart` suites pass. - **WebParagraph corpus (`preferWebParagraph` + `forceTestFonts` vs `origin/master` `b7315d76579e`)**: **0 fixed standalone, +2 new tests passing, 0 regressions** (base of the flutter#193736 -> flutter#193773 -> flutter#193774 -> flutter#193403 stack that fixes **34** corpus tests; framework `TextPainter line-height` tests in `test/painting/text_painter_test.dart` also call `getBoxesForSelection` with `BoxHeightStyle.tight` and are unlocked when combined with flutter#193734). ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Mouad Debbar <[email protected]>
suryaKommana2662
pushed a commit
to suryaKommana2662/flutter
that referenced
this pull request
Oct 29, 2026
…boxes (flutter#193734) Part of flutter#193731. Split out of flutter#193403 (second of four independent PRs; the others are flutter#193733 and flutter#193736, and flutter#193403 itself will be rebased on top once these land). ### Problem - **Placeholder vertical position was computed too early.** `PlaceholderBlock.calculatePlaceholderTop` set `block.advance` from the line ascent *as it was at that moment* before folding `block.ascent` into the line. A placeholder taller than the preceding text (or a later, taller placeholder on the same line) grew `line.fontBoundingBoxAscent` afterwards in `line.updateBoundingBox(block)`, leaving placeholder `advance.top` positioned against a stale ascent. `getBoxesForPlaceholders` returned those stale rects, so a placeholder box could sit above the line's top (negative `top`) and disagree with `getBoxesForRange` for the same range. - **`BoxHeightStyle.tight` returned the height-scaled box for text.** `getBoxesForRange` used `multipliedFontBoundingBoxAscent` / `multipliedHeight`, i.e. the box included the extra leading added by `TextStyle.height`. SkParagraph's `TextLine::getRectsForRange` returns the unscaled glyph bounds for `tight` (`Rect::MakeXYWH(..., -run.ascent(), ..., run.descent())` relative to the baseline). The same code path was also applied to `PlaceholderBlock`, whose `advance` is already line-relative, producing a double-shifted box. ### Fix - `PlaceholderBlock` & `addLine`: split placeholder layout into two passes: - Pass 1 calls `block.calculatePlaceholderMetrics(line.fontBoundingBoxAscent, line.fontBoundingBoxDescent)` to compute each placeholder's `ascent`/`descent` and folds them into the line metrics via `line.updateBoundingBox(block)`. - Pass 2, after `line.fontBoundingBoxAscent` is final, calls `block.calculatePlaceholderTop(line.fontBoundingBoxAscent)` to compute `top = lineAscent - ascent` and set `block.advance = Rect.fromLTWH(spanShiftFromLineStart, top, span.width, span.height)`. - `getBoxesForRange`, `BoxHeightStyle.tight`: - `TextBlock`: `top = line.advance.top + line.fontBoundingBoxAscent - block.rawFontBoundingBoxAscent`, `height = rawFontBoundingBoxAscent + rawFontBoundingBoxDescent` (unscaled glyph bounds positioned at the baseline, matching SkParagraph). - `PlaceholderBlock`: `top = firstRect.top + line.advance.top`, `height = span.height` — `block.advance` is already relative to the line, so no extra ascent shift. - Removed the unused vertical `.translate(...)` on `firstRect` for `TextBlock`, since `firstRect` only provides `left` and `width` while `top` and `bottom` are computed per `BoxHeightStyle`. ### Tests - `paragraph_get_boxes_test.dart`: - `getBoxesForRange and getBoxesForPlaceholders return matching boxes for placeholders` — two placeholders on one line where the second is taller; checks both APIs agree, `top >= 0`, and the box sizes equal the placeholder sizes. On `master`: `getBoxesForPlaceholders` returns `(0, -12, 50, 18)` for the first placeholder while `getBoxesForRange(0, 1)` returns `(0, 14, 50, 44)`. - `BoxHeightStyle.tight uses unscaled glyph height and respects TextHeightBehavior` — `height: 3.0` with `even` leading; `tight` box height equals the unscaled line height and is centered in the 60px line. On `master` the `tight` box is 60px tall instead of 22px. - Existing `paragraph_get_boxes_test.dart` and `paragraph_placeholders_test.dart` pass. - **Full WebParagraph corpus (`--full`, `preferWebParagraph` + `forceTestFonts` vs `origin/master` `b7315d76579e`)**: - Part 1 (`web_ui`): `16/16` `webparagraph/*` (`+198 ~5 -1`, **+2** new tests passing) + `60/60` `test/ui/*` (`+1354 ~5 -41`). - Part 2 (`packages/flutter`, `894/894` suites across `8/8` shards): `+16712 ~1237 -459` vs baseline `+16710 ~1237 -461` (**+3** existing tests fixed; **1** apparent golden diff in `test/cupertino/text_field_test.dart: Overflow clipBehavior none golden` where on `origin/master` two bugs cancelled out — missing `StrutStyle(forceStrutHeight: true)` from flutter#193774 inflated the line ascent by 83px while the old single-pass `calculatePlaceholderTop` shifted the placeholder by `-83px` detached from its `'Hi'` text; resolved once flutter#193774 lands). - Jointly with flutter#193736 + flutter#193773 also unlocks `test/painting/text_painter_test.dart: TextPainter line-height half-leading with leading trim`. <details> <summary>Fixed corpus tests vs <code>origin/master</code> (3)</summary> - `packages/flutter` :: `test/painting/text_painter_test.dart: TextPainter line-height TextLeadingDistribution falls back to paragraph style` - `packages/flutter` :: `test/painting/text_painter_test.dart: TextPainter line-height half-leading` - `packages/flutter` :: `test/painting/text_painter_test.dart: TextPainter widget span` </details> ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
suryaKommana2662
pushed a commit
to suryaKommana2662/flutter
that referenced
this pull request
Oct 29, 2026
… applyHeightToLastDescent (flutter#193773) Honors `TextHeightBehavior.applyHeightToFirstAscent` and `applyHeightToLastDescent` when computing `WebParagraph` line metrics (for non-empty paragraphs). Part of flutter#193731. > [!NOTE] > Split out from flutter#193403 to keep each change small and focused. ### Problem Both `applyHeightToFirstAscent` and `applyHeightToLastDescent` were ignored: the first line always kept its height-scaled ascent and the last line always kept its height-scaled descent. ### Fix - `TextLine.rawFontBoundingBoxAscent` / `rawFontBoundingBoxDescent`: accumulate unscaled font metrics alongside height-scaled metrics in `TextLine.updateBoundingBox` (and copy them in `TextLine.inheritMetricsFrom`). - **Identifying the first and last lines**: - `isFirstLine` is determined in `TextLayout._finishLine` via `lines.isEmpty` before the line is appended. - `isLastLine` is passed from `_LineBuilder.build` (`wrapper.dart`) and stored on `line.lastLine` in `_finishLine`: a content line is the last line when it hits the `maxLines` limit or reaches the artificial EOF cluster without a trailing newline; when a trailing `\n` is present (and `maxLines` is not yet reached), the synthetic empty line added after `\n` is marked `isLastLine: true`. - **Single-pass metric adjustment (`TextLayout._finishLine` -> `TextLine.applyTextHeightBehavior`)**: before computing `line.advance`, `isFirstLine && !applyHeightToFirstAscent` sets `usesRawAscent = true` and reverts `fontBoundingBoxAscent` to `rawFontBoundingBoxAscent`, and `isLastLine && !applyHeightToLastDescent` sets `usesRawDescent = true` and reverts `fontBoundingBoxDescent` to `rawFontBoundingBoxDescent`. Because this happens before `line.advance` is returned to the wrapper, line tops and paragraph height are accumulated in a single pass. - **Placeholders & backgrounds**: `PlaceholderBlock.positionInLine` positions placeholders after `applyTextHeightBehavior` so first-line placeholders align against the reverted ascent, and `WebParagraphPainter._paintAllBlocks` bounds background rects by raw ascent/descent when `usesRawAscent`/`usesRawDescent` are set. - **Trailing newline**: the synthetic empty line created after a trailing `\n` inherits the preceding line's scaled and raw metrics via `inheritMetricsFrom`, so the synthetic last line drops its scaled descent while the preceding text line keeps its scaled descent — matching `SkParagraph`. ### Tests - `paragraph_line_height_test.dart` (+6 tests): - `TextHeightBehavior reverts a single line to the unscaled font metrics` - `TextHeightBehavior affects only the first line ascent and the last line descent` - `TextHeightBehavior treats the empty line after a trailing newline as the last line` - `TextHeightBehavior applies applyHeightToLastDescent to the maxLines limit line` - `TextHeightBehavior positions first-line placeholders against the reverted ascent` - `TextHeightBehavior bounds background rects by reverted first ascent and last descent` - **WebParagraph corpus (`preferWebParagraph` + `forceTestFonts` vs `origin/master` `2e53c3af41a`)**: - Part 1 (`web_ui` `test/webparagraph/*` + `test/ui/*`): **+6 new tests passing (`207` vs `201`), 0 regressions**. - Part 2 (`packages/flutter`): **+2 tests fixed (`test/painting/text_painter_test.dart: TextHeightBehavior with strut on empty paragraph` and `test/painting/text_painter_test.dart: TextPainter widget span`), 0 regressions**. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Mouad Debbar <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Computes empty paragraph (
paragraph.text.isEmpty) line metrics inWebParagraphtaking into account the style'sheightmultiplier,TextHeightBehavior, andStrutStyle, matchingSkParagraph. Part of #193731 (fixes it once the whole set has landed).Note
Stacked on top of #193774 (
StrutStyle), #193773 (TextHeightBehavior), and #193736 (TextLeadingDistribution). The earlier parts of this PR were split out into #193733 (ui.kTextHeightNone), #193734 (PlaceholderBlockpositioning andBoxHeightStyle.tight), #193736 (TextLeadingDistribution), #193773 (TextHeightBehavior), and #193774 (StrutStyle).Problem
Empty paragraphs (
paragraph.text.isEmptyinTextLayout.wrapText) ignoredheight,StrutStyle, andTextHeightBehaviorentirely;paragraph.heightand both baselines were taken directly from the unscaled single empty cluster.Fix
SkParagraph'sParagraphImpl::computeEmptyMetricsinTextLayout.wrapTextwhenparagraph.text.isEmpty, in the same order:ascentanddescentof the empty cluster.heightmultiplier (height != null && height != ui.kTextHeightNone).TextHeightBehavior(!applyHeightToFirstAscentrevertsascenttorawAscent;!applyHeightToLastDescentrevertsdescenttorawDescent, since the single line of an empty paragraph is both the first and the last line).StrutStyle(forceStrutHeightreplacesascent,descent, andleadingwith the strut's metrics; otherwiseascent,descent, andleadingtakemath.maxwith the strut's metrics).alphabeticBaseline = leading / 2 + ascent,ideographicBaseline = ascent + descent + leading, andparagraph.height = (ascent + descent + leading).roundToDouble().SkParagraph quirks mirrored on purpose (empty paragraph only)
ParagraphImpl::computeEmptyMetricsdiffers from a line with text in two ways.WebParagraphmirrors both so that all three renderers agree; there is aTODO(jlavrova)inwrapTextto drop them onceSkParagraphis fixed:paragraphStyle.strutStyle.halfLeading, not from the text style. On the Flutter side that isstrutStyle.leadingDistribution ?? textHeightBehavior.leadingDistribution, and without a strut it is alwaysproportional— soTextHeightBehavior(leadingDistribution: even)alone has no effect on an empty paragraph.fLeading) instead of being split between ascent and descent. HenceTextHeightBehaviordoesn't remove it, a non-forced strut is compared against the unscaled ascent/descent, andalphabeticBaseline = leading / 2 + ascent.Tests
paragraph_line_height_test.dart:Empty paragraph applies height, TextHeightBehavior, and StrutStyleEmpty paragraph mirrors SkParagraph half-leading quirkspreferWebParagraph+forceTestFonts, stacked on [web_paragraph] Apply StrutStyle to line metrics and placeholders #193774 -> [web_paragraph] Honor TextHeightBehavior.applyHeightToFirstAscent and applyHeightToLastDescent #193773 -> [web_paragraph] Honor TextHeightBehavior.leadingDistribution and default to proportional #193736 vsorigin/masterb7315d76579e): 20,021 tests (18,774 executed, 1,247 skipped) — 503 -> 469 failures (-34 cumulative vsorigin/master, -7 incremental over [web_paragraph] Apply StrutStyle to line metrics and placeholders #193774; +2 new tests passing, +11 in stack; 0 regressions).Fixed corpus tests (+7 incremental over #193774, 34 cumulative vs
origin/master)Unlocked by this PR (+7):
packages/flutter::test/material/input_decorator_test.dart: Material2 alignLabelWithHint expands true multiline TextFieldpackages/flutter::test/material/input_decorator_test.dart: Material2 alignLabelWithHint expands true multiline TextField with outline borderpackages/flutter::test/material/input_decorator_test.dart: Material2 uses alphabetic baseline for CJK layoutpackages/flutter::test/material/input_decorator_test.dart: Material3 - InputDecoration hint InputDecoration.alignLabelWithHint in expanded multiline TextField positions the label correctly when border is outlinedpackages/flutter::test/painting/text_painter_rtl_test.dart: TextPainter - empty text baselinepackages/flutter::test/painting/text_painter_test.dart: TextHeightBehavior with strut on empty paragraphpackages/flutter::test/painting/text_painter_test.dart: strut style strut style applies when the span has no styleFrom #193774 (+27):
chrome-dart2js-webparagraph-ui-text::paragraph_builder_test.dart: kTextHeightNone StrutStylepackages/flutter::test/painting/text_painter_test.dart: strut style force strut height applies to widget spanspackages/flutter::test/painting/text_painter_test.dart: strut style strut half leading + force strut heightpackages/flutter::test/widgets/editable_text_show_on_screen_test.dart: EditableText._showCaretOnScreen, readOnly=false selection-triggered showCaretOnScreen: text selection delegatepackages/flutter::test/widgets/editable_text_show_on_screen_test.dart: EditableText._showCaretOnScreen, readOnly=false selection-triggered showCaretOnScreen: virtual keyboardpackages/flutter::test/widgets/editable_text_show_on_screen_test.dart: EditableText._showCaretOnScreen, readOnly=true selection-triggered showCaretOnScreen: text selection delegatepackages/flutter::test/widgets/editable_text_show_on_screen_test.dart: EditableText._showCaretOnScreen, readOnly=true selection-triggered showCaretOnScreen: virtual keyboardpackages/flutter::test/widgets/editable_text_show_on_screen_test.dart: scrolls into view with scrollInserts after the keyboard pops uppackages/flutter::test/widgets/editable_text_show_on_screen_test.dart: tapping on a partly visible editable brings it fully on screen with scrollInsetspackages/flutter::test/widgets/editable_text_test.dart: Dragging selection base handle upwards scrolls the viewport (variant: TargetPlatform.android)packages/flutter::test/widgets/editable_text_test.dart: Repeatedly receiving [TextEditingValue] will not trigger a keyboard requestpackages/flutter::test/widgets/editable_text_test.dart: Selection changed scroll into view Selection will be scrolled into view with SelectionChangedCause (variant: TargetPlatform.android)packages/flutter::test/widgets/editable_text_test.dart: Selection changed scroll into view Selection will be scrolled into view with SelectionChangedCause (variant: TargetPlatform.fuchsia)packages/flutter::test/widgets/editable_text_test.dart: Selection changed scroll into view Selection will be scrolled into view with SelectionChangedCause (variant: TargetPlatform.linux)packages/flutter::test/widgets/editable_text_test.dart: Selection changed scroll into view Selection will be scrolled into view with SelectionChangedCause (variant: TargetPlatform.windows)packages/flutter::test/widgets/editable_text_test.dart: TextEditingController TextEditingController.clear() behavior testpackages/flutter::test/widgets/editable_text_test.dart: bringIntoView brings the caret into view when in a viewportpackages/flutter::test/widgets/editable_text_test.dart: bringIntoView does nothing if the physics prohibits implicit scrollingpackages/flutter::test/widgets/editable_text_test.dart: selectWordEdge is correct when EditableText is scaledpackages/flutter::test/widgets/editable_text_test.dart: selection behavior when receiving focus tabbing between fields (variant: TargetPlatform.android)packages/flutter::test/widgets/editable_text_test.dart: selection behavior when receiving focus tabbing between fields (variant: TargetPlatform.fuchsia)packages/flutter::test/widgets/editable_text_test.dart: selection behavior when receiving focus tabbing between fields (variant: TargetPlatform.iOS)packages/flutter::test/widgets/editable_text_test.dart: selection behavior when receiving focus tabbing between fields (variant: TargetPlatform.linux)packages/flutter::test/widgets/editable_text_test.dart: selection behavior when receiving focus tabbing between fields (variant: TargetPlatform.macOS)packages/flutter::test/widgets/editable_text_test.dart: selection behavior when receiving focus tabbing between fields (variant: TargetPlatform.windows)packages/flutter::test/widgets/text_selection_test.dart: Mouse edge scrolling works in an outer scrollablepackages/flutter::test/widgets/text_selection_test.dart: Mouse edge scrolling works with both an outer scrollable and scrolling in the EditableTextPre-launch Checklist
///).