From a2f5a7ffbad32e473f60cd5469677bc9ed08e129 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Sun, 4 Oct 2026 08:16:03 +0100 Subject: [PATCH] Fix collective resize waits and fullscreen ownership --- compositor/src/state/capture_privacy_tests.rs | 50 ++++++ .../src/state/presentation_dependencies.rs | 165 ++++++++++++++++-- 2 files changed, 199 insertions(+), 16 deletions(-) diff --git a/compositor/src/state/capture_privacy_tests.rs b/compositor/src/state/capture_privacy_tests.rs index 9965500..829ceb7 100644 --- a/compositor/src/state/capture_privacy_tests.rs +++ b/compositor/src/state/capture_privacy_tests.rs @@ -993,4 +993,54 @@ fn resize_dependencies_protocol_and_pixels() { "stacked replacement lost canonical presented source" ); } + // Fullscreen geometry is output-owned. Keep its own configure barrier, + // but release it independently of a slow sibling in the same column. + state.set_window_fullscreen(d_id, true); + state.display_handle.flush_clients().unwrap(); + ack_configure(&mut d_wire, 3); + dispatch(&mut events, &mut state); + assert!( + state.windows.transaction(&d_id).is_some(), + "fullscreen ack alone released its barrier" + ); + let before = state.windows.geometry(&d_id).unwrap().visual.current; + state.advance_animations_at(Duration::from_millis(16), Duration::from_millis(10016)); + assert_eq!(state.windows.geometry(&d_id).unwrap().visual.current, before); + + request(&mut d_wire, 2, 1, &[6, 0, 0], None); + request(&mut d_wire, 2, 6, &[], None); + dispatch(&mut events, &mut state); + assert!( + state.windows.transaction(&d_id).is_none(), + "own fullscreen configure did not release after commit" + ); + assert!( + state.windows.transaction(&b_id).is_some(), + "other tile must still be pending" + ); + assert!(!state.presentation_dependencies.reflow_blocked(d_id, &state.windows)); + assert!(state.windows.geometry(&d_id).unwrap().is_zooming()); + // Consume the release tick without charging the preceding client wait. + state.advance_animations_at(Duration::ZERO, Duration::from_millis(10016)); + state.advance_animations_at(Duration::from_millis(16), Duration::from_millis(10032)); + assert_ne!( + state.windows.geometry(&d_id).unwrap().visual.current, + before, + "fullscreen zoom stayed held by an unrelated column resize after its own commit" + ); + + // Returning to the column needs its real reflow wait again, even after + // the returning client's own configure has committed. + state.set_window_fullscreen(d_id, false); + state.display_handle.flush_clients().unwrap(); + ack_configure(&mut d_wire, 3); + request(&mut d_wire, 2, 1, &[6, 0, 0], None); + request(&mut d_wire, 2, 6, &[], None); + dispatch(&mut events, &mut state); + assert!(state.windows.transaction(&d_id).is_none()); + assert!(state.windows.transaction(&b_id).is_some()); + assert!(state.presentation_dependencies.reflow_blocked(d_id, &state.windows)); + let returning = state.windows.geometry(&d_id).unwrap().visual.current; + state.advance_animations_at(Duration::from_millis(16), Duration::from_millis(10048)); + assert_eq!(state.windows.geometry(&d_id).unwrap().visual.current, returning); } diff --git a/compositor/src/state/presentation_dependencies.rs b/compositor/src/state/presentation_dependencies.rs index de5039d..334f471 100644 --- a/compositor/src/state/presentation_dependencies.rs +++ b/compositor/src/state/presentation_dependencies.rs @@ -206,28 +206,53 @@ fn viewport_dependencies( owners.extend(inputs.iter().map(|input| input.commit)); owners.sort_by_key(|owner| owner.window.0); owners.dedup(); - let evaluate = |only: Option, exclude: Option| { + let evaluate = |only: Option, exclude: Option, retained: Option<&HashSet>| { let mut start = None; for step in &previous.steps { let widths = step .inputs .iter() - .filter(|input| only.is_none_or(|only| input.commit == only) && exclude != Some(input.commit)) + .filter(|input| { + only.is_none_or(|only| input.commit == only) + && exclude != Some(input.commit) + && retained.is_none_or(|owners| owners.contains(&input.commit.window)) + }) .map(|input| (input.commit.window, input.width.0, input.width.1)) .collect::>(); start = Some(step.target.with_held_widths(&widths, start)); } start.unwrap_or(layout.viewport_x()) }; - let held = evaluate(None, None); + let held = evaluate(None, None, None); previous.waits = owners - .into_iter() + .iter() + .copied() .filter(|owner| { previous.overflow - || (evaluate(Some(*owner), None) - layout.viewport_x()).abs() > 0.001 - || (held - evaluate(None, Some(*owner))).abs() > 0.001 + || (evaluate(Some(*owner), None, None) - layout.viewport_x()).abs() > 0.001 + || (held - evaluate(None, Some(*owner), None)).abs() > 0.001 }) .collect(); + + // Page packing is nonlinear: all held widths can change the page even + // when neither a single input nor removing one input exposes the change. + // Retain a collective wait in that case, removing inputs that are not + // needed to keep the held target different from the requested target. + if previous.waits.is_empty() && (held - layout.viewport_x()).abs() > 0.001 { + let mut collective = owners.iter().map(|owner| owner.window).collect::>(); + for owner in &owners { + collective.remove(&owner.window); + if (evaluate(None, None, Some(&collective)) - layout.viewport_x()).abs() <= 0.001 { + collective.insert(owner.window); + } + } + + previous.waits = owners + .into_iter() + .filter(|owner| collective.contains(&owner.window)) + .collect(); + } + // Keep a bounded recipe while any of its inputs is pending: a partial // commit can expose a page-packing dependency previously masked by others. if previous.steps.iter().all(|step| step.inputs.is_empty()) { @@ -272,17 +297,25 @@ impl Ferese { window: *slow, serial: transaction.serial(), }; + // Fullscreen bounds come from the output, not a column. Its + // own configure still gates presentation; tiled reflow waits + // apply again when the window returns to normal geometry. let linked = id == *slow - || match &workspace.layout { - WorkspaceLayout::Scrolling(layout) => own_column.is_some_and(|own| { - layout - .columns() - .iter() - .position(|column| column.windows.contains(slow)) - .is_some_and(|source| source == own || (source < own && transaction.width_changes())) - }), - WorkspaceLayout::Tree(_) => workspace.layout.contains(id) && workspace.layout.contains(*slow), - }; + || (workspace.fullscreen != Some(id) + && match &workspace.layout { + WorkspaceLayout::Scrolling(layout) => own_column.is_some_and(|own| { + layout + .columns() + .iter() + .position(|column| column.windows.contains(slow)) + .is_some_and(|source| { + source == own || (source < own && transaction.width_changes()) + }) + }), + WorkspaceLayout::Tree(_) => { + workspace.layout.contains(id) && workspace.layout.contains(*slow) + } + }); if linked { waits.push(commit); } @@ -476,4 +509,104 @@ mod tests { assert!(!dependencies.needs_tick()); assert!(dependencies.reflow.is_empty() && dependencies.viewport.is_empty()); } + + #[test] + fn collective_page_waits_survive_redundant_pending_widths() { + use ferese_layout::{ColumnWidth, ScrollingLayout}; + let bounds = Rect::new(0.0, 0.0, 200.0, 100.0); + let gaps = GapConfig { + inner: 0.0, + outer: 0.0, + smart: false, + }; + let mut layout = ScrollingLayout::with_default_width(ColumnWidth::Fixed(40.0)); + layout.set_focus_strategy(ViewportFocusStrategy::Paged); + let ids = [ + WindowId(1), + WindowId(2), + WindowId(3), + WindowId(4), + WindowId(5), + WindowId(6), + ]; + for (i, id) in ids.iter().enumerate() { + layout.insert(*id, i.checked_sub(1).map(|j| ids[j])).unwrap(); + } + + for (id, width) in ids.into_iter().zip([40.0, 40.0, 80.0, 80.0, 40.0, 40.0]) { + layout.set_column_width(id, ColumnWidth::Fixed(width)).unwrap(); + } + + layout.focus(ids[4]).unwrap(); + layout + .geometry_with_constraints(bounds, gaps, &HashMap::new(), Some(ids[4])) + .unwrap(); + assert_eq!(layout.viewport_x(), 160.0); + let widths = [ + (ids[0], 120.0, 40.0), + (ids[1], 120.0, 40.0), + (ids[4], 80.0, 40.0), + (ids[5], 160.0, 40.0), + ]; + let pending = widths + .into_iter() + .enumerate() + .map(|(i, (id, source, target))| { + ( + id, + ResizeTransaction::new((i as u32 + 10).into(), Duration::ZERO) + .with_column_width(Some(source), Some(target)), + ) + }) + .collect::>(); + assert_eq!(layout.viewport_target().unwrap().with_held_widths(&widths, None), 320.0); + let graph = viewport_dependencies(&layout, &pending, Default::default()); + assert!( + !graph.waits.is_empty(), + "the collectively different page needs a commit wait" + ); + assert!( + !graph.waits.iter().any(|wait| wait.window == ids[5]), + "a column after focus cannot own this page wait" + ); + let reversed = pending.iter().copied().rev().collect::>(); + assert_eq!( + viewport_dependencies(&layout, &reversed, Default::default()).waits, + graph.waits + ); + + // Exercise every partial-commit order, including a redundant owner + // committing before an owner in the retained collective group. + for mask in 0..1 << pending.len() { + let remaining = pending + .iter() + .enumerate() + .filter(|(index, _)| mask & (1 << index) != 0) + .map(|(_, input)| *input) + .collect::>(); + let held_widths = remaining + .iter() + .map(|(id, transaction)| { + let (source, target) = transaction.column_width().unwrap(); + (*id, source, target) + }) + .collect::>(); + let held = layout.viewport_target().unwrap().with_held_widths(&held_widths, None); + let partial = viewport_dependencies(&layout, &remaining, graph.clone()); + if (held - layout.viewport_x()).abs() > 0.001 { + assert!( + !partial.waits.is_empty(), + "collective wait lost after partial commits: {mask}" + ); + } + + assert!( + !partial.waits.iter().any(|wait| wait.window == ids[5]), + "unrelated column blocked mask {mask}" + ); + if remaining.is_empty() { + assert!(partial.steps.is_empty() && partial.waits.is_empty()); + } + } + } }