1
0
Fork 0
forked from wry/wry

Fix animation retarget and reflow regressions

This commit is contained in:
atagen 2026-05-24 14:55:24 +10:00
parent dfcb2d0fd6
commit 0f6f9f2602
6 changed files with 261 additions and 33 deletions

View file

@ -67,7 +67,7 @@ These paths should not be used as evidence of multiphase behavior:
- tile-to-float and float-to-tile, which deliberately use linear animation
- command-driven floating move/resize, which may animate but can overlap
- pointer or tablet drag/resize, which should not animate
- spawn-in and spawn-out, which are always linear
- spawn-in and spawn-out, which are single-phase and use the configured curve
- cross-output or cross-scale movement, which should snap
- layer-shell, overlay, override-redirect, and fullscreen map/unmap paths
@ -288,9 +288,9 @@ Hard failure:
## 9. Mixed-Action Phases
This case is easiest to prove with the planner tests because Wry currently has
few user commands that create a same-batch move for one tiled window and an
independent resize for another. The canonical geometry is:
This case is easiest to prove with the planner tests because Wry does not yet
have a confirmed stock command that reliably creates a same-batch move for one
tiled window and an independent resize for another. The canonical geometry is:
```text
start: A = (0,0)-(80,80) B = (200,0)-(280,80)
@ -306,9 +306,19 @@ To exercise the current proof directly, run:
cargo test animation::multiphase::tests::mixed_single_phase_accepts_move_and_scale_when_proven
```
For visual/manual testing, use any command sequence that can produce the same
shape in one layout batch: one window changes only x-position, and a separate,
non-overlapping window changes only height.
For visual/manual testing, the target shape is:
```text
before: [ A ] [ B ]
after: [ A ] [ B ]
[ ]
```
`A` must move horizontally without resizing. `B` must resize vertically without
moving. The two motion bounds must remain separate for the whole animation. If a
normal command sequence cannot produce that in one layout batch, treat the unit
test as the authority and record the visual test as not applicable rather than a
failure.
Expected:
@ -344,7 +354,8 @@ Use a long duration, then issue commands mid-animation:
- swap, then reverse before completion
- resize, then resize in the other direction before completion
- move a window out of a stack, then back before completion
- build `[A | [B | C | D]]`, move `C` left to form `[A | C | [B | D]]`,
then move `C` back into the stack before completion
- start a multiphase group, then change only one window's destination if a
command sequence allows it

View file

