Skip to content

fix: align styled-text style positions with grapheme clusters#1115

Open
greymoth-jp wants to merge 2 commits into
kaplayjs:masterfrom
greymoth-jp:fix/styled-text-grapheme-index
Open

fix: align styled-text style positions with grapheme clusters#1115
greymoth-jp wants to merge 2 commits into
kaplayjs:masterfrom
greymoth-jp:fix/styled-text-grapheme-index

Conversation

@greymoth-jp

Copy link
Copy Markdown

compileStyledText builds charStyleMap keyed by renderText.length, i.e. UTF-16 code units, while emitting the text one code unit at a time. formatText then splits the rendered text with runes() and applies styles with charStyleMap[cursor], where cursor is a grapheme index.

For plain ASCII those two indexings match, but they drift apart as soon as the text contains a character that is more than one code unit: an emoji, a ZWJ sequence, a skin-tone or flag sequence, or an astral-plane CJK ideograph (CJK Extension B, e.g. names written with 𠮷). After such a character every following style is shifted, so it lands on the wrong grapheme or is dropped.

A couple of concrete cases:

  • 😀[c]x[/c] keys the style at 2, but runes("😀x") puts x at grapheme index 1, so the style is lost.
  • [c]😀b[/c] produces keys {0,1,2} for a string runes() sees as two graphemes.

The fix makes compileStyledText walk grapheme clusters with the same runes() helper formatText already uses, keying charStyleMap by grapheme index. The input is normalized to NFC up front (which runes() does internally too) so the slice lengths stay consistent. Escapes (\x) now also consume a whole grapheme rather than a single code unit.

Added tests/auto/styledText.spec.ts covering ASCII (unchanged), an emoji, an astral CJK character, and a ZWJ family sequence.

compileStyledText keyed charStyleMap by renderText.length (UTF-16 code
units) while formatText walks the rendered text with runes() and looks up
styles by grapheme index. For any emoji, ZWJ sequence or astral character
(e.g. a CJK Extension B ideograph), the two indexings diverge, so every
style after such a character was applied to the wrong grapheme or dropped.

Walk grapheme clusters via runes() and normalize to NFC so the style keys
line up with how formatText consumes them.

@dragoncoder047 dragoncoder047 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.

If you're going to use runes() like this, you should change it to have a parameter to just get the first rune and not all of them, otherwise the runtime will be $O(n^2)$ (up from $O(n)$!!) since it is now trying to split the entire remaining string into runes for every character in the string.

Also, using const { 0: grapheme } = ... instead of const [grapheme] = ... is faster and uses less memory since it doesn't create an iterator.

@greymoth-jp

Copy link
Copy Markdown
Author

Good catch. I added a firstRune() helper next to runes() that pulls just the first grapheme from the Intl.Segmenter iterator and returns on the first segment, so it no longer splits the whole remaining string on every character and the walk is back to O(n). compileStyledText now calls firstRune(text) directly, so there is no array or destructuring at the call site either. It falls back to runesLegacy()[0] where Intl.Segmenter is unavailable.

@dragoncoder047

Copy link
Copy Markdown
Contributor

I just thought of another optimization: what is effectively being done is that compileStyledText is chopping runes off incrementally, and then formatText is calling runes() again, so it's duplicating the work. Maybe the StyledTextInfo can be made to have an array of runes instead of a string which has to be cut up again.

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