perf: reduce idle-input CPU — region-gate pointer full-present + skip no-op PTY repaints (#459) - #464
Merged
Merged
Conversation
Mouse movement anywhere in the window forced a full GL clear + full cached-primitive repaint + full swap on every frame, because the FrameDamage axis's `force_full` OR'd in an unconditional `pointer_moving` (`ctx.input(|i| i.pointer.is_moving())`). This spikes CPU on slower hardware whenever the pointer moves, even over plain terminal content that changes no chrome pixels. Narrow it: pointer motion only forces a full present when the pointer is over a chrome-interactive region (menu/tab bar, pane border) or a pane-border drag is latched. The region test hit-tests the local `win.chrome_head_rects`/`chrome_border_rects` directly via `point_in_chrome_rects` -- NOT `self.is_chrome_interactive_at`, which is a no-op inside `update()` (the window is removed from `self.windows` for the frame). `border_drag_active` (the existing latch) covers a drag that moved the pointer off the ±3px sensor. This is a FrameDamage (present-scope) change only. It does not touch the frame-scheduling path (egui-winit's unconditional repaint on CursorMoved still schedules every frame), so `write_input_to_terminal` and xterm mouse-tracking report generation run exactly as before -- input forwarding is never delayed (issue #459 point 4 constraint). Narrowing exposed two staleness cases the coarse `pointer_moving` previously masked; both fixed here by mirroring the #461 gutter fix: - The scrollbar thumb is painted on the plain egui painter with a hover-varying alpha, outside per-pane VBO damage. A hover-alpha change or a rendered->not-rendered vanish now forces one Full frame (request_repaint + PaneFrameDamage::Full), so a coincident Partial present (an unrelated pane's cursor blink) can't leave a stale/ ghosted thumb. Paint alpha and damage decision both use the same window-exit-corrected `latest_pos()`-folded hover signal, so they can't drift a frame apart on window-exit (the interact_pos() lag that bit #461). Pure decision functions (`pointer_forces_full_present`, `scrollbar_damage_decision`) extracted with unit tests.
The PTY consumer thread's trailing `post_event` unconditionally requested a GUI wake (`request_repaint_after(16ms)`) after every processed input, even when the input changed nothing the GUI renders. `InputEvent::Key` (and `FocusChange`) only write bytes to the child PTY fd via `write_raw_bytes` -- they mutate no emulator state; the visible change (the echo) arrives later via `pty_read_rx`, which requests its own, necessary repaint. So the per-input repaint was a wasted ~60Hz GUI wake during typing / mouse-tracking streams -- the second half of the idle-input CPU cost (the first being the GUI-thread schedule on the raw event; this removes the redundant PTY-thread wake stacked on top). `post_event` gains a `request_repaint: bool`: the window-command / command-event drains and the snapshot store ALWAYS run (never stranded), but the repaint request is skipped when the input changed nothing visible. `handle_input` now returns an `InputOutcome` (Repaint/NoRepaint/Closed); the `input_rx` arm skips the wake on NoRepaint. Mirrors the existing idle-compaction arm's "byte-identical snapshot -> no spurious wake" discipline, applied per-InputEvent-variant. Classification is a pure, exhaustive (no wildcard), unit-tested `input_event_needs_repaint` -- the single source of truth the side-effecting arms defer to, so they cannot drift, and a future `InputEvent` variant forces an explicit compile-time decision. NoRepaint: Key, FocusChange (child-fd writes), ExtractSelection (GUI blocks on clipboard_rx same-frame). Repaint: everything that mutates snapshot-visible state, plus RequestSearchBuffer (GUI polls its result on a later frame, so it needs a guaranteed wake -- a design-review finding that ruled out the naive read-only=>no-repaint reading). Verified: latency unchanged (bytes reach the child fd before the outcome is returned; the echo path is untouched); teardown paths and the snapshot store are unconditional; two adversarial review passes traced the two subtlest cases (scroll-to-bottom-on-input and focus cursor rendering) to ground and found both driven by separate, still-repainting paths.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR refines full-frame presentation for pointer interactions, classifies PTY input events to gate GUI wake-ups, and tracks scrollbar rendering and hover state across frames to detect damage accurately. ChangesRepaint and frame-damage refinement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InputEvent
participant handle_input
participant PTYConsumer
participant SnapshotStore
participant GUI
InputEvent->>handle_input: classify event
handle_input-->>PTYConsumer: InputOutcome
PTYConsumer->>SnapshotStore: publish snapshot and drain commands
alt Repaint or PTY read
PTYConsumer->>GUI: request repaint
else NoRepaint
PTYConsumer-->>GUI: no immediate wake-up
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
fredclausen
added a commit
that referenced
this pull request
Jul 28, 2026
The previous version of this document was a framing exercise arguing whether to move off egui. That question is now closed by maintainer ruling, so the document is rewritten as the plan an agent executes from, keeping only the context future agents need. Retained: the evidence trail (PRs #460/#461/#464), the catalogue of egui-internal dependencies that kept failing adversarial review, the architecture invariants that must survive, and the pointers. Added: - Measured numbers from the Task 121 harness (commit 0620cc6), including the finding that ChromeMode::Replay engages 0 times in 360 idle frames, and the inconvenient result that egui is only 21% of an idle frame while swap alone is 52%. The plan states plainly that the performance case is weak and the justification is architectural. - The per-OS-window partition: main window egui-free, auxiliary windows keep egui indefinitely. This is what reduces the bespoke text-input requirement from nine surfaces to four single-line fields. - Why the main window cannot simply drop egui (the band is an egui PaintCallback inside a background-layer shape range, not merely hosted by egui). - Chrome partition table with HOT/AUX buckets and text-input flags. - Platform strategy: winit and glutin stay, we write zero platform backends. Corrects an earlier error that implied Wayland-first with X11 behind a feature flag; winit already covers both, so platform ordering is a validation concern only. Also records that we do not need wezterm's four bespoke IME implementations. - Rejected alternatives with reasons, so they are not re-litigated: wezterm's window crate, native-toolkit-per-platform, cached-texture chrome, existing retained-mode crates, dropping the menu bar. - Six phases with numbered subtasks, an inventory of already-built reusable assets, and explicit abort criteria (subtask 2.6, the text field with IME, is the gate). - Correction to the earlier claim of leaked crate boundaries: the crate graph is clean. freminal-common, freminal-buffer and freminal-terminal-emulator have zero egui in Cargo.toml and zero live egui-typed code. The coupling is concentrated in the binary's god functions. Roadmap re-sequencing is deliberately deferred per maintainer instruction; the document notes the pressure points for whoever does it.
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
1 participant
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.
Summary
Two independent CPU-efficiency fixes from the issue #459 real-workload profiling investigation. Both target the "why does freminal draw CPU during ordinary interaction when ghostty/wezterm sit at 0.0" question.
Commit 1 —
perf: region-gate pointer-move full-present(a9fb1210)Mouse motion anywhere in the window forced a full GL clear + full cached-primitive repaint + full swap every frame, because the
FrameDamageaxis'sforce_fullOR'd in an unconditionalpointer_moving. Narrowed so pointer motion only forces a full present when the pointer is over a chrome-interactive region (menu/tab bar, pane border) or a pane-border drag is latched — motion over plain terminal content no longer forces a full present.win.chrome_head_rects/chrome_border_rectsdirectly (NOTself.is_chrome_interactive_at, which is a no-op insideupdate()because the window is removed fromself.windowsfor the frame — this trap was caught in review).Profiled: ~15% fewer on-CPU samples during continuous mouse motion over terminal content; chrome tessellation (
stroke_and_fill_path,add_line_loop) and GPU framebuffer-state (si_set_framebuffer_state,si_bind_ps_shader) costs measurably dropped.Commit 2 —
perf: skip GUI repaint for no-op PTY inputs(cb1d5155)The PTY consumer thread's trailing
post_eventunconditionally requested a GUI wake (request_repaint_after(16ms)) after every processed input — even inputs that change nothing the GUI renders.InputEvent::Key/FocusChangeonly write bytes to the child PTY fd; they mutate no emulator state, and the visible change (the echo) arrives later viapty_read_rx(which requests its own, necessary, repaint). So the per-input repaint was a wasted ~60Hz GUI wake during typing / mouse-tracking streams.post_eventgains arequest_repaint: boolgating only the wake — the window-command/command-event drains and the snapshot store always run (never stranded).handle_inputreturns anInputOutcomederived from a pure, exhaustive (no wildcard), unit-testedinput_event_needs_repaintclassifier — single source of truth, so a futureInputEventvariant forces an explicit compile-time decision.RequestSearchBufferis deliberately kept as Repaint (the GUI polls its result on a later frame, so it needs a guaranteed wake) — a design-review finding that ruled out the naive "read-only ⇒ no repaint" reading.Result: CPU now sits at 0.0 during typing where it previously drew steadily.
Validation
cargo test --all,cargo clippy --all-targets --all-features -- -D warnings,cargo machete,cargo fmt --check— all green.Known remaining (scoped, not in this PR)
Idle CPU now sits mostly at 0.0 but still walks to ~0.1 while actively moving the mouse / typing. This is the residual GUI-thread frame schedule: egui-winit returns
repaint:trueunconditionally on everyCursorMoved/KeyboardInput, andevent_loop.rsschedules a frame from it regardless of whether anything visible changes. Removing that (event-time input forwarding that bypasses the GUI frame schedule for PTY-bound events) is the next, larger piece of work — designed but deliberately deferred; thepost_eventfix here is the foundation it requires. Tracked in #459.Closes item 9 of #459 (partial — FrameDamage + post_event axes; frame-schedule axis tracked as follow-up).
Summary by CodeRabbit
Performance Improvements
Bug Fixes