diff --git a/docs/window-animations-plan.md b/docs/window-animations-plan.md index fd99ac39..61c91b84 100644 --- a/docs/window-animations-plan.md +++ b/docs/window-animations-plan.md @@ -233,6 +233,10 @@ Current pure planner status: parent, depth, sibling index, split axis, mono state, and transition kind. The current planner records this information but does not yet use it to order nested-container phases. +- Multiphase planning has a diagnostic entry point used by live fallback logs. + It distinguishes request validation errors, missing patterns, shrink-bound + rejections, invalid phase steps, and exact validation failures such as stale + starts or phase overlap. Tests: diff --git a/src/animation/multiphase.rs b/src/animation/multiphase.rs index 75c63493..641e8826 100644 --- a/src/animation/multiphase.rs +++ b/src/animation/multiphase.rs @@ -159,8 +159,59 @@ pub enum MultiphaseError { NoPlan, } +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub struct MultiphasePlanDiagnostic { + pub forward: MultiphasePlanFailure, + pub reverse: Option, +} + +impl MultiphasePlanDiagnostic { + fn legacy_error(self) -> MultiphaseError { + match self.forward { + MultiphasePlanFailure::Request(error) => error, + _ => MultiphaseError::NoPlan, + } + } +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum MultiphasePlanFailure { + Request(MultiphaseError), + NoPattern, + ShrinkBound { + axis: PhaseAxis, + available: i32, + required: i32, + }, + InvalidPhaseStep { + action: PhaseAction, + node_id: NodeId, + }, + Validation(MultiphaseValidationError), +} + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum MultiphaseValidationError { + DuplicatePhaseStep { phase: usize, node_id: NodeId }, + UnknownPhaseStep { phase: usize, node_id: NodeId }, + StaleStepStart { phase: usize, node_id: NodeId }, + PhaseOverlap { phase: usize, a: NodeId, b: NodeId }, + FinalMismatch { node_id: NodeId }, +} + pub fn plan_no_overlap(request: &MultiphaseRequest) -> Result { - validate_request(request)?; + plan_no_overlap_with_diagnostics(request).map_err(|diagnostic| diagnostic.legacy_error()) +} + +pub fn plan_no_overlap_with_diagnostics( + request: &MultiphaseRequest, +) -> Result { + if let Err(error) = validate_request(request) { + return Err(MultiphasePlanDiagnostic { + forward: MultiphasePlanFailure::Request(error), + reverse: None, + }); + } if request .windows .iter() @@ -168,14 +219,18 @@ pub fn plan_no_overlap(request: &MultiphaseRequest) -> Result return Ok(plan), + Err(error) => error, + }; let reversed = reverse_request(request); - if let Some(plan) = plan_forward(&reversed) { - return Ok(reverse_plan(plan)); + match plan_forward(&reversed) { + Ok(plan) => Ok(reverse_plan(plan)), + Err(reverse) => Err(MultiphasePlanDiagnostic { + forward, + reverse: Some(reverse), + }), } - Err(MultiphaseError::NoPlan) } pub(crate) fn partition_motion_groups(windows: &[MultiphaseWindow]) -> Vec> { @@ -228,33 +283,48 @@ fn validate_request(request: &MultiphaseRequest) -> Result<(), MultiphaseError> Ok(()) } -fn plan_forward(request: &MultiphaseRequest) -> Option { +fn plan_forward(request: &MultiphaseRequest) -> Result { + let mut rejection = None; for axis in [PhaseAxis::Horizontal, PhaseAxis::Vertical] { - if let Some(plan) = plan_axis_crossing_lanes(request, axis) { - return Some(plan); + match plan_axis_crossing_lanes(request, axis) { + Ok(plan) => return Ok(plan), + Err(MultiphasePlanFailure::NoPattern) => {} + Err(error) => { + rejection.get_or_insert(error); + } } } - plan_space_then_orthogonal_growth(request, PhaseAxis::Horizontal) - .or_else(|| plan_space_then_orthogonal_growth(request, PhaseAxis::Vertical)) + for axis in [PhaseAxis::Horizontal, PhaseAxis::Vertical] { + match plan_space_then_orthogonal_growth(request, axis) { + Ok(plan) => return Ok(plan), + Err(MultiphasePlanFailure::NoPattern) => {} + Err(error) => { + rejection.get_or_insert(error); + } + } + } + Err(rejection.unwrap_or(MultiphasePlanFailure::NoPattern)) } fn plan_axis_crossing_lanes( request: &MultiphaseRequest, axis: PhaseAxis, -) -> Option { +) -> Result { if request.windows.len() != 2 { - return None; + return Err(MultiphasePlanFailure::NoPattern); } let orth_min = request .windows .iter() .map(|window| orth_start(window.from, axis)) - .min()?; + .min() + .ok_or(MultiphasePlanFailure::NoPattern)?; let orth_max = request .windows .iter() .map(|window| orth_end(window.from, axis)) - .max()?; + .max() + .ok_or(MultiphasePlanFailure::NoPattern)?; if request.windows.iter().any(|window| { main_size(window.from, axis) != main_size(window.to, axis) || orth_start(window.from, axis) != orth_min @@ -263,11 +333,16 @@ fn plan_axis_crossing_lanes( || orth_end(window.to, axis) != orth_max || main_start(window.from, axis) == main_start(window.to, axis) }) { - return None; + return Err(MultiphasePlanFailure::NoPattern); } let lane_size = (orth_max - orth_min) / request.windows.len() as i32; - if lane_size < sane_min_size(orth_max - orth_min) { - return None; + let required = sane_min_size(orth_max - orth_min); + if lane_size < required { + return Err(MultiphasePlanFailure::ShrinkBound { + axis: axis.other(), + available: lane_size, + required, + }); } let mut windows = request.windows.clone(); @@ -275,7 +350,7 @@ fn plan_axis_crossing_lanes( if windows.windows(2).any(|pair| { lane_index_for_direction(pair[0], axis) == lane_index_for_direction(pair[1], axis) }) { - return None; + return Err(MultiphasePlanFailure::NoPattern); } let mut phase1 = vec![]; let mut phase2 = vec![]; @@ -320,9 +395,9 @@ fn lane_index_for_direction(window: MultiphaseWindow, axis: PhaseAxis) -> Option fn plan_space_then_orthogonal_growth( request: &MultiphaseRequest, axis: PhaseAxis, -) -> Option { +) -> Result { if request.windows.len() < 2 { - return None; + return Err(MultiphasePlanFailure::NoPattern); } let orth_axis = axis.other(); let min_width = sane_min_size(request.bounds.width()); @@ -331,8 +406,19 @@ fn plan_space_then_orthogonal_growth( let mut phase2 = vec![]; let mut phase3 = vec![]; for window in &request.windows { - if window.to.width() < min_width || window.to.height() < min_height { - return None; + if window.to.width() < min_width { + return Err(MultiphasePlanFailure::ShrinkBound { + axis: PhaseAxis::Horizontal, + available: window.to.width(), + required: min_width, + }); + } + if window.to.height() < min_height { + return Err(MultiphasePlanFailure::ShrinkBound { + axis: PhaseAxis::Vertical, + available: window.to.height(), + required: min_height, + }); } let main_changes = main_start(window.from, axis) != main_start(window.to, axis) || main_end(window.from, axis) != main_end(window.to, axis); @@ -365,7 +451,7 @@ fn plan_space_then_orthogonal_growth( } } if phase1.is_empty() || phase2.is_empty() || phase3.is_empty() { - return None; + return Err(MultiphasePlanFailure::NoPattern); } build_validated_plan( request, @@ -380,7 +466,7 @@ fn plan_space_then_orthogonal_growth( fn build_validated_plan( request: &MultiphaseRequest, phases: [(PhaseKind, PhaseAxis, Vec); N], -) -> Option { +) -> Result { let phases: Vec<_> = phases .into_iter() .filter_map(|(kind, axis, steps)| { @@ -390,41 +476,58 @@ fn build_validated_plan( }) }) .collect(); - if phases.iter().any(|phase| { - phase - .steps - .iter() - .any(|step| classify_step(*step) != Some(phase.action)) - }) { - return None; + for phase in &phases { + for step in &phase.steps { + if classify_step(*step) != Some(phase.action) { + return Err(MultiphasePlanFailure::InvalidPhaseStep { + action: phase.action, + node_id: step.node_id, + }); + } + } } let plan = MultiphasePlan { phases }; - validate_plan_continuous(request, &plan).then_some(plan) + validate_plan_continuous_diagnostic(request, &plan) + .map(|_| plan) + .map_err(MultiphasePlanFailure::Validation) } fn validate_plan_continuous(request: &MultiphaseRequest, plan: &MultiphasePlan) -> bool { + validate_plan_continuous_diagnostic(request, plan).is_ok() +} + +fn validate_plan_continuous_diagnostic( + request: &MultiphaseRequest, + plan: &MultiphasePlan, +) -> Result<(), MultiphaseValidationError> { let mut current: Vec<_> = request .windows .iter() .map(|window| (window.node_id, window.from)) .collect(); - if overlaps(current.iter().map(|(_, rect)| *rect)) { - return false; - } - for phase in &plan.phases { + for (phase_idx, phase) in plan.phases.iter().enumerate() { for (idx, step) in phase.steps.iter().enumerate() { if phase.steps[..idx] .iter() .any(|prev| prev.node_id == step.node_id) { - return false; + return Err(MultiphaseValidationError::DuplicatePhaseStep { + phase: phase_idx, + node_id: step.node_id, + }); } let Some((_, rect)) = current.iter().find(|(node_id, _)| *node_id == step.node_id) else { - return false; + return Err(MultiphaseValidationError::UnknownPhaseStep { + phase: phase_idx, + node_id: step.node_id, + }); }; if *rect != step.from { - return false; + return Err(MultiphaseValidationError::StaleStepStart { + phase: phase_idx, + node_id: step.node_id, + }); } } let motions: Vec<_> = current @@ -440,11 +543,16 @@ fn validate_plan_continuous(request: &MultiphaseRequest, plan: &MultiphasePlan) }) .collect(); for (idx, motion) in motions.iter().enumerate() { - if motions[idx + 1..] + if let Some((other_idx, _)) = motions[idx + 1..] .iter() - .any(|other| motions_overlap_during_phase(*motion, *other)) + .enumerate() + .find(|(_, other)| motions_overlap_during_phase(*motion, **other)) { - return false; + return Err(MultiphaseValidationError::PhaseOverlap { + phase: phase_idx, + a: current[idx].0, + b: current[idx + 1 + other_idx].0, + }); } } for step in &phase.steps { @@ -454,16 +562,19 @@ fn validate_plan_continuous(request: &MultiphaseRequest, plan: &MultiphasePlan) .unwrap(); *rect = step.to; } - if overlaps(current.iter().map(|(_, rect)| *rect)) { - return false; - } } - request.windows.iter().all(|window| { - current + for window in &request.windows { + if !current .iter() .find(|(node_id, _)| *node_id == window.node_id) .is_some_and(|(_, rect)| *rect == window.to) - }) + { + return Err(MultiphaseValidationError::FinalMismatch { + node_id: window.node_id, + }); + } + } + Ok(()) } #[derive(Copy, Clone)] @@ -1044,6 +1155,43 @@ mod tests { hierarchy: Default::default(), }]); assert_eq!(plan_no_overlap(&req), Err(MultiphaseError::NoPlan)); + assert_eq!( + plan_no_overlap_with_diagnostics(&req).unwrap_err(), + MultiphasePlanDiagnostic { + forward: MultiphasePlanFailure::NoPattern, + reverse: Some(MultiphasePlanFailure::NoPattern), + } + ); + } + + #[test] + fn diagnostics_report_shrink_bound_rejections() { + let req = MultiphaseRequest { + bounds: rect(0, 0, 400, 100), + windows: vec![ + MultiphaseWindow { + node_id: id(1), + from: rect(0, 0, 200, 100), + to: rect(0, 0, 10, 100), + hierarchy: Default::default(), + }, + MultiphaseWindow { + node_id: id(2), + from: rect(200, 0, 400, 100), + to: rect(10, 0, 400, 100), + hierarchy: Default::default(), + }, + ], + }; + + assert!(matches!( + plan_no_overlap_with_diagnostics(&req).unwrap_err().forward, + MultiphasePlanFailure::ShrinkBound { + axis: PhaseAxis::Horizontal, + available: 10, + required: 100, + } + )); } #[test] @@ -1115,7 +1263,14 @@ mod tests { }], }; - assert!(!validate_plan_continuous(&req, &plan)); + assert_eq!( + validate_plan_continuous_diagnostic(&req, &plan), + Err(MultiphaseValidationError::PhaseOverlap { + phase: 0, + a: id(1), + b: id(2), + }) + ); } #[test] @@ -1173,7 +1328,13 @@ mod tests { }], }; - assert!(!validate_plan_continuous(&req, &plan)); + assert_eq!( + validate_plan_continuous_diagnostic(&req, &plan), + Err(MultiphaseValidationError::StaleStepStart { + phase: 0, + node_id: id(1), + }) + ); } #[test] diff --git a/src/state.rs b/src/state.rs index 0d120721..4fc15231 100644 --- a/src/state.rs +++ b/src/state.rs @@ -7,7 +7,7 @@ use { expand_damage_rect, multiphase::{ MultiphaseRequest, MultiphaseWindow, MultiphaseWindowHierarchy, - partition_motion_groups, plan_no_overlap, + partition_motion_groups, plan_no_overlap_with_diagnostics, }, spawn_in_start_rect, }, @@ -1661,11 +1661,20 @@ impl State { for window in &request_windows[1..] { bounds = bounds.union(window.from).union(window.to); } - let Ok(plan) = plan_no_overlap(&MultiphaseRequest { + let request = MultiphaseRequest { bounds, windows: request_windows, - }) else { - return false; + }; + let plan = match plan_no_overlap_with_diagnostics(&request) { + Ok(plan) => plan, + Err(diagnostic) => { + log::debug!( + "falling back to linear layout animation for group {:?}: {:?}", + group, + diagnostic + ); + return false; + } }; if plan.phases.is_empty() { return false;