perf(diffs/edit): Cut wrap-mode re-measurement on typing and scroll - #1011
Conversation
Type or scroll in a wrapped file with long lines: every keystroke and scroll re-measured wrap points character by character for each line, one Range.getBoundingClientRect() per character, making minified or long-line files sluggish. Three changes to wrap measurement and its caching: - Find each wrap point by binary-searching for the first grapheme whose rect top drops to the next visual line (tops are non-decreasing with text offset), replacing the per-character scan: O(wraps * log chars) measurements instead of O(chars). - On edits that renumber no lines, invalidate only the lines the edit rewrote instead of every line at or after it. Per-edit line deltas guard the multi-edit batch whose net delta cancels to zero while still shifting lines between the edits. - Keep wrap offsets across render-range syncs: they depend on line text, font metrics, and content width, not on rendered rows. The sites where those inputs change (document swap, font load, width resize, cleanup) now clear the cache themselves, with a 10k-line cap since entries persist across scrolls.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd2ebf605a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| #resetCache(): void { | ||
| this.#lineYCache.clear(); | ||
| this.#wrapLineOffsetsCache.clear(); | ||
| this.#lineElementsCache.clear(); | ||
| this.#lastAccessedCharX = undefined; |
There was a problem hiding this comment.
Invalidate wrap offsets across scroll-mode edits
When the same editor is toggled from wrap to scroll, this cache now survives the render sync, but #applyChange only deletes wrap offsets inside if (this.#isWrap). Editing a previously measured line while overflow is scroll therefore leaves stale offsets behind; toggling back to wrap reuses measurements for the old text, which misplaces wrapped carets/selections until some unrelated width/font/document invalidation happens.
Useful? React with 👍 / 👎.
amadeus
left a comment
There was a problem hiding this comment.
Seems good, but it's probably a bit above my head on how it works 😅
…1011) Type or scroll in a wrapped file with long lines: every keystroke and scroll re-measured wrap points character by character for each line, one Range.getBoundingClientRect() per character, making minified or long-line files sluggish. Three changes to wrap measurement and its caching: - Find each wrap point by binary-searching for the first grapheme whose rect top drops to the next visual line (tops are non-decreasing with text offset), replacing the per-character scan: O(wraps * log chars) measurements instead of O(chars). - On edits that renumber no lines, invalidate only the lines the edit rewrote instead of every line at or after it. Per-edit line deltas guard the multi-edit batch whose net delta cancels to zero while still shifting lines between the edits. - Keep wrap offsets across render-range syncs: they depend on line text, font metrics, and content width, not on rendered rows. The sites where those inputs change (document swap, font load, width resize, cleanup) now clear the cache themselves, with a 10k-line cap since entries persist across scrolls.
Type or scroll in a wrapped file with long lines: every keystroke and scroll re-measured wrap points character by character for each line, one Range.getBoundingClientRect() per character, making minified or long-line files sluggish.
Three changes to wrap measurement and its caching:
Two regression tests in
editorWrapCaretPosition.test.ts, backed by ameasurement counter added to the mock harness:
< 500rect measurements while thecaret still lands on the right visual row/column (old code: exactly 1000).
zero measurements (old code re-measured line 1 from scratch).