editor: Fix cursor animation across width and scroll changes (#64184)

Follow-up to #63195.

This PR fixes the cursor animation problems reported in [this
comment](https://github.com/zed-industries/zed/pull/63195#issuecomment-5662566417):

- Block cursors snapped instead of animating when moving between
characters with different display widths, such as an ASCII character and
an emoji or CJK character.
- Cursor animation disappeared near the top and bottom scroll margins
when keyboard navigation triggered vertical autoscrolling.

The first issue happened because any cursor geometry size change was
treated as a reason to snap. Cursor movement now retargets the animation
when either the rectangle position or size changes, while geometry
changes without a logical cursor move still snap immediately.

The second issue happened because every viewport change cleared the
active animation. Scroll-only viewport changes now translate the current
and target corner positions, as well as the previous target geometry, by
the scroll pixel delta before retargeting. This preserves the spring's
motion and velocity across autoscroll. Actual layout changes, including
text-area size, line height, and font advance changes, continue to snap.

Regression coverage includes:

- Both narrow-to-wide and wide-to-narrow cursor transitions, including
convergence to the new rectangle.
- Cursor movement in both directions while the viewport scrolls.
- Scrolling during an active animation without clearing its position or
velocity.
- Layout changes that must still snap.
- An editor-level keyboard autoscroll test using a vertical scroll
margin of 3.

Validation:

- `cargo test -p editor cursor_animation --lib`
- `cargo fmt --all --check`
- `cargo build -p zed --bin zed --profile release-fast`
- Manual verification with the release-fast binary

Release Notes:

- Fixed cursor animations when moving between characters of different
widths and during keyboard-triggered autoscrolling.

---------

Co-authored-by: Christopher Biscardi <chris@christopherbiscardi.com>
This commit is contained in:
CainandChristopher Biscardi authored and GitHub committed 2026-09-16 23:26:23 +00:00
1 parent bbacc15a4f
commit 5428eb8399
2 files changed
+339 -14

No files matched your search

+276 -14
View File
@@ -73,6 +73,13 @@ impl CursorViewport {
&& f32::from(self.line_height).is_finite()
&& f32::from(self.em_advance).is_finite()
}
fn has_same_layout(self, other: Self) -> bool {
self.content_origin == other.content_origin
&& self.text_bounds == other.text_bounds
&& self.line_height == other.line_height
&& self.em_advance == other.em_advance
}
}
#[derive(Clone, Copy, Debug, Default, PartialEq)]
@@ -400,6 +407,11 @@ impl CursorAnimationStates {
self.newest_selection_id = None;
self.newest_state = None;
}
#[cfg(test)]
pub(crate) fn has_active_animation(&self) -> bool {
self.states.values().any(|state| state.active)
}
}
impl Default for CursorAnimationState {
@@ -436,20 +448,40 @@ impl CursorAnimationState {
return None;
}
let Some(previous_geometry) = self.target_geometry else {
let Some(mut previous_geometry) = self.target_geometry else {
self.snap(logical_position, target_geometry, viewport, now);
return None;
};
let Some(previous_viewport) = self.last_viewport else {
self.snap(logical_position, target_geometry, viewport, now);
return None;
};
let logical_position_changed = self.last_logical_position != Some(logical_position);
let target_origin_changed = !previous_geometry.has_same_origin(target_geometry);
let viewport_changed = self.last_viewport != Some(viewport);
let geometry_size_changed = !previous_geometry.has_same_size(target_geometry);
let viewport_changed = previous_viewport != viewport;
if viewport_changed
|| geometry_size_changed
|| (!logical_position_changed && target_origin_changed)
{
if viewport_changed {
if !previous_viewport.has_same_layout(viewport) {
self.snap(logical_position, target_geometry, viewport, now);
return None;
}
self.translate(AnimationPoint {
x: (previous_viewport.scroll_pixel_position.x - viewport.scroll_pixel_position.x)
as f32,
y: (previous_viewport.scroll_pixel_position.y - viewport.scroll_pixel_position.y)
as f32,
});
if let Some(target_geometry) = self.target_geometry {
previous_geometry = target_geometry;
}
}
let target_origin_changed = !previous_geometry.has_same_origin(target_geometry);
let geometry_size_changed = !previous_geometry.has_same_size(target_geometry);
let geometry_changed = target_origin_changed || geometry_size_changed;
if !logical_position_changed && geometry_changed {
self.snap(logical_position, target_geometry, viewport, now);
return None;
}
@@ -458,7 +490,7 @@ impl CursorAnimationState {
self.last_viewport = Some(viewport);
self.target_geometry = Some(target_geometry);
if target_origin_changed {
if logical_position_changed && geometry_changed {
let elapsed = if self.active {
self.elapsed_since_last_frame(now)
} else {
@@ -479,6 +511,16 @@ impl CursorAnimationState {
})
}
fn translate(&mut self, delta: AnimationPoint) {
for corner in &mut self.corners {
corner.current_position = corner.current_position + delta;
corner.target_position = corner.target_position + delta;
}
if let Some(target_geometry) = &mut self.target_geometry {
target_geometry.origin = target_geometry.origin + delta;
}
}
pub(crate) fn reset(&mut self) {
self.target_geometry = None;
self.last_logical_position = None;
@@ -562,9 +604,13 @@ mod tests {
use gpui::size;
fn bounds(x: f32, y: f32) -> Bounds<Pixels> {
bounds_with_width(x, y, 2.0)
}
fn bounds_with_width(x: f32, y: f32, width: f32) -> Bounds<Pixels> {
Bounds {
origin: point(px(x), px(y)),
size: size(px(2.0), px(20.0)),
size: size(px(width), px(20.0)),
}
}
@@ -586,6 +632,32 @@ mod tests {
LogicalCursorPosition { row, column }
}
fn assert_converges_to(
state: &mut CursorAnimationState,
logical_position: LogicalCursorPosition,
target_bounds: Bounds<Pixels>,
viewport: CursorViewport,
now: Instant,
) {
for frame in 1..30 {
state.update(
logical_position,
target_bounds,
viewport,
now + Duration::from_millis(frame * 16),
);
}
assert!(!state.active);
let target_geometry = CursorGeometry::from_bounds(target_bounds);
assert_eq!(
state.corners.map(|corner| corner.current_position),
state
.corners
.map(|corner| corner.destination(target_geometry))
);
}
#[test]
fn spring_converges() {
let mut spring = DampedSpringAnimation {
@@ -734,6 +806,96 @@ mod tests {
assert!(corners[0].x < px(4.0));
}
#[test]
fn moving_to_a_wider_cursor_animates_and_converges() {
let now = Instant::now();
let mut state = CursorAnimationState::default();
state.update(
logical_position(0, 0),
bounds_with_width(0.0, 0.0, 8.0),
viewport(0.0),
now,
);
assert!(
state
.update(
logical_position(0, 1),
bounds_with_width(8.0, 0.0, 16.0),
viewport(0.0),
now,
)
.is_some()
);
assert!(state.active);
assert_converges_to(
&mut state,
logical_position(0, 1),
bounds_with_width(8.0, 0.0, 16.0),
viewport(0.0),
now,
);
}
#[test]
fn moving_to_a_narrower_cursor_animates_and_converges() {
let now = Instant::now();
let mut state = CursorAnimationState::default();
state.update(
logical_position(0, 0),
bounds_with_width(0.0, 0.0, 16.0),
viewport(0.0),
now,
);
assert!(
state
.update(
logical_position(0, 1),
bounds_with_width(16.0, 0.0, 8.0),
viewport(0.0),
now,
)
.is_some()
);
assert!(state.active);
assert_converges_to(
&mut state,
logical_position(0, 1),
bounds_with_width(16.0, 0.0, 8.0),
viewport(0.0),
now,
);
}
#[test]
fn geometry_change_without_logical_movement_snaps() {
let now = Instant::now();
let mut state = CursorAnimationState::default();
state.update(
logical_position(0, 0),
bounds_with_width(0.0, 0.0, 8.0),
viewport(0.0),
now,
);
assert!(
state
.update(
logical_position(0, 0),
bounds_with_width(4.0, 0.0, 16.0),
viewport(0.0),
now + Duration::from_millis(16),
)
.is_none()
);
assert!(!state.active);
assert_eq!(state.corners[0].current_position.x, 4.0);
assert_eq!(state.corners[1].current_position.x, 20.0);
}
#[test]
fn long_movement_ranks_leading_corners_ahead_of_trailing_corners() {
let now = Instant::now();
@@ -789,7 +951,35 @@ mod tests {
}
#[test]
fn viewport_movement_snaps_even_during_animation() {
fn moving_down_while_the_viewport_scrolls_starts_animation() {
let now = Instant::now();
let mut state = CursorAnimationState::default();
state.update(logical_position(0, 0), bounds(0.0, 0.0), viewport(0.0), now);
assert!(
state
.update(logical_position(1, 0), bounds(0.0, 0.0), viewport(1.0), now,)
.is_some()
);
assert!(state.active);
}
#[test]
fn moving_up_while_the_viewport_scrolls_starts_animation() {
let now = Instant::now();
let mut state = CursorAnimationState::default();
state.update(logical_position(1, 0), bounds(0.0, 0.0), viewport(1.0), now);
assert!(
state
.update(logical_position(0, 0), bounds(0.0, 0.0), viewport(0.0), now,)
.is_some()
);
assert!(state.active);
}
#[test]
fn scrolling_during_animation_preserves_motion_and_velocity() {
let now = Instant::now();
let mut state = CursorAnimationState::default();
state.update(logical_position(0, 0), bounds(0.0, 0.0), viewport(0.0), now);
@@ -799,7 +989,20 @@ mod tests {
viewport(0.0),
now,
);
state.update(
logical_position(0, 10),
bounds(100.0, 0.0),
viewport(0.0),
now + Duration::from_millis(16),
);
assert!(state.active);
let positions_before_scroll = state.corners.map(|corner| corner.current_position);
let velocities_before_scroll = state.corners.map(|corner| {
(
corner.horizontal_animation.velocity,
corner.vertical_animation.velocity,
)
});
assert!(
state
@@ -809,10 +1012,69 @@ mod tests {
viewport(1.0),
now + Duration::from_millis(16),
)
.is_none()
.is_some()
);
assert!(!state.active);
assert_eq!(state.corners[0].current_position.y, -20.0);
assert!(state.active);
assert_eq!(
state.corners.map(|corner| corner.current_position),
positions_before_scroll.map(|position| AnimationPoint {
x: position.x,
y: position.y - 20.0,
})
);
assert_eq!(
state.corners.map(|corner| (
corner.horizontal_animation.velocity,
corner.vertical_animation.velocity,
)),
velocities_before_scroll
);
}
#[test]
fn layout_changes_snap_even_during_animation() {
let now = Instant::now();
for changed_viewport in [
CursorViewport {
line_height: px(24.0),
..viewport(0.0)
},
CursorViewport {
text_bounds: Bounds {
origin: point(px(0.0), px(0.0)),
size: size(px(700.0), px(500.0)),
},
..viewport(0.0)
},
CursorViewport {
em_advance: px(10.0),
..viewport(0.0)
},
] {
let mut state = CursorAnimationState::default();
state.update(logical_position(0, 0), bounds(0.0, 0.0), viewport(0.0), now);
state.update(
logical_position(0, 10),
bounds(100.0, 0.0),
viewport(0.0),
now,
);
assert!(state.active);
assert!(
state
.update(
logical_position(0, 10),
bounds(100.0, 0.0),
changed_viewport,
now + Duration::from_millis(16),
)
.is_none()
);
assert!(!state.active);
assert_eq!(state.corners[0].current_position.x, 100.0);
}
}
#[test]
+63
View File
@@ -3756,6 +3756,69 @@ async fn test_autoscroll(cx: &mut TestAppContext) {
});
}
#[gpui::test]
async fn test_cursor_animation_remains_active_during_keyboard_autoscroll(cx: &mut TestAppContext) {
init_test(cx, |_| {});
update_test_editor_settings(cx, &|settings| {
settings.cursor_animation.get_or_insert_default().enabled = Some(true);
});
let mut cx = EditorTestContext::new(cx).await;
let line_height = cx.update_editor(|editor, window, cx| {
editor.set_vertical_scroll_margin(3, cx);
editor.set_cursor_shape(CursorShape::Block, cx);
editor
.style(cx)
.text
.line_height_in_pixels(window.rem_size())
});
let window = cx.window;
cx.simulate_window_resize(window, size(px(1000.0), 8.0 * line_height));
let text = (0..40)
.map(|row| format!("line {row}"))
.collect::<Vec<_>>()
.join("\n");
cx.set_state(&format!("ˇ{text}"));
cx.update(|window, cx| {
window.refresh();
let _ = window.draw(cx);
});
let mut previous_scroll_y = 0.0;
let mut observed_autoscroll = false;
for _ in 0..20 {
cx.update_editor(|editor, window, cx| {
editor.move_down(&Default::default(), window, cx);
});
cx.update(|window, cx| {
window.refresh();
let _ = window.draw(cx);
});
let (scroll_y, animation_active) = cx.update_editor(|editor, window, cx| {
(
editor.snapshot(window, cx).scroll_position().y,
editor.cursor_animations.has_active_animation(),
)
});
if scroll_y > previous_scroll_y {
assert!(
animation_active,
"cursor animation should remain active when keyboard movement autoscrolls"
);
observed_autoscroll = true;
break;
}
previous_scroll_y = scroll_y;
}
assert!(
observed_autoscroll,
"expected cursor movement to autoscroll"
);
}
#[gpui::test]
async fn test_autoscroll_relative(cx: &mut TestAppContext) {
init_test(cx, |_| {});