From ea544e5a17c9249ce35715654dbd81d81a3e1bd1 Mon Sep 17 00:00:00 2001 From: novakduc Date: Sun, 20 Sep 2026 12:14:06 +0700 Subject: [PATCH 1/2] input: Clear the blink state when the cursor stops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BlinkCursor::stop`, called from `on_blur`, resets the epoch but leaves `paused` and `visible` holding whatever the last `pause` set. A blur inside the 300ms pause window — tabbing away right after a keystroke — therefore leaves `paused == true` on a stopped cursor. The next `start` then hits `blink`'s `if self.paused` early return and never moves the epoch off zero, so the refocused input has a cursor that is not blinking. The defect is masked today, and masked by accident: `pause` acts on any cursor, blinking or not, so the next keystroke starts the loop again. That accident is itself a bug — it is what makes a programmatic `set_value` blink an unfocused input forever — and the next commit removes it. This one has to land first so that removing the mask does not turn a hidden defect into a visible one. Clear `paused` and `visible` alongside the epoch, so a stopped cursor carries nothing into the next focus. As a side effect the caret now appears the moment an input is focused, rather than starting from whatever `visible` the previous focus left behind. novakduc@arch --- crates/base/src/input/base/blink_cursor.rs | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/base/src/input/base/blink_cursor.rs b/crates/base/src/input/base/blink_cursor.rs index cece966321..3a8f82b0ae 100644 --- a/crates/base/src/input/base/blink_cursor.rs +++ b/crates/base/src/input/base/blink_cursor.rs @@ -39,8 +39,12 @@ impl BlinkCursor { self.blink(self.epoch, cx); } + /// Stop the blinking and clear the blink state, so the next [`Self::start`] + /// begins from a visible cursor instead of resuming a stale pause. pub(crate) fn stop(&mut self, cx: &mut Context) { self.epoch = 0; + self.paused = false; + self.visible = false; cx.notify(); } @@ -117,4 +121,28 @@ mod tests { cx.run_until_parked(); assert!(cursor.read_with(cx, |cursor, _| cursor.visible())); } + + #[gpui::test] + fn blurring_a_paused_cursor_leaves_the_next_focus_blinking(cx: &mut TestAppContext) { + let cursor = cx.new(|_| BlinkCursor::new()); + cursor.update(cx, |cursor, cx| cursor.start(cx)); + cx.run_until_parked(); + + // Typing pauses the blink, then the input is blurred before the pause + // elapses: tabbing away right after a keystroke does exactly this. + cursor.update(cx, |cursor, cx| cursor.pause(cx)); + cx.run_until_parked(); + cursor.update(cx, |cursor, cx| cursor.stop(cx)); + cx.run_until_parked(); + assert!(!cursor.read_with(cx, |cursor, _| cursor.visible())); + + // Focusing again shows the cursor and blinks it, rather than leaving a + // stale pause to swallow the start. + cursor.update(cx, |cursor, cx| cursor.start(cx)); + cx.run_until_parked(); + assert!(cursor.read_with(cx, |cursor, _| cursor.visible())); + cx.executor().advance_clock(INTERVAL); + cx.run_until_parked(); + assert!(!cursor.read_with(cx, |cursor, _| cursor.visible())); + } } From 5c2ead2dd93ffc74eb2f3511a97bb2bfdba70d6f Mon Sep 17 00:00:00 2001 From: novakduc Date: Sun, 20 Sep 2026 12:14:43 +0700 Subject: [PATCH 2/2] input: Do not start the blink cursor on an unfocused input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `InputState::set_value` pauses the blink cursor whether or not the input is focused, and a pause resumes blinking 300ms later. On an input that was never focused, or that was blurred, the pause is what *starts* the blink loop, and nothing ever stops it: the blur that would call `stop` has already happened. Every blink calls `cx.notify()`, and `InputState` observes its own cursor, so each blink repaints the whole view the input sits in. One programmatic `set_value` on an off-screen input therefore costs two full view repaints per second for the life of the window, and a pane that seeds twenty inputs pays it twenty times over. Nothing appears on screen: the input is not focused, so it draws no cursor. ### The fix `BlinkCursor` already uses `epoch == 0` as its "not blinking" sentinel — `new` starts there, `stop` resets to it, and `start` moves off it. `pause` now honours that sentinel and returns early, so only a cursor that is already blinking can be paused. A focused input is unaffected: `on_focus` calls `start`, which leaves the epoch at 1 or more, and there is no window in which a focused input has a zero epoch. The guard is in `pause` rather than at the call site because `pause_blink_cursor` has ten production callers across `state.rs` and `movement.rs`, plus two direct `BlinkCursor::pause` calls; guarding one entry point would leave the rest. It also matches the keystroke path, which already wraps its `pause` in `focus_handle.is_focused(window)`. This is also what stops masking the stale-pause defect fixed in the previous commit, which is why that one comes first. ### Verification - `cargo test -p gpui-base --lib` (991) and `cargo test -p gpui-component --lib` (547) pass; clippy and `cargo fmt --check` are clean at both commits. - The two new tests were confirmed red without the guard and green with it. - `repeated_pauses_keep_cursor_visible_until_idle` now starts the cursor before pausing it: it asserts the focused typing behaviour, and a never-started cursor no longer stands in for a focused one. - `test_multi_cursor_actions_reveal_hidden_carets` faked the hidden blink phase with a fresh `BlinkCursor`; it now starts one and advances the clock into the hidden phase instead. - `test_cursor_layout_consumer_updates_after_selection` was relying on the stray blink loop for the frame in which its consumer read the caret geometry. A notify sent from inside a draw marks the view dirty without asking for another frame, so the test now invalidates the input explicitly. novakduc@arch --- crates/base/src/input/base/blink_cursor.rs | 42 ++++++++++++ crates/base/src/input/base/state.rs | 74 +++++++++++++++++++--- 2 files changed, 108 insertions(+), 8 deletions(-) diff --git a/crates/base/src/input/base/blink_cursor.rs b/crates/base/src/input/base/blink_cursor.rs index 3a8f82b0ae..8ba85946e5 100644 --- a/crates/base/src/input/base/blink_cursor.rs +++ b/crates/base/src/input/base/blink_cursor.rs @@ -78,7 +78,17 @@ impl BlinkCursor { } /// Show the cursor immediately and restart the idle delay before blinking resumes. + /// + /// This is a no-op on a cursor that is not blinking. `epoch` is zero only + /// before the first [`Self::start`] and after [`Self::stop`], so a zero + /// epoch means the input is not focused, and there is no cursor on screen + /// to keep visible. Pausing it anyway would start a blink loop that no blur + /// is left to stop, and every blink repaints the view that the input is in. pub(crate) fn pause(&mut self, cx: &mut Context) { + if self.epoch == 0 { + return; + } + self.paused = true; self.visible = true; cx.notify(); @@ -102,11 +112,15 @@ impl BlinkCursor { mod tests { use super::*; use gpui::{AppContext as _, TestAppContext}; + use std::{cell::Cell, rc::Rc}; #[gpui::test] fn repeated_pauses_keep_cursor_visible_until_idle(cx: &mut TestAppContext) { let cursor = cx.new(|_| BlinkCursor::new()); assert!(!cursor.read_with(cx, |cursor, _| cursor.visible())); + // Only a focused input blinks, and only a blinking cursor pauses. + cursor.update(cx, |cursor, cx| cursor.start(cx)); + cx.run_until_parked(); for _ in 0..5 { cursor.update(cx, |cursor, cx| cursor.pause(cx)); cx.run_until_parked(); @@ -122,6 +136,34 @@ mod tests { assert!(cursor.read_with(cx, |cursor, _| cursor.visible())); } + #[gpui::test] + fn pausing_a_cursor_that_is_not_blinking_does_not_start_it(cx: &mut TestAppContext) { + // Never focused, so `start` was never called: the epoch is still zero. + let cursor = cx.new(|_| BlinkCursor::new()); + + // What a programmatic `set_value` on an unfocused input does. + cursor.update(cx, |cursor, cx| cursor.pause(cx)); + cx.run_until_parked(); + cx.executor().advance_clock(PAUSE_DELAY); + cx.run_until_parked(); + assert!(!cursor.read_with(cx, |cursor, _| cursor.visible())); + + let notifies = Rc::new(Cell::new(0usize)); + let counter = notifies.clone(); + let _observer = + cx.update(|cx| cx.observe(&cursor, move |_, _| counter.set(counter.get() + 1))); + cx.run_until_parked(); + + cx.executor().advance_clock(INTERVAL * 6); + cx.run_until_parked(); + + assert_eq!( + notifies.get(), + 0, + "a cursor that was never started is blinking, and every blink repaints the view" + ); + } + #[gpui::test] fn blurring_a_paused_cursor_leaves_the_next_focus_blinking(cx: &mut TestAppContext) { let cursor = cx.new(|_| BlinkCursor::new()); diff --git a/crates/base/src/input/base/state.rs b/crates/base/src/input/base/state.rs index 06638af857..431e582c1e 100644 --- a/crates/base/src/input/base/state.rs +++ b/crates/base/src/input/base/state.rs @@ -4856,6 +4856,12 @@ mod tests { cx.run_until_parked(); let input = input.unwrap(); let before = input.read_with(cx, |state, _| state.cursor_layout()); + // The caret geometry only exists once the input has painted, and a + // notify sent from inside a draw marks the view dirty without asking + // for another frame, so the consumer reads the geometry on the next + // invalidation rather than during the paint that produced it. + input.update(cx, |_, cx| cx.notify()); + cx.run_until_parked(); assert_eq!(observed.get(), before); window .update(cx, |_, _, cx| { @@ -4935,6 +4941,46 @@ mod tests { ); } + #[gpui::test] + fn test_set_value_on_unfocused_input_stays_quiet(cx: &mut TestAppContext) { + use std::{cell::Cell, rc::Rc}; + + cx.update(crate::init); + let mut input = None; + let window = cx.open_window(size(px(400.), px(100.)), |window, cx| { + input = Some(cx.new(|cx| crate::input::InputState::new(window, cx))); + gpui::EmptyView + }); + let input = input.unwrap(); + cx.run_until_parked(); + + let notifications = Rc::new(Cell::new(0)); + let count = notifications.clone(); + let _subscription = + cx.update(|cx| cx.observe(&input, move |_, _| count.set(count.get() + 1))); + cx.run_until_parked(); + + // Seeding a form is a write, so it notifies once. The input is not + // focused and draws no caret, so nothing may notify after that. + window + .update(cx, |_, window, cx| { + input.update(cx, |state, cx| state.set_value("seeded", window, cx)); + }) + .unwrap(); + cx.run_until_parked(); + let settled = notifications.get(); + + cx.executor() + .advance_clock(std::time::Duration::from_secs(3)); + cx.run_until_parked(); + + assert_eq!( + notifications.get(), + settled, + "an unfocused input is blinking, and every blink repaints the view it is in" + ); + } + #[gpui::test] fn test_input_does_not_invalidate_cached_parent_during_paint(cx: &mut TestAppContext) { use std::{cell::Cell, rc::Rc}; @@ -7421,13 +7467,25 @@ mod tests { let view = InputView::::new(cx); let mut cx = VisualTestContext::from_window(view.window_handle.into(), cx); setup_cursors(&mut cx, &view.input, "ab\na|b\nab"); - cx.update(|window, cx| { - view.input.update(cx, |state, cx| { - // Start each action in the hidden phase without depending on a - // key-down listener: actions and text input also arrive directly. - for action in 0..6 { + // Start each action in the hidden phase without depending on a + // key-down listener: actions and text input also arrive directly. + for action in 0..6 { + cx.update(|_, cx| { + view.input.update(cx, |state, cx| { state.blink_cursor = cx.new(|_| BlinkCursor::new()); - assert!(!state.blink_cursor.read(cx).visible()); + state.blink_cursor.update(cx, |cursor, cx| cursor.start(cx)); + }); + }); + cx.run_until_parked(); + cx.executor() + .advance_clock(std::time::Duration::from_millis(500)); + cx.run_until_parked(); + view.input.read_with(&cx, |state, cx| { + assert!(!state.blink_cursor.read(cx).visible(), "action {action}"); + }); + + cx.update(|window, cx| { + view.input.update(cx, |state, cx| { match action { 0 => state.add_cursor_above(&AddCursorAbove, window, cx), 1 => state.add_cursor_below(&AddCursorBelow, window, cx), @@ -7437,9 +7495,9 @@ mod tests { _ => state.backspace(&Backspace, window, cx), } assert!(state.blink_cursor.read(cx).visible(), "action {action}"); - } + }); }); - }); + } } #[gpui::test]