diff --git a/crates/bitty-runtime/src/runtime/animations.rs b/crates/bitty-runtime/src/runtime/animations.rs index 8d9c0e1d..f48fa0e4 100644 --- a/crates/bitty-runtime/src/runtime/animations.rs +++ b/crates/bitty-runtime/src/runtime/animations.rs @@ -421,11 +421,12 @@ impl PanelAnimator { return false; } // Restart (bounded) any existing animation of the same kind on the - // surface. Close and the gesture kinds also drop an in-flight bounds - // tween on that surface (the surface is going away, or the gesture - // re-arms its own geometry on the next present). Focus/Open/Workspace - // leave bounds interpolation untouched: they share the same surface - // without conflicting with its painted geometry (CTX-1335). + // surface. Close drops an in-flight bounds tween (the surface is going + // away); the gesture kinds replace it but carry its painted frame + // forward, because the gesture re-arms its own geometry on the next + // present. Focus/Open/Workspace leave bounds interpolation untouched: + // they share the same surface without conflicting with its painted + // geometry (CTX-1335). let replaces_bounds = matches!( kind, AnimationKind::Close @@ -433,6 +434,25 @@ impl PanelAnimator { | AnimationKind::Resize | AnimationKind::Drag ); + // A gesture re-arm replaces its bounds entry, but the painted frame + // must not jump back to the last committed target before the next + // present retargets. Carry the current interpolated frame forward as + // the new start, keeping the previous destination until the detector + // supplies the freshly committed one (CodeRabbit PR #2231, finding 1). + let carried_bounds = if matches!( + kind, + AnimationKind::Move | AnimationKind::Resize | AnimationKind::Drag + ) { + let destination = self + .active + .iter() + .find(|a| a.surface == surface && a.bounds.is_some() && !a.done(now)) + .and_then(|a| a.bounds) + .map(|(_, to)| to); + self.interpolated_bounds(surface, now).zip(destination) + } else { + None + }; self.active.retain(|a| { !(a.surface == surface && (a.kind == kind || (replaces_bounds && a.bounds.is_some()))) }); @@ -450,7 +470,7 @@ impl PanelAnimator { surface, started: now, duration, - bounds: None, + bounds: carried_bounds, ring: true, }); true @@ -668,7 +688,15 @@ impl PanelAnimator { /// dropped surface paints its committed end state immediately. pub fn drop_one_bounds(&mut self, now: Instant) -> bool { self.active.retain(|a| !a.done(now)); - if let Some(index) = self.active.iter().position(|a| a.bounds.is_some()) { + // Only bounds-only entries free a budget slot. A reused ring slot + // (Move or the Alt-drag Drag entry) hosts bounds too; removing it + // would delete the accepted fade this priority rule protects + // (CodeRabbit PR #2231, finding 2). + if let Some(index) = self + .active + .iter() + .position(|a| a.bounds.is_some() && !a.ring) + { self.active.remove(index); return true; } @@ -1174,6 +1202,56 @@ mod tests { ); } + #[test] + fn overflow_preserves_a_ring_entry_that_hosts_bounds() { + let now = t0(); + let mut anim = PanelAnimator::new(AnimationPolicy::default()); + let dragged = ViewId::new(35); + let bounds_only = ViewId::new(36); + + // The Alt-drag ring hosts its own bounds (one surface, one entry). + assert!(anim.trigger(AnimationKind::Drag, Some(dragged), now)); + assert!(anim.trigger_bounds( + AnimationKind::Move, + Some(dragged), + bounds(0, 0, 10, 10), + bounds(10, 10, 20, 20), + now, + )); + // A separate bounds-only tween is the only safe eviction candidate. + assert!(anim.trigger_bounds( + AnimationKind::Move, + Some(bounds_only), + bounds(0, 0, 10, 10), + bounds(10, 10, 20, 20), + now, + )); + assert_eq!(anim.active_count(now), 2); + + assert!(anim.drop_one_bounds(now)); + assert_eq!(anim.active_count(now), 1); + assert!( + anim.progress(AnimationKind::Drag, Some(dragged), now) + .is_some(), + "the accepted drag ring survives" + ); + assert!( + anim.interpolated_bounds(Some(dragged), now).is_some(), + "the ring's hosted bounds survive with it" + ); + assert_eq!(anim.interpolated_bounds(Some(bounds_only), now), None); + + // Only a ring-hosted tween remains: no slot can be freed without + // deleting the ring, so the trigger must be refused. + assert!(!anim.drop_one_bounds(now)); + assert_eq!(anim.active_count(now), 1); + assert!( + anim.progress(AnimationKind::Drag, Some(dragged), now) + .is_some() + ); + assert!(anim.interpolated_bounds(Some(dragged), now).is_some()); + } + #[test] fn instant_policy_never_arms_and_reports_inactive() { let now = t0(); diff --git a/crates/bitty-runtime/tests/panel_animations.rs b/crates/bitty-runtime/tests/panel_animations.rs index 26d119b3..75edffe3 100644 --- a/crates/bitty-runtime/tests/panel_animations.rs +++ b/crates/bitty-runtime/tests/panel_animations.rs @@ -1053,6 +1053,80 @@ fn alt_drag_float_keeps_one_animation_entry_under_a_full_budget() { rt.handle_key_event(named_key(NamedKey::Alt, PressState::Released)); } +#[test] +fn consecutive_alt_drag_updates_carry_painted_bounds_forward() { + // CodeRabbit PR #2231 finding 1: re-arming the Drag ring on a second + // motion must start from the currently painted frame, not from the last + // committed target, so continuous Alt+drag never snaps. + let mut rt = runtime_with(AnimationPolicy::default()); + let start = Instant::now(); + rt.set_container(UiRect::new(0, 0, 80, 24)); + let _ = rt.tick_at(start).expect("baseline presents"); + rt.set_layout(two_pane_split()); + let _ = rt.tick_at(start).expect("split presents"); + let settled = start + Duration::from_millis(200); + rt.tick_at(settled); + rt.toggle_floating(Some(ViewId::new(1))) + .expect("detach view 1"); + let detach_settled = settled + Duration::from_millis(200); + rt.tick_at(detach_settled); + let detach_idle = detach_settled + Duration::from_millis(200); + rt.tick_at(detach_idle); + assert!(rt.tick_at(detach_idle).is_none(), "detach settled"); + + let grab = point_in_frame(&rt, ViewId::new(1), 0.5, 0.5); + rt.handle_cursor_moved(grab); + rt.handle_key_event(named_key(NamedKey::Alt, PressState::Pressed)); + rt.handle_mouse_input(press(MouseButton::Left)); + assert!( + rt.alt_drag_active(), + "Alt+press on the detached float grabs" + ); + let focus_settled = Instant::now(); + rt.tick_at(focus_settled); + let focus_idle = focus_settled + Duration::from_millis(200); + rt.tick_at(focus_idle); + assert!(rt.tick_at(focus_idle).is_none(), "focus settled"); + + let t0 = Instant::now(); + let first = CursorPosition { + x: grab.x + 18.0, + y: grab.y + 38.0, + }; + assert!(rt.update_alt_drag_at(first, t0)); + let _ = rt.tick_at(t0).expect("first drag present attaches bounds"); + let first_target = frame_for(&rt.present_frames(), ViewId::new(1)).frame; + + let mid = t0 + Duration::from_millis(40); + rt.tick_at(mid).expect("mid drag frame presents"); + let continuity = frame_for(&rt.paint_frames_at(mid), ViewId::new(1)).frame; + assert_ne!(continuity, first_target, "mid tween is not at its target"); + + // A second motion re-arms the same Drag entry. Its carried start must be + // the painted mid frame, so the present at the same instant is continuous. + let second = CursorPosition { + x: first.x + 18.0, + y: first.y + 38.0, + }; + assert!(rt.update_alt_drag_at(second, mid)); + let second_target = frame_for(&rt.present_frames(), ViewId::new(1)).frame; + assert_ne!(second_target, first_target, "second motion moved further"); + let _ = rt.tick_at(mid).expect("second drag present retargets"); + let retargeted = frame_for(&rt.paint_frames_at(mid), ViewId::new(1)).frame; + assert_eq!( + retargeted, continuity, + "the retarget must not snap back to the previous committed target" + ); + + let later = mid + Duration::from_millis(40); + rt.tick_at(later).expect("later drag frame presents"); + let later_paint = frame_for(&rt.paint_frames_at(later), ViewId::new(1)).frame; + assert_ne!(later_paint, continuity, "the retargeted tween advances"); + assert_ne!(later_paint, second_target, "and has not already settled"); + rt.handle_mouse_input(release(MouseButton::Left)); + rt.handle_key_event(named_key(NamedKey::Alt, PressState::Released)); +} + #[test] fn pinned_overlay_does_not_slide_with_the_workspace() { // CTX-1335 F5: pins are window-global chrome; the incoming workspace