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

[web] Separate text fragmenting from layout #34085

Merged
merged 45 commits into from
Oct 18, 2022

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Jun 15, 2022

Text layout used to be intertwined with text fragmenting. The idea originally was to do the entire layout in a single pass. But that led to extra complexity in the code and difficulty of maintenance.

With this PR, we are separating the fragmenting of the text from the layout. The high level algorithm looks roughly like this:

  1. Break the paragraph into fragments.
  2. Measure these fragments.
  3. Layout the fragments into lines.
  4. Calculate paragraph-level metrics (height, min intrinsic width, longest line, etc).

In addition to simplifying the code and making it more maintainable, this PR also opens up the door for a few other changes that we intend to make in the text area:

  • Ability to reuse the fragmenting logic with the CanvasKit renderer.
  • Adoption of v8BreakIterator in Chrome.

This PR is necessary towards removing ICU data from CanvasKit (issue).

@mdebbar mdebbar requested a review from yjbanov June 15, 2022 19:34
@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Jun 15, 2022
@mdebbar mdebbar force-pushed the fragmenting_line_breaks branch 2 times, most recently from c8c353d to 7b13622 Compare June 22, 2022 20:06
@mdebbar mdebbar force-pushed the fragmenting_line_breaks branch from 7b13622 to 9ea6cc1 Compare June 23, 2022 20:05
@mdebbar mdebbar changed the title [web] Refactor line breaker to resemble how v8BreakIterator works [web] Separate text fragmenting from layout Jun 29, 2022
void addPlaceholder(PlaceholderSpan placeholder) {
// Increase the line's height to fit the placeholder, if necessary.
MeasuredFragment _updateHeightForPlaceholder(MeasuredFragment fragment) {
final PlaceholderSpan placeholder = fragment.span as PlaceholderSpan;
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here. It feels like we should know ahead of time that the span is a PlaceholderSpan without a cast 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one I couldn't figure out a clean way to remove the cast. If you have ideas, I'm happy to consider.

@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
@flutter flutter deleted a comment from skia-gold Oct 3, 2022
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

@skia-gold
Copy link

Gold has detected about 6 new digest(s) on patchset 45.
View them at https://flutter-engine-gold.skia.org/cl/github/34085

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants