base: Walk rows, not characters, for Inline text line bounds - #3070
Merged
Merged
Conversation
Inline::paint registered its line boxes by calling TextLayout::position_for_index twice per character, and each call scans the line's rows and glyphs, so every selectable inline cost O(chars × glyphs) per frame. Profiling the AI chat on iOS while scrolling a real conversation put that walk at 10% of all CPU — the largest single symbol — and at 3% even on plain synthetic paragraphs. Build the boxes from the wrapped line layouts instead: one box per row, from the row's start to its last glyph, extended by half a line height when another row follows, which is the width the character walk gave to a character whose successor sat on the next row. The row walk also covers the first glyph of a wrapped row, which the character walk credited to the end of the row before it (and so skipped a row holding only that glyph); the test pins the agreement on every row the old walk could see. Synthetic scroll benchmark (iPhone 17 Pro simulator, Release): total CPU over 20 s of continuous swiping 6.33 s -> 5.53 s; Inline::paint 0.35 s -> 0.15 s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HY6KNBpkTcKkW2PpjrFhji
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.
Description
Inline::paintregistered its selection line boxes by callingTextLayout::position_for_indextwice per character, and each call scans the line's rows and glyphs, so every selectable inline cost O(chars × glyphs) per frame.Profiling the Longbridge AI chat on iOS (Time Profiler, iPhone 17 Pro simulator, Release) while scrolling a real conversation put that walk at 10% of all CPU — the largest single symbol (
WrappedLineLayout::position_for_indexself time), and at 3% even on plain synthetic paragraphs.This builds the boxes from the wrapped line layouts instead: one box per row, from the row's start to its last glyph, extended by half a line height when another row follows — the width the character walk gave to a character whose successor sat on the next row, which the selection geometry was tuned against.
One behavioural difference, on purpose: the character walk placed a wrapped row's first character at the end of the row before it (that is where
position_for_indexreports it), so that row's box started one glyph in, and a row holding only that one glyph had no box at all. The row walk covers every row from its start. The new test pins agreement on every row the old walk could see (same top / bottom / right edge; left edge never further right).Measured on the same synthetic scroll benchmark (20 s of continuous finger swipes through 80 markdown messages, same gesture script, Release):
Inline::paintposition_for_indexselfOn real chat content (long paragraphs) the walk was 1.13 s of a 25 s window and is now 0.
How to Test
cargo test -p gpui-base --lib— includesline_bounds_tests::row_walk_matches_the_character_walk, which keeps the old character walk as an oracle and compares it against the row walk over wrapped, multi-line, CJK and trailing-newline texts at three wrap widths and two clip masks.TextView(drag across wrapped lines, across paragraphs, triple-click) behaves as before.Checklist
cargo runfor story tests related to the changes. (not run — no story renders this path differently; covered by the unit test)🤖 Generated with Claude Code
https://claude.ai/code/session_01HY6KNBpkTcKkW2PpjrFhji