Skip to content

fix: Fix text rendering issue where spaces are missing - #3192

Merged
luanpotter merged 10 commits into
mainfrom
luan.spaces
Jun 23, 2024
Merged

luanpotter merged 10 commits into
mainfrom
luan.spaces

Conversation

@luanpotter

@luanpotter luanpotter commented Jun 8, 2024 •

Copy link
Copy Markdown
Member

Description

Fix text rendering issue where spaces end up missing.

What happens is that if a single paragraph is split into multiple spans, due to having different formatting, for example, the layouting engine will run into an edge case when trying to place the last span. It will:

  • split it and try to put each word at a time with the preceding space
  • it will eventually find a word that cannot fit, and return to a partial state
  • however, the layouting engine then keeps going and try the same element again. that is because there could be more elements, and the engine conflates bits and elements
  • now the element "starts" at the next word, and suddenly it fits, because a new space is not added. this only happens if the final word is such that it fits by itself but doesn't with the leading space.

That is not normally an issue because if it doesn't fit, it will go to the next line and the space becomes unnecessary in that case. But in case we are still trying to populate the same line, we need to make sure we don't "eat up" the space.

There could be other similar edge cases where a space could go missing, so I opted to make it explicit whether the initial space should be added or not by adding a parameter to determine if it is the start of a new line or not.

Examples

Before:

image

image

After:

image

image

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

@luanpotter luanpotter changed the title fix: Fix text rendering issue where spaces are missing [WIP] fix: Fix text rendering issue where spaces are missing Jun 23, 2024
@luanpotter
luanpotter marked this pull request as ready for review June 23, 2024 03:35

@spydon spydon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lgtm!

@luanpotter
luanpotter merged commit 28fd2a0 into main Jun 23, 2024
@luanpotter
luanpotter deleted the luan.spaces branch June 23, 2024 14:07
spydon added a commit that referenced this pull request Oct 1, 2026
The rich text document pipeline (`DocumentRoot`, `ParagraphNode`,
`HeaderNode`, and therefore `flame_markdown`) was the last place in
Flame that broke lines itself. Its layout builder split on ASCII spaces,
re-measured every tentative line with a fresh `TextPainter`, could not
wrap CJK, threw on `TextAlign.justify`, lost kerning and ligatures at
span boundaries, and needed glue heuristics for punctuation that were
patched again in #4060 after #3192 and #3939.

Since `InlineTextStyle` always renders through `TextPaint`, that
pipeline can hand the whole block to Flutter's paragraph engine without
losing any capability:

- `InlineTextNode.layoutBuilder` is replaced by `InlineSpan
toInlineSpan()`. `PlainTextNode` and `GroupTextNode` build a `TextSpan`
tree, the wrapper nodes delegate to their child.
- `TextBlockNode.format` lays out one `TextPainter` per block and
returns the new `ParagraphTextElement`, which exposes `lines` and
`lineMetrics`. Justify, CJK wrapping, hard line breaks, and shaping
across differently styled spans now work.
- `TextElement` gained a no-op `dispose()`, forwarded by `GroupElement`,
which `ParagraphTextElement` uses to release its painter.
- `TextBoxComponent` lays out a dedicated painter for wrapping and reads
lines through `ParagraphTextElement`. It previously called
`layout(maxWidth:)` on the painter shared through the `TextPaint` cache,
so the wrapped layout leaked into later `format` calls for the same
string. Per-line rendering, `align`, `growingBox` and the typewriter
effect are unchanged.

---------

Co-authored-by: Luan Nico <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants