Skip to content

[web_paragraph] Honor kTextHeightNone in TextStyle and ParagraphStyle - #193733

Merged
Rusino merged 3 commits into
flutter:masterfrom
Rusino:webparagraph_ktextheightnone
Oct 12, 2026
Merged

Rusino merged 3 commits into
flutter:masterfrom
Rusino:webparagraph_ktextheightnone

Conversation

@Rusino

@Rusino Rusino commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Part of #193731. Split out of #193403 (first of four independent PRs; the others are #193734 and #193736, and #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 [web_paragraph] Apply StrutStyle to line metrics and placeholders #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

`ChildStyleNode._height` turned `kTextHeightNone` into `null` before
falling back to the parent height, so a child span could not disable an
inherited height multiplier. `LineBlock` only checked `height != null`,
so `ParagraphStyle(height: kTextHeightNone)` scaled the line metrics by
0.0. Both now treat `kTextHeightNone` as "no height multiplier".
@Rusino
Rusino marked this pull request as ready for review October 9, 2026 15:34

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the web paragraph layout engine to correctly handle ui.kTextHeightNone. It preserves ui.kTextHeightNone on child style nodes so they can override parent height multipliers, disables the height multiplier when ui.kTextHeightNone is specified, and replaces a hardcoded fallback font size with StyleManager.defaultFontSize. Additionally, unit tests are added to verify these behaviors. There are no review comments, and I have no feedback to provide.

@Rusino Rusino self-assigned this Oct 9, 2026
@Rusino Rusino added the CICD Run CI/CD label Oct 9, 2026
@Rusino
Rusino requested a review from mdebbar October 9, 2026 17:37
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Oct 14, 2026
…#13220)

Manual roll Flutter from d03768eef055 to 4249ccbc18a3 (158 revisions)

Manual roll requested by [email protected]

flutter/flutter@d03768e...4249ccb

2026-10-12 [email protected] [AGP 9.1.0 Migration #8] Unify add-to-app module wiring on the variant API (flutter/flutter#193717)
2026-10-12 [email protected] [master] Update Flutter DEPS to Dart 5933cb486b4aef2f8cf6b9f89a5651c0d5b0955e (flutter/flutter#193982)
2026-10-12 [email protected] [Android] Fix use-after-free of SurfaceControl.Transaction in HCPP (flutter/flutter#192969)
2026-10-12 [email protected] [windows] Add OpenGL compositor tests against ANGLE at D3D11 feature level 10_0 (flutter/flutter#194114)
2026-10-12 [email protected] Fix HDR image packing for ios (flutter/flutter#192526)
2026-10-12 [email protected] Bump customer tests to pick up material_ui and cupertino_ui (flutter/flutter#194236)
2026-10-12 [email protected] Remove no-shuffle from text_field_test.dart (flutter/flutter#193550)
2026-10-12 [email protected] [web] Clip semantics DOM text labels to stop stealing taps (flutter/flutter#194011)
2026-10-12 [email protected] Roll Skia from 9b6ceba4728d to 17f6986c226c (6 revisions) (flutter/flutter#194232)
2026-10-12 [email protected] [web] Skip non-focusable containers in Focusable.focusAsRouteDefault (flutter/flutter#194138)
2026-10-12 [email protected] [web_paragraph] Honor kTextHeightNone in TextStyle and ParagraphStyle (flutter/flutter#193733)
2026-10-12 [email protected] Roll Skia from c88b49d1d6c7 to 9b6ceba4728d (4 revisions) (flutter/flutter#194226)
2026-10-12 [email protected] Migrate remaining test to macOS 26 (flutter/flutter#193825)
2026-10-12 [email protected] Roll Dart SDK from 047ca322aa9c to 221363557163 (1 revision) (flutter/flutter#194219)
2026-10-12 [email protected] [flutter_tools] Include dev_dependencies hooks for release integration tests (flutter/flutter#193436)
2026-10-12 [email protected] Roll Skia from d8afad1b1829 to c88b49d1d6c7 (1 revision) (flutter/flutter#194198)
2026-10-12 [email protected] Roll Skia from b23ddbff1265 to d8afad1b1829 (4 revisions) (flutter/flutter#194195)
2026-10-11 [email protected] Roll Skia from a168863e44ec to b23ddbff1265 (1 revision) (flutter/flutter#194185)
2026-10-11 [email protected] Roll Skia from 0c1d93abfa04 to a168863e44ec (1 revision) (flutter/flutter#194181)
2026-10-11 [email protected] iOS: Re-enable rendering on willEnterForeground (flutter/flutter#191350)
2026-10-11 [email protected] Roll Skia from 93c9d3ffda25 to 0c1d93abfa04 (1 revision) (flutter/flutter#194179)
2026-10-11 [email protected] Remove more instances of deprecated `withOpacity` (flutter/flutter#193559)
2026-10-11 [email protected] Fix another `parameter_assignments` warning (flutter/flutter#193669)
2026-10-11 [email protected] Roll Skia from 3136abef8f58 to 93c9d3ffda25 (1 revision) (flutter/flutter#194169)
2026-10-11 [email protected] [Impeller] Guard the sampler library caches with a mutex (flutter/flutter#193738)
2026-10-11 [email protected] Roll Dart SDK from e9c89888bcc1 to 047ca322aa9c (1 revision) (flutter/flutter#194157)
2026-10-10 [email protected] Roll Dart SDK from 787baa8aae78 to e9c89888bcc1 (2 revisions) (flutter/flutter#194141)
2026-10-10 [email protected] Fix OverlayPortal layout in shrink-wrapped overlays (flutter/flutter#193079)
2026-10-10 [email protected] Roll Skia from 353076e27b1c to 3136abef8f58 (1 revision) (flutter/flutter#194130)
2026-10-10 [email protected] Roll Dart SDK from 88a07589b1ff to 787baa8aae78 (1 revision) (flutter/flutter#194124)
2026-10-10 [email protected] Route windowing view lookups through WidgetsBinding (flutter/flutter#192666)
2026-10-10 [email protected] Roll Dart SDK from 2a06cab34715 to 88a07589b1ff (1 revision) (flutter/flutter#194117)
2026-10-10 [email protected] Roll Skia from f9043b7aa798 to 353076e27b1c (2 revisions) (flutter/flutter#194115)
2026-10-10 [email protected] Refactored PipelineLibraryGLES (flutter/flutter#193985)
2026-10-09 [email protected] add shorthands for scroll physics subclasses in framework (flutter/flutter#193082)
2026-10-09 [email protected] Roll Dart SDK from ba889e3cee8a to 2a06cab34715 (1 revision) (flutter/flutter#194105)
2026-10-09 [email protected] Roll Skia from 5ea03d8a53a6 to f9043b7aa798 (2 revisions) (flutter/flutter#194099)
2026-10-09 [email protected] Add docs for using bb and led with LUCI (flutter/flutter#193859)
2026-10-09 [email protected] Roll Skia from e329bb617820 to 5ea03d8a53a6 (5 revisions) (flutter/flutter#194087)
2026-10-09 [email protected] Roll Packages from 54706bd to d8d9e97 (5 revisions) (flutter/flutter#194080)
2026-10-09 [email protected] Move the Vulkan wrapped-texture helper out of the GPU surface (flutter/flutter#193001)
2026-10-09 [email protected] Roll Skia from 3e54bcf4c963 to e329bb617820 (1 revision) (flutter/flutter#194078)
2026-10-09 [email protected] Roll Dart SDK from 23f0308f9af8 to ba889e3cee8a (2 revisions) (flutter/flutter#194077)
2026-10-09 [email protected] Bump dartdoc to 9.0.11 (flutter/flutter#194076)
...
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]>
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-web Web applications specifically team-web Owned by Web platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants