Repository navigation
terminal: preserve grapheme data when widening wraps - #14552
Conversation
mitchellh
left a comment
mitchellh
left a comment
There was a problem hiding this comment.
Thank you so much for your good work. You're clearly a very good AI driver and I appreciate it. For this one, due to where this is touching things, I'm going to ask if we can run some ghostty-bench against the terminal stream (to exercise this) with a variety of ghostty-gen data ranging from natural to a purposely hostile mix of inputs to see what the throughput effect of this change is.
I'm asking because print is the HOTTEST path in libghostty and I want to make sure this is thoughtfully done.
With grapheme clustering (mode 2027) enabled, a cluster can widen after it has been printed. Codepoints that join a cluster without changing its width, such as a zero-width joiner, are stored on the cluster's single narrow cell. A later joining codepoint can then make the cluster two columns wide (for example a heart, a zero-width joiner, then thumbs up). When that happens in the last column and auto-wrap is enabled, the cluster wraps to the next line and its stored codepoints have to move with it. Copy them before wrapping instead of looking them up on the row above afterwards. That row isn't the original one when the wrap scrolls a single-row screen or a margin region, or doesn't scroll at all below the scroll region. So the codepoints were dropped, taken from an unrelated cell, or the lookup panicked. Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
Re-attach the grapheme codepoints copied across a widening wrap with a single setGraphemes call instead of appending them one at a time. Appending probed the grapheme map and could reallocate the chunk once per codepoint, which cost about 10% on input where every wrap carries a maximal cluster. Screen.setGraphemes mirrors appendGrapheme: on a full grapheme map or allocator it grows the page's grapheme capacity, reloads the cell and retries. Add a test that wraps onto a compacted page with no grapheme capacity so that path is covered. Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
28ca334 to
6dae79d
Compare
👍 Makes sense! Benchmark setup and result follows - let me know what you think about it and possible follow ups. Setup: Inputs (described here, see the attached benchmark script for details):
Commits:
Result: Median of 10 runs, change vs
Benchmark script: pr-14552-bench.py |
With grapheme clustering enabled, a cluster can widen after it has been printed. Codepoints that join a cluster without changing its width, such as a zero-width joiner, are stored on the cluster's single narrow cell. A later joining codepoint can then make the cluster two columns wide (for example a heart, a zero-width joiner, then thumbs up).
When that happens in the last column and auto-wrap is enabled, the cluster wraps to the next line and its stored codepoints have to move with it. Copy them before wrapping instead of looking them up on the row above afterwards: that row isn't the original one when the wrap scrolls a single-row screen or a margin region, or doesn't scroll at all below the scroll region.
To reproduce, open a window and run the below using a safe build:
AI disclosure: Initially created with the help of gpt-6 astra in codex. Iterated on, reviewed and tested by me.