fix(ime): render preedit inline and stop Windows key-repeat from stalling redraws - #1846
Closed
raphamorim wants to merge 8 commits into
Closed
raphamorim wants to merge 8 commits into
raphamorim wants to merge 8 commits into
Conversation
The v4 grid renderer (build_row_bg/fg in grid_emit) reads cells straight from terminal state and has no hook for IME composition, so preedit text was invisible past the first char that landed on the cursor cell. Plumb a PreeditOverlay through the row emit pass so the composing string actually paints, and shape it like wezterm: every composition cell takes the cursor color as a block bg with the glyph inverted on top, the IME caret breaks the block as a thin vertical beam, and CJK wide chars stay contiguous (Spacer cells emit no glyph so the wide leading char's two-cell advance covers the continuation without literal-space gaps). - ime: tighten Preedit::new (reject non-char-boundary byte_offset); pin cursor anchor for the active session so a composition started right after Home/Ctrl-A or a commit doesn't snap to the previous line end while the PTY echo catches up. - application: mark TerminalDamage::Full on preedit change so the overlay refreshes — the grid would otherwise keep its previous CPU buffers. - renderer/preedit: PreeditOverlay maps the composition string onto visible cells, tracks the IME cursor position, and exposes a per-row "any preedit?" bit for the build_row_* fast paths. - renderer/mod (Renderer::run): capture the cursor the user last saw before the snapshot overwrites it; if a preedit is active, swap in the anchored position via Ime::preedit_anchor_or_init. - grid_emit: PreeditRow threaded through build_row_bg/fg; build_row_bg overrides bg to the cursor color on preedit cells; build_row_fg breaks runs at preedit boundaries and emits each composition char as a single-cell shape with bg-color fg + BOOL_IS_CURSOR_GLYPH so the cell-text shader keeps our inverse color instead of swapping it. Plain font attrs are forced so an italic prompt segment under the cursor doesn't bleed into the composing text. Underlines / strikethroughs skip preedit cells. - DecorationStyle::ImeCaret: thin full-cell-height vertical bar rasterized as a grayscale sprite, emitted at the IME cursor column to break the block where arrow-key navigation lands. Co-authored-by: KOGA Mitsuhiro <shiena.jp@gmail.com>
c39803c pinned the preedit overlay to the cursor captured at composition start so it would not drift as the PTY thread caught up with a just-committed run or a cursor-movement key. Pinning cures the drift but freezes whichever transient position the first frame happens to sample: - seeding from the previous render's cursor leaves the anchor on the pre-movement line end when a Home (or Up) after a wrapped line coalesces with the preedit event into one frame, so the overlay snaps to the wrap's end; - seeding from the snapshot cursor instead can catch the PTY mid-escape-sequence for Home (e.g. row-move processed, column move not yet), freezing the overlay a few cells to the right of the real line end. Either seed trades drift for a stuck-at-wrong-position bug whose exact symptom depends on which phase of PTY processing the first frame lands on. wezterm's renderer reads `term.cursor_pos()` on every frame and accepts the short flicker while PTY output settles; the overlay always converges to the right place. Match that: drop the anchor + pinning machinery and let the overlay follow whatever `terminal_snapshot.cursor` reports each frame.
…s live Under sustained IME key-repeat (corvus-skk hiragana holding a vowel) each tick floods the message queue with WM_KEYDOWN + the three WM_IME_* messages + a TerminalDamaged user event. The drain loop never sees an empty queue, so `about_to_wait` — which is the only site that calls `scheduler.update()` — never fires. The render scheduler timer therefore never expires, and even if it did, its RedrawRequested would still depend on WM_PAINT, the lowest-priority slot in a Win32 message queue and also starved by the same flood. Net effect: every `あ` is pasted to the PTY and echoed by the shell while held, but the screen only repaints when the key is released and the queue finally drains. Register a custom `REDRAW_REQUESTED_MSG` via `RegisterWindowMessageA` and post it from `Window::request_redraw`. Regular posted messages are returned by `PeekMessageW` ahead of the synthesized WM_PAINT, so the paint trigger interleaves with the IME flood. On the application side, `RioEvent::TerminalDamaged` skips the scheduler timer and calls `request_redraw` directly — vblank-rate pacing still belongs to the DwmFlush VSync worker via the dirty flag, so we aren't burning frames.
The previous commit's `Window::request_redraw` both posts `REDRAW_REQUESTED_MSG` *and* raises the DwmFlush worker's dirty flag, so each paint request caused two `RedrawRequested` dispatches per vblank: one via the custom message, one via the worker's `RedrawWindow(RDW_INVALIDATE)` -> `WM_PAINT` path. The second one is a no-op inside per-context render (the dirty flag was already cleared by the first pass) but still burns a frame of `begin_render` / `pre_present_notify` / present work. Clear the worker's dirty flag from the `REDRAW_REQUESTED_MSG` handler so the next tick sees it false and skips the redundant invalidate. Also drop the `present_after_input` fallback (and the `last_input_timestamp` / `mark_input_received` plumbing that fed it) since rio already drives redraws explicitly via `request_redraw` on every event that can mutate the terminal — the fallback was driving another 60 fps stream of invalidations during any input window, stacking on top of the custom-message paints.
…handshake
Tracing the WndProc for a held 'a' in corvus-skk hiragana
direct-input showed a very sharp pattern: WM_IME_STARTCOMPOSITION
was delivered immediately after WM_KEYDOWN, but the paired
WM_IME_COMPOSITION only arrived ~95–100 ms later, with the main
thread completely idle in between. The OS auto-repeat (~30 Hz)
kept queuing WM_KEYDOWN messages with accumulating repeat counts
(lparam low word 2, 4, …) while we were stuck waiting, so the
user only saw ~10 cps. Forwarding the IME messages to
`DefWindowProc` runs a synchronous handshake with the TIP — that
handshake is the 100 ms wait.
Return 0 for both START and ENDCOMPOSITION the way wezterm does
for ENDCOMPOSITION. The composition result still arrives via
`WM_IME_COMPOSITION`, just without the handshake stall, and the
per-keystroke cycle drops to ~31 ms — matching the OS auto-repeat
cadence (confirmed on a corvus-skk hiragana trace: 21.502086 s
START → 21.502169 s COMPOSITION, i.e. 0.08 ms vs. the prior
~98 ms).
Drop the `Ime::Enabled` / `Ime::Disabled` dispatches that START /
END used to send: the app's handler only toggled an unread
`enabled` flag, so for IMEs that fire this whole trio every
keystroke we were paying one full event-handler round-trip per
press for nothing.
The pre-commit `Ime::Preedit("")` in the GCS_RESULTSTR branch is
intentionally kept: most IMEs (MS-IME, Google Japanese Input,
ATOK) fire WM_IME_COMPOSITION on confirm with only GCS_RESULTSTR
— no GCS_COMPSTR — so the GCS_COMPSTR branch below never runs and
the app's `ime.preedit` would otherwise stay `Some(...)` from the
last preedit update. `process_key_event` short-circuits while a
preedit is active, so without this clear every keystroke after a
commit is silently dropped. For direct-input IMEs (corvus-skk
hiragana) that never had a preedit, the app's handler skips
damage/redraw because `None != None` is false — the extra event
is one no-op handler call, paid only on commit (not auto-repeat).
WM_IME_ENDCOMPOSITION no longer dispatches Ime::Disabled, which was the last guaranteed preedit-clear on the cancel path. If an IME ends composition without a final WM_IME_COMPOSITION carrying an empty GCS_COMPSTR (Esc, focus loss, IME switch), the app's ime.preedit stays Some and process_key_event short-circuits every subsequent key press — dead keyboard until the next composition. Send a synthetic empty Preedit when ENDCOMPOSITION fires with a live preedit and no composed result to recover.
The block wrote renderable_content.cursor.content / is_ime_enabled every frame, but nothing has read either field since the cursor pipeline moved to PanelFrame uniforms — and the preedit overlay now renders the whole composition inline, so the single-char cursor swap it fed is superseded anyway. Drop the now-unread is_ime_enabled field with it.
map_or(false, ..) -> is_some_and, drop two usize -> usize casts, and allow too_many_arguments on emit_ime_caret to match the neighboring emit helpers.
raphamorim
marked this pull request as draft
August 9, 2026 22:34
Owner
Author
|
Closing in favor of a from-scratch implementation with a corrected design (single-row slide-left layout, grapheme-cluster shaping, contrast-safe caret, lifecycle handling); the review punch list in the body above documents why. The Windows redraw-pump work will be redone separately. |
6 tasks done
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
3 participants
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.
Supersedes #1557 (commits by @Tryanks and @shiena, rebased onto main's grid pipeline, plus review fixes). Draft — deep review 2026-08-10 found blocking defects in both halves; punch list below. CI is green, but CI cannot see any of these.
What this PR does
WM_PAINT, plus IME message-handshake perf changes.Blockers found by deep review (must fix before un-drafting)
Rendering half:
named_colors.cursoron a block filled with the same color (grid_emit.rs:2340 vs 1108-1110). Arrow-keying between Japanese conversion segments — the reason the caret exists — shows nothing.cursor.state.posis unshifted by display offset, and preedit forces a Block cursor before the visibility check (grid_emit.rs:527-531), so scrolling back paints the overlay + cursor over history at a stale position. Should suppress whendisplay_offset > 0.Ime::Preeditmutates only the current context and nothing clears it on switch; the old split keeps a frozen block and swallows Enter/Backspace/Ctrl-C on refocus (screen/mod.rs:694-696) until a new composition clears it. Needs preedit-clear on context switch.Ime::Disabledclears preedit without damage/redraw (application.rs:1983-1991) — switching input source mid-composition leaves a ghost overlay.e+U+0302 renders as two cells. Needs grapheme segmentation in the overlay model.Windows half:
7. Lost redraw in the buffered branch — the
REDRAW_REQUESTED_MSGhandler callsclear_dirty(disarming the vsync backstop), then parks the request inredraw_requested, which onlyWM_PAINTreads — and nothing invalidates (event_loop.rs:2697-2712). Needs theRDW_INTERNALPAINTre-arm the WM_PAINT arm has.8. Double-paint race — vblank worker
swap(false)+RedrawWindowcan fire before the posted MSG dispatches (likely exactly when the queue is flooded — the PR's own premise), yielding two paints per request (vsync.rs:114-141).9. In-handler
request_redrawre-posts unpaced (clear-at-entry) and double-dispatches via the WM_PAINT re-arm — animation loops get 2× events with no vblank pacing.Also to fold in
WM_IME_SETCONTEXTmasking — it masksISC_SHOWUICOMPOSITIONWINDOWoff wParam, but the flag lives in lParam (upstream winit fixed this; the fork predates it). The mask is currently a no-op, which means the OS composition window suppression on main has never worked — and this fix must ship with this PR, never before it, or Windows users lose their only visible composition UI.DefWindowProcforWM_IME_STARTCOMPOSITION/ENDCOMPOSITION— verified against both wezterm (no STARTCOMPOSITION handler at all) and upstream winit (DefWindowProc for both): consuming them has no precedent and risks candidate-window placement under legacy IMM32 IMEs. With the SETCONTEXT mask fixed, suppression works the documented way.PostMessageWresult (failure currently wedges the coalescing latch — one line), gate the MSG dispatch on window visibility, skip emptyCommit("")on cancel-with-GCS_RESULTSTR-size-0 IMEs.PreeditOverlay::getlacks a column bound; ImeCaret atlas key omitscell_h; overlay allocates rows×cols every frame (~190 KB at 300×80) — a sparse representation is O(preedit).What held up under attack
Run-cache keying (no poisoning either direction — verified against the main emit loop's hasher scheme),
Preedit::newchar-boundary hardening (fixes a real panic vector from macOS surrogate-split offsets),BOOL_IS_CURSOR_GLYPHsemantics in both render backends, wide-char lead/spacer math, run-breaking and decoration suppression, damage on preedit change, the Hangul IME fixes surviving the merge, WM_PAINT validation, vsync thread lifecycle, and the ENDCOMPOSITION stale-preedit guard.Original verification:
cargo check/clippyclean, rio-window type-checks forx86_64-pc-windows-msvc, 182 rioterm tests pass.