@ -12,6 +12,7 @@ use {
ahash::AHashMap,
std::{
cell::{Cell, RefCell},
collections::VecDeque,
rc::{Rc, Weak},
},
};
@ -368,6 +369,14 @@ impl AnimationState {
.into_iter()
.map(|(from, to)| PhasedSegment { from, to })
.collect();
let mut route_edges = route_edges_from_segments(&segments);
if let Some(anim) = self.phased.borrow().get(&node_id)
&& !anim.done(now_nsec)
{
for &(from, to) in &anim.route_edges {
push_unique_route_edge(&mut route_edges, from, to);
}
}
self.windows.borrow_mut().remove(&node_id);
self.phased.borrow_mut().insert(
node_id,
@ -378,6 +387,7 @@ impl AnimationState {
curve,
last_damage: from,
final_rect,
route_edges,
retained,
},
);
@ -601,6 +611,7 @@ struct PhasedWindowAnimation {
curve: AnimationCurve,
last_damage: Rect,
final_rect: Rect,
route_edges: Vec<(Rect, Rect)>,
retained: Option<Rc<RetainedToplevel>>,
}
@ -641,36 +652,110 @@ impl PhasedWindowAnimation {
(phase < self.segments.len()).then_some(phase)
}
fn path_points(&self) -> Option<Vec<Rect>> {
let first = self.segments.first()?;
let mut points = vec![first.from];
points.extend(self.segments.iter().map(|segment| segment.to));
Some(points)
}
fn route_to(&self, target: Rect, now_nsec: u64) -> Option<Vec<(Rect, Rect)>> {
let phase = self.phase_at(now_nsec)?;
let points = self.path_points()?;
let target_idx = points.iter().position(|point| *point == target)?;
let current = self.rect_at(now_nsec);
if current == target {
return Some(vec![]);
}
let segment = self.segments.get(phase)?;
route_through_edges(current, target, segment.from, segment.to, &self.route_edges)
}
}
let mut route = vec![];
if target_idx <= phase {
push_non_empty_segment(&mut route, current, points[phase]);
for idx in (target_idx..phase).rev() {
push_non_empty_segment(&mut route, points[idx + 1], points[idx]);
}
} else {
push_non_empty_segment(&mut route, current, points[phase + 1]);
for idx in (phase + 1)..target_idx {
push_non_empty_segment(&mut route, points[idx], points[idx + 1]);
fn route_edges_from_segments(segments: &[PhasedSegment]) -> Vec<(Rect, Rect)> {
let mut edges = vec![];
for segment in segments {
push_unique_route_edge(&mut edges, segment.from, segment.to);
}
edges
}
fn push_unique_route_edge(edges: &mut Vec<(Rect, Rect)>, from: Rect, to: Rect) {
if from == to {
return;
}
if edges
.iter()
.any(|&(a, b)| (a == from && b == to) || (a == to && b == from))
{
return;
}
edges.push((from, to));
}
fn route_through_edges(
current: Rect,
target: Rect,
current_from: Rect,
current_to: Rect,
known_edges: &[(Rect, Rect)],
) -> Option<Vec<(Rect, Rect)>> {
let mut edges = known_edges.to_vec();
push_unique_route_edge(&mut edges, current, current_from);
push_unique_route_edge(&mut edges, current, current_to);
rect_graph_route(current, target, &edges)
}
fn rect_graph_route(
start: Rect,
target: Rect,
edges: &[(Rect, Rect)],
) -> Option<Vec<(Rect, Rect)>> {
let mut nodes = vec![];
let mut adjacency: Vec<Vec<usize>> = vec![];
let start_idx = rect_graph_node(&mut nodes, &mut adjacency, start);
let target_idx = rect_graph_node(&mut nodes, &mut adjacency, target);
for &(from, to) in edges {
let from_idx = rect_graph_node(&mut nodes, &mut adjacency, from);
let to_idx = rect_graph_node(&mut nodes, &mut adjacency, to);
if !adjacency[from_idx].contains(&to_idx) {
adjacency[from_idx].push(to_idx);
}
if !adjacency[to_idx].contains(&from_idx) {
adjacency[to_idx].push(from_idx);
}
}
let mut previous = vec![None; nodes.len()];
let mut queue = VecDeque::from([start_idx]);
previous[start_idx] = Some(start_idx);
while let Some(idx) = queue.pop_front() {
if idx == target_idx {
break;
}
for &next in &adjacency[idx] {
if previous[next].is_none() {
previous[next] = Some(idx);
queue.push_back(next);
}
}
Some(route)
}
previous[target_idx]?;
let mut reversed_nodes = vec![target_idx];
let mut idx = target_idx;
while idx != start_idx {
idx = previous[idx]?;
reversed_nodes.push(idx);
}
reversed_nodes.reverse();
let mut route = vec![];
for pair in reversed_nodes.windows(2) {
push_non_empty_segment(&mut route, nodes[pair[0]], nodes[pair[1]]);
}
Some(route)
}
fn rect_graph_node(nodes: &mut Vec<Rect>, adjacency: &mut Vec<Vec<usize>>, rect: Rect) -> usize {
if let Some(idx) = nodes.iter().position(|&node| node == rect) {
return idx;
}
let idx = nodes.len();
nodes.push(rect);
adjacency.push(vec![]);
idx
}
fn push_non_empty_segment(route: &mut Vec<(Rect, Rect)>, from: Rect, to: Rect) {
@ -977,6 +1062,41 @@ mod tests {
);
}
#[test]
fn phased_route_remembers_original_path_after_retarget() {
let state = AnimationState::default();
let id = NodeId(1);
let a = Rect::new_sized_saturating(0, 0, 100, 100);
let b = Rect::new_sized_saturating(0, 0, 100, 50);
let c = Rect::new_sized_saturating(100, 0, 100, 50);
assert!(state.set_phased_target(
id,
vec![(a, b), (b, c)],
None,
0,
100,
AnimationCurve::Linear
));
let current = lerp_rect(b, c, 0.5);
let reverse = state.phased_route_to(id, a, 150_000_000).unwrap();
assert_eq!(reverse, vec![(current, b), (b, a)]);
assert!(state.set_phased_target(
id,
reverse,
None,
150_000_000,
100,
AnimationCurve::Linear
));
let current = lerp_rect(current, b, 0.5);
assert_eq!(
state.phased_route_to(id, c, 200_000_000).unwrap(),
vec![(current, b), (b, c)]
);
}
#[test]
fn linear_retarget_interrupts_phased_animation_from_current_rect() {
let state = AnimationState::default();

View file

@ -1,6 +1,6 @@
use {crate::rect::Rect, crate::tree::NodeId};
const MIN_SHRINK_DENOMINATOR: i32 = 4;
const MIN_SHRINK_DENOMINATOR: i32 = 8;
#[derive(Clone, Debug)]
pub struct MultiphaseRequest {
@ -2198,6 +2198,23 @@ mod tests {
assert!(validate_plan_continuous(&req, &planned.plan));
}
#[test]
fn single_window_one_axis_group_is_still_multiphase_plannable() {
let req = request(vec![window(1, rect(0, 0, 100, 100), rect(40, 0, 140, 100))]);
let planned = plan_no_overlap_explained(&req).unwrap();
assert_eq!(planned.explanation.strategy, PlanStrategy::SingleAction);
assert_eq!(
planned.plan.phases[0].action,
MultiphasePhaseAction::Uniform(PhaseAction {
kind: PhaseKind::Move,
axis: PhaseAxis::Horizontal,
})
);
assert_eq!(planned.explanation.phases[0].nodes, vec![id(1)]);
assert!(validate_plan_continuous(&req, &planned.plan));
}
#[test]
fn mixed_single_phase_still_rejects_diagonal_per_window_motion() {
let req = request(vec![
@ -2424,6 +2441,61 @@ mod tests {
assert!(validate_plan_continuous(&req, plan));
}
#[test]
fn three_child_stack_extraction_plans_without_linear_fallback() {
let old = split(
10,
PhaseAxis::Horizontal,
&[1, 1],
vec![
leaf(1),
split(
11,
PhaseAxis::Vertical,
&[1, 1, 1],
vec![leaf(2), leaf(3), leaf(4)],
),
],
);
let new = split(
10,
PhaseAxis::Horizontal,
&[1, 1, 1],
vec![
leaf(1),
leaf(3),
split(11, PhaseAxis::Vertical, &[1, 1], vec![leaf(2), leaf(4)]),
],
);
let req = generated_request(&old, &new, rect(0, 0, 600, 300));
let planned = plan_no_overlap_explained(&req).unwrap();
assert_eq!(
planned.explanation.strategy,
PlanStrategy::SpaceThenOrthogonalGrowth {
axis: PhaseAxis::Horizontal,
}
);
assert_eq!(
actions(&planned.plan),
vec![
PhaseAction {
kind: PhaseKind::Scale,
axis: PhaseAxis::Horizontal,
},
PhaseAction {
kind: PhaseKind::Move,
axis: PhaseAxis::Horizontal,
},
PhaseAction {
kind: PhaseKind::Scale,
axis: PhaseAxis::Vertical,
},
]
);
assert!(validate_plan_continuous(&req, &planned.plan));
}
#[test]
fn validated_phase_paths_accept_interrupted_reverse_route() {
let a_current = rect(50, 0, 150, 50);
@ -2793,7 +2865,7 @@ mod tests {
MultiphasePlanFailure::ShrinkBound {
axis: PhaseAxis::Horizontal,
available: 10,
required: 100,
required: 50,
}
));
}

View file

@ -553,6 +553,7 @@ impl Renderer<'_> {
return;
}
let bounds = self.base.scale_rect(body);
self.render_window_body_background(&bounds);
self.stretch = if frame.source_body_size != body.size() {
Some(self.base.scale_point(body.width(), body.height()))
} else {
@ -573,6 +574,21 @@ impl Renderer<'_> {
self.corner_radius = None;
}
fn render_window_body_background(&mut self, bounds: &Rect) {
if bounds.is_empty() {
return;
}
let color = self.state.theme.colors.background.get();
self.base.sync();
self.base.fill_scaled_boxes(
slice::from_ref(bounds),
&color,
None,
&self.state.color_manager.srgb_gamma22().linear,
RenderIntent::Perceptual,
);
}
fn render_retained_surface_scaled(
&mut self,
retained: &RetainedSurface,
@ -744,6 +760,9 @@ impl Renderer<'_> {
}
let body = visual_mb.move_(x, y);
let body = self.base.scale_rect(body);
if !child.node.node_is_container() {
self.render_window_body_background(&body);
}
let content = container
.mono_content
.get()
@ -818,6 +837,9 @@ impl Renderer<'_> {
}
let body = body.move_(x, y);
let body = self.base.scale_rect(body);
if !child.node.node_is_container() {
self.render_window_body_background(&body);
}
self.render_child_or_snapshot(
&child.node,
x + content.x1(),
@ -1087,6 +1109,7 @@ impl Renderer<'_> {
let inner_cr = self.scale_corner_radius(cr.expanded_by(-(bw as f32)));
self.corner_radius = Some(inner_cr);
}
self.render_window_body_background(&scissor_body);
self.render_child_or_snapshot(&child, body.x1(), body.y1(), Some(&scissor_body));
self.stretch = None;
self.corner_radius = None;

View file

@ -1663,9 +1663,6 @@ impl State {
group: &[usize],
now_nsec: u64,
) -> bool {
if group.len() < 2 {
return false;
}
let request_windows: Vec<_> = group.iter().map(|&idx| windows[idx]).collect();
let Some(first) = request_windows.first() else {
return false;

View file

@ -2303,6 +2303,11 @@ impl ContainingNode for ContainerNode {
}
// log::info!("cnode_remove_child2");
self.rebuild_tab_bar();
if self.state.animations.enabled.get()
&& !self.state.suppress_animations_for_next_layout.get()
{
self.animate_next_layout.set(true);
}
self.schedule_layout();
self.cancel_seat_ops();
self.child_removed.trigger();