Repository navigation
1.13.0 pre-release review: candidate-bound fix, stress/fuzz tests, docs - #31
Merged
Merged
Conversation
…se review for 1.13.0 Line breaking - _safeCandidateCharLimit guessed the bound from whether the first 32 characters contained CJK and ignored the text scaler, so a Latin run after a single CJK character (e.g. `<b>注</b>日本語 english …`) was cut at ~83 characters and the line ended up to 18 glyphs short. It now walks the text from the break position, adding each character's narrowest width (CJK 1em, other 0.25em, scaled by the text scaler). Removes the unused _isLeadingCjk. Tests - line_breaking_stress_test: narrow glyphs, no character lost or duplicated, zero/1px widths, padding and floats wider than the container, intrinsic sizes in table cells and inline-blocks, 200,000-character paragraphs and a 100,000-character paragraph on the bounded fallback path. - fuzz/layout_fuzz_test: lays mutated documents out at 0, 1, 37 and 300 px. - char_range_and_hooks_test: ruby ranges, the ellipsis-only line, and the TextPainter layout hooks as a clock-free per-line bound. Docs - CSS matrix: word-break keep-all is parsed but not applied (was listed as supported); add overflow-wrap; drop stale line-number references. - ROADMAP: 1.13.0 is a release candidate until published. - New doc/TESTING.md; rewrite test/integration/README.md, which listed six files that do not exist; refresh stale version stamps.
Contributor
|
| Fixture | Budget (ms) | Median (ms) | P95 (ms) | |
|---|---|---|---|---|
| ❌ | simple_paragraph |
8 | 17 | 28 |
| ❌ | mixed_inline |
10 | 15 | 18 |
| ❌ | float_layout |
12 | 18 | 21 |
| ❌ | table_20_rows |
14 | 46 | 68 |
| ✅ | cjk_ruby |
14 | 10 | 12 |
| ❌ | large_article |
16 | 51 | 69 |
One or more fixtures exceeded the 16 ms budget.
Flutter
3.41.5· ubuntu-22.04
Informational, does not block merge: CI runners use software
rendering and are 2-3x slower than the devices the budgets are
calibrated for. Compare against the base branch; if a fixture
regressed relative to it, profile with:
flutter test benchmark/layout_regression.dart --reporter expandedand check _performLineLayout / _buildCharacterMapping for
any new O(N²) or O(N log N) paths introduced in this PR.
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.
Final review pass before publishing 1.13.0. Nothing is published or tagged.
Fix
_safeCandidateCharLimit(from #29) guessed the candidate-prefix bound from whether the first 32 characters contained CJK, and ignored the text scaler. A Latin run after one CJK character (<b>注</b>日本語 english …) was cut at ~83 characters, leaving the line up to 18 glyphs short. Before #29 the same input filled the line. The bound now comes from walking the actual text with each character's narrowest width. Reproduced withTextScaler.linear(0.5)at 800 px, because the test font (Ahem) makes every glyph 1 em wide and hides this class of bug.Tests (+49)
line_breaking_stress_test.dart: narrow glyphs, no character lost or duplicated, 0 / 1 px widths, padding and floats wider than the container, intrinsic sizes in table cells and inline-blocks, 200,000-character paragraphs, a 100,000-character paragraph on the bounded fallback path.fuzz/layout_fuzz_test.dart: lays mutated documents out at 0, 1, 37 and 300 px (the existing fuzz suite only parsed).char_range_and_hooks_test.dart: ruby ranges, the ellipsis-only line,TextPainterlayout hooks as a clock-free per-line bound.Docs
word-break: keep-allas supported; nothing implements it. Corrected, andoverflow-wrapadded.doc/TESTING.md;test/integration/README.mdlisted six files that do not exist.Checked, no change needed
lib/is byte-identical to what is on pub.dev and their tests pass against core 1.13.0. Only core and root need a release.dart pub publish --dry-run: core 0 warnings. pana on core: 160/160. Rootpub downgrade+ analyze clean.ef062d9: Ubuntu stable/beta, Windows, macOS all green.