From 6ebb2bb36428caa90fd50c3ec8a4470ac2241732 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 23:18:33 -0700 Subject: [PATCH] perf(terminal): stop the IME candidate anchor forcing layout per compositionupdate --- .../terminal-ime-candidate-anchor.test.ts | 209 ++++++++++++++++++ .../terminal-ime-candidate-anchor.ts | 75 ++++++- 2 files changed, 274 insertions(+), 10 deletions(-) create mode 100644 src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.test.ts diff --git a/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.test.ts b/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.test.ts new file mode 100644 index 00000000000..55816bfb8e9 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.test.ts @@ -0,0 +1,209 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { IBuffer, IBufferCell, IBufferLine, Terminal } from '@xterm/xterm' +import { installTerminalImeCandidateAnchor } from './terminal-ime-candidate-anchor' + +const COLS = 80 +const ROWS = 24 +const CELL_WIDTH = 8 +const CELL_HEIGHT = 17 + +type AnchorHarness = { + terminal: Terminal + element: HTMLElement + style: { top: string; left: string } + counts: { rectReads: number; styleWrites: number } + setCursor: (cursorX: number, cursorY: number) => void + setLines: (lines: string[]) => void +} + +function makeLine(text: string): IBufferLine { + const chars = Array.from(text) + while (chars.length < COLS) { + chars.push(' ') + } + const cellAt = (column: number): IBufferCell | undefined => { + const char = chars[column] + return char === undefined + ? undefined + : ({ getWidth: () => 1, getChars: () => (char === ' ' ? '' : char) } as IBufferCell) + } + return { + isWrapped: false, + length: chars.length, + getCell: cellAt, + translateToString: (trimRight = false, start = 0, end = chars.length) => { + const result = chars.slice(start, end).join('') + return trimRight ? result.replace(/\s+$/, '') : result + } + } as IBufferLine +} + +function createHarness(): AnchorHarness { + const counts = { rectReads: 0, styleWrites: 0 } + const element = document.createElement('div') + const screen = document.createElement('div') + screen.className = 'xterm-screen' + element.appendChild(screen) + document.body.appendChild(element) + + screen.getBoundingClientRect = (): DOMRect => { + counts.rectReads++ + return { width: COLS * CELL_WIDTH, height: ROWS * CELL_HEIGHT } as DOMRect + } + + const style = { top: '', left: '' } + const textarea = { + isConnected: true, + style: new Proxy(style, { + set(target, key: string, value: string) { + counts.styleWrites++ + target[key as 'top' | 'left'] = value + return true + } + }) + } as unknown as HTMLTextAreaElement + + let lines = [''] + const buffer = { + baseY: 0, + cursorX: 0, + cursorY: 0, + length: ROWS, + getLine: (row: number) => (row < lines.length ? makeLine(lines[row] ?? '') : makeLine('')) + } as unknown as IBuffer + + const terminal = { + element, + textarea, + cols: COLS, + rows: ROWS, + buffer: { active: buffer } + } as unknown as Terminal + + return { + terminal, + element, + style, + counts, + setCursor: (cursorX: number, cursorY: number) => { + Object.assign(buffer, { cursorX, cursorY }) + }, + setLines: (next: string[]) => { + lines = next + } + } +} + +function fire(element: HTMLElement, type: string): void { + element.dispatchEvent(new Event(type)) +} + +/** One Hangul syllable: xterm sees compositionstart then one update per jamo. */ +function typeHangulSyllable( + harness: AnchorHarness, + cursorX: number, + updates = 3, + cursorY = 0 +): void { + harness.setCursor(cursorX, cursorY) + fire(harness.element, 'compositionstart') + for (let update = 0; update < updates; update++) { + fire(harness.element, 'compositionupdate') + } +} + +describe('installTerminalImeCandidateAnchor', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + document.body.innerHTML = '' + }) + + it('returns null before the terminal has opened its DOM', () => { + expect(installTerminalImeCandidateAnchor({ element: null } as unknown as Terminal)).toBeNull() + }) + + it('keeps forced layout at one read per composition across a Hangul burst', () => { + const harness = createHarness() + installTerminalImeCandidateAnchor(harness.terminal) + + for (let syllable = 0; syllable < 30; syllable++) { + typeHangulSyllable(harness, syllable) + } + + // 30 compositions x 4 events: reads collapse to one per composition, and the + // 90 updates re-write nothing because the anchor is already on the textarea. + expect(harness.counts.rectReads).toBe(30) + expect(harness.counts.styleWrites).toBe(31) + expect(harness.style.left).toBe(`${29 * CELL_WIDTH}px`) + }) + + it('does no work for Latin input, which fires no composition events', () => { + const harness = createHarness() + installTerminalImeCandidateAnchor(harness.terminal) + + fire(harness.element, 'keydown') + fire(harness.element, 'input') + + expect(harness.counts).toEqual({ rectReads: 0, styleWrites: 0 }) + }) + + it('re-corrects the anchor mid-composition after xterm rewrites the textarea', () => { + const harness = createHarness() + installTerminalImeCandidateAnchor(harness.terminal) + harness.setCursor(4, 2) + fire(harness.element, 'compositionstart') + + // xterm's own compositionupdate handler repositions from its uncorrected + // cursor; long CJK compositions depend on us winning that back. + harness.style.top = '0px' + harness.setCursor(6, 2) + fire(harness.element, 'compositionupdate') + + expect(harness.style).toEqual({ top: `${2 * CELL_HEIGHT}px`, left: `${6 * CELL_WIDTH}px` }) + }) + + it('re-measures mid-composition when the terminal is refit to new dimensions', () => { + const harness = createHarness() + installTerminalImeCandidateAnchor(harness.terminal) + fire(harness.element, 'compositionstart') + expect(harness.counts.rectReads).toBe(1) + + Object.assign(harness.terminal, { cols: 40 }) + fire(harness.element, 'compositionupdate') + + expect(harness.counts.rectReads).toBe(2) + }) + + it('coalesces the deferred Cursor Agent re-apply to one timer per burst', () => { + const harness = createHarness() + harness.setLines(['Cursor Agent', '', '→ hello']) + installTerminalImeCandidateAnchor(harness.terminal) + + typeHangulSyllable(harness, 0, 5, 1) + + expect(vi.getTimerCount()).toBe(1) + harness.style.top = '0px' + vi.runAllTimers() + expect(harness.style).toEqual({ top: `${2 * CELL_HEIGHT}px`, left: `${7 * CELL_WIDTH}px` }) + }) + + it('stops writing once the textarea has been detached', () => { + const harness = createHarness() + harness.setLines(['Cursor Agent', '', '→ hello']) + installTerminalImeCandidateAnchor(harness.terminal) + typeHangulSyllable(harness, 0, 1, 1) + + Object.assign(harness.terminal.textarea as object, { isConnected: false }) + const writesBeforeTimer = harness.counts.styleWrites + harness.style.top = '0px' + vi.runAllTimers() + + expect(harness.counts.styleWrites).toBe(writesBeforeTimer) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.ts b/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.ts index c8979c6f315..25b039480b6 100644 --- a/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.ts +++ b/src/renderer/src/lib/pane-manager/terminal-ime-candidate-anchor.ts @@ -1,6 +1,13 @@ import type { Terminal } from '@xterm/xterm' import { resolveCursorAgentImeAnchor } from './terminal-ime-anchor' +type ImeAnchorCellMetrics = { + cellWidth: number + cellHeight: number + cols: number + rows: number +} + /** * Keep the OS IME candidate window anchored to the cell the user is typing in. * @@ -10,6 +17,13 @@ import { resolveCursorAgentImeAnchor } from './terminal-ime-anchor' * force-sync after xterm's own composition handlers so the OS sees the corrected * location before it opens the candidate window. * + * xterm rewrites the textarea's position from its own `compositionupdate` + * handler, so the update listener has to stay — CJK IMEs compose long sequences + * (romaji→kana→kanji, pinyin phrases) during which xterm's uncorrected position + * would otherwise win. Instead the update path is made free of forced layout: + * cell metrics are measured once per composition and reused, and a style write is + * skipped when the inline value already matches (a CSSOM read, not a layout one). + * * Cell dimensions are derived from the public .xterm-screen element's bounds * (xterm sizes that element to cols*cellWidth × rows*cellHeight) rather than * poking `_core._renderService.dimensions` — keeps us on the public API surface @@ -24,14 +38,50 @@ export function installTerminalImeCandidateAnchor(terminal: Terminal): (() => vo } const screenElement = terminal.element.querySelector('.xterm-screen') const textarea = terminal.textarea - const handler = (): void => { + let metrics: ImeAnchorCellMetrics | null = null + let deferredApply: number | null = null + let deferredAnchor: { row: number; column: number } | null = null + + const measureCells = (): ImeAnchorCellMetrics | null => { if (!screenElement) { - return + return null } const rect = screenElement.getBoundingClientRect() const cellWidth = rect.width / terminal.cols const cellHeight = rect.height / terminal.rows if (!(cellWidth > 0) || !(cellHeight > 0)) { + return null + } + return { cellWidth, cellHeight, cols: terminal.cols, rows: terminal.rows } + } + + // Why: xterm rewrites these between our events, so compare against the live + // inline value — a CSSOM read, unlike getBoundingClientRect — and skip the + // write when it already matches instead of re-invalidating layout. + const writeOffset = (property: 'top' | 'left', value: string): void => { + if (textarea.style[property] !== value) { + textarea.style[property] = value + } + } + + const applyAnchor = (row: number, column: number, cells: ImeAnchorCellMetrics): void => { + writeOffset('top', `${row * cells.cellHeight}px`) + writeOffset('left', `${column * cells.cellWidth}px`) + } + + const handler = (event?: Event): void => { + if (!screenElement) { + return + } + // Re-measure per composition (font size or zoom may have changed since the + // last one); every compositionupdate then reuses it and forces no layout. + const staleMetrics = + !metrics || metrics.cols !== terminal.cols || metrics.rows !== terminal.rows + if (event?.type !== 'compositionupdate' || staleMetrics) { + metrics = measureCells() + } + const cells = metrics + if (!cells) { return } const buf = terminal.buffer.active @@ -48,19 +98,24 @@ export function installTerminalImeCandidateAnchor(terminal: Terminal): (() => vo row: buf.cursorY, column: Math.min(buf.cursorX, terminal.cols - 1) } - const applyAnchor = (): void => { - textarea.style.top = `${anchor.row * cellHeight}px` - textarea.style.left = `${anchor.column * cellWidth}px` + applyAnchor(anchor.row, anchor.column, cells) + // Why: xterm re-positions the textarea from a setTimeout(0) of its own after + // each compositionupdate, so the correction has to land after that timer — + // one pending timer per burst, re-reading the anchor when it fires. + if (!cursorAgentAnchor) { + return } - applyAnchor() - if (cursorAgentAnchor) { - window.setTimeout(() => { - if (textarea.isConnected) { - applyAnchor() + deferredAnchor = anchor + if (deferredApply === null) { + deferredApply = window.setTimeout(() => { + deferredApply = null + if (textarea.isConnected && metrics && deferredAnchor) { + applyAnchor(deferredAnchor.row, deferredAnchor.column, metrics) } }, 0) } } + terminal.element.addEventListener('compositionstart', handler) terminal.element.addEventListener('compositionupdate', handler) return handler