From d6bdaa38a75ce75b29e95d79a5a4607f3f2b64b2 Mon Sep 17 00:00:00 2001 From: Quentin Eude Date: Tue, 5 May 2026 22:31:18 +0200 Subject: [PATCH] fix(layout): make fullscreen only apply to focused window Previously, toggling fullscreen on a window and then cycling focus would leave the unfocused fullscreen window covering the screen, causing it to stay in the background and obscure other windows. Fullscreen state now persists on nodes, but layout calculation only applies screen bounds when the node is in the focused path. This affects Traditional, BSP, and Scrolling layout systems. --- src/layout_engine/systems/bsp.rs | 111 ++++++++++++++----- src/layout_engine/systems/master_stack.rs | 1 + src/layout_engine/systems/scrolling.rs | 72 +++++++++++- src/layout_engine/systems/traditional.rs | 129 +++++++++++++++++++++- 4 files changed, 279 insertions(+), 34 deletions(-) diff --git a/src/layout_engine/systems/bsp.rs b/src/layout_engine/systems/bsp.rs index 7144653c..71fe9c0b 100644 --- a/src/layout_engine/systems/bsp.rs +++ b/src/layout_engine/systems/bsp.rs @@ -282,20 +282,6 @@ impl BspLayoutSystem { } } - fn has_fullscreen_in_subtree(&self, node: NodeId) -> bool { - match self.kind.get(node) { - Some(NodeKind::Leaf { - fullscreen, - fullscreen_within_gaps, - .. - }) => *fullscreen || *fullscreen_within_gaps, - Some(NodeKind::Split { .. }) => { - node.children(&self.tree.map).any(|child| self.has_fullscreen_in_subtree(child)) - } - None => false, - } - } - fn find_layout_root(&self, mut node: NodeId) -> NodeId { while let Some(p) = node.parent(&self.tree.map) { node = p; @@ -453,6 +439,7 @@ impl BspLayoutSystem { screen: CGRect, constraints: &HashMap, gaps: &crate::common::config::GapSettings, + focused_node: NodeId, out: &mut Vec<(WindowId, CGRect)>, ) { match self.kind.get(node) { @@ -463,9 +450,10 @@ impl BspLayoutSystem { .. }) => { if let Some(w) = window { - let mut target = if *fullscreen { + let is_focused = node == focused_node; + let mut target = if *fullscreen && is_focused { screen - } else if *fullscreen_within_gaps { + } else if *fullscreen_within_gaps && is_focused { Self::apply_outer_gaps(screen, gaps) } else { rect @@ -549,10 +537,10 @@ impl BspLayoutSystem { ); let mut it = node.children(&self.tree.map); if let Some(first) = it.next() { - self.calculate_layout_recursive(first, r1, screen, constraints, gaps, out); + self.calculate_layout_recursive(first, r1, screen, constraints, gaps, focused_node, out); } if let Some(second) = it.next() { - self.calculate_layout_recursive(second, r2, screen, constraints, gaps, out); + self.calculate_layout_recursive(second, r2, screen, constraints, gaps, focused_node, out); } } Orientation::Vertical => { @@ -604,10 +592,10 @@ impl BspLayoutSystem { ); let mut it = node.children(&self.tree.map); if let Some(first) = it.next() { - self.calculate_layout_recursive(first, r1, screen, constraints, gaps, out); + self.calculate_layout_recursive(first, r1, screen, constraints, gaps, focused_node, out); } if let Some(second) = it.next() { - self.calculate_layout_recursive(second, r2, screen, constraints, gaps, out); + self.calculate_layout_recursive(second, r2, screen, constraints, gaps, focused_node, out); } } }, @@ -918,6 +906,68 @@ mod tests { "orthogonal max-only constraint should not change the parent split allocation" ); } + + #[test] + fn fullscreen_window_returns_to_normal_bounds_when_unfocused() { + let mut system = BspLayoutSystem::default(); + let layout = system.create_layout(); + + let w1 = w(100); + let w2 = w(101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen it + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_of_selection(layout); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, screen, "focused fullscreen window should cover screen"); + + // Move focus to w2 + system.select_window(layout, w2); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + let w2_frame = frames.get(&w2).copied().expect("w2 missing"); + + assert_ne!(w1_frame, screen, "unfocused fullscreen window should return to normal bounds"); + assert!(w1_frame.size.width < 1000.0, "w1 should be smaller than screen"); + assert!(w2_frame.size.width > 0.0, "w2 should have some space"); + } + + #[test] + fn fullscreen_window_re_expands_when_refocused() { + let mut system = BspLayoutSystem::default(); + let layout = system.create_layout(); + + let w1 = w(100); + let w2 = w(101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen it + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_of_selection(layout); + + // Move focus away and back + system.select_window(layout, w2); + system.select_window(layout, w1); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, screen, "refocused fullscreen window should cover screen again"); + } } impl LayoutSystem for BspLayoutSystem { @@ -1000,7 +1050,8 @@ impl LayoutSystem for BspLayoutSystem { let mut out = Vec::new(); if let Some(state) = self.layouts.get(layout).copied() { let rect = Self::apply_outer_gaps(screen, gaps); - self.calculate_layout_recursive(state.root, rect, screen, constraints, gaps, &mut out); + let focused = self.tree.data.selection.current_selection(state.root); + self.calculate_layout_recursive(state.root, rect, screen, constraints, gaps, focused, &mut out); } out } @@ -1459,11 +1510,19 @@ impl LayoutSystem for BspLayoutSystem { } fn has_any_fullscreen_node(&self, layout: LayoutId) -> bool { - if let Some(state) = self.layouts.get(layout).copied() { - self.has_fullscreen_in_subtree(state.root) - } else { - false - } + let Some(state) = self.layouts.get(layout).copied() else { + return false; + }; + let focused = self.tree.data.selection.current_selection(state.root); + state.root.traverse_preorder(&self.tree.map).any(|node| { + match self.kind.get(node) { + Some(NodeKind::Leaf { fullscreen, fullscreen_within_gaps, .. }) => { + (*fullscreen || *fullscreen_within_gaps) + && (node == focused || focused.ancestors(&self.tree.map).any(|a| a == node)) + } + _ => false, + } + }) } fn join_selection_with_direction(&mut self, layout: LayoutId, direction: Direction) { diff --git a/src/layout_engine/systems/master_stack.rs b/src/layout_engine/systems/master_stack.rs index 8b2c532b..4856de74 100644 --- a/src/layout_engine/systems/master_stack.rs +++ b/src/layout_engine/systems/master_stack.rs @@ -490,6 +490,7 @@ impl LayoutSystem for MasterStackLayoutSystem { if self.inner.visible_windows_in_subtree(stack).is_empty() { let rect = compute_tiling_area(screen, gaps); return self.inner.calculate_layout_for_node( + master, master, screen, rect, diff --git a/src/layout_engine/systems/scrolling.rs b/src/layout_engine/systems/scrolling.rs index f38458da..571605d3 100644 --- a/src/layout_engine/systems/scrolling.rs +++ b/src/layout_engine/systems/scrolling.rs @@ -812,9 +812,10 @@ impl LayoutSystem for ScrollingLayoutSystem { CGPoint::new(x.round(), y_cursor.round()), CGSize::new(column_width.round(), row_height.round()), ); - if state.fullscreen.contains(wid) { + let is_selected = state.selected == Some(*wid); + if state.fullscreen.contains(wid) && is_selected { frame = screen; - } else if state.fullscreen_within_gaps.contains(wid) { + } else if state.fullscreen_within_gaps.contains(wid) && is_selected { frame = tiling; } else if let Some(c) = constraints.get(wid).copied() { let c = c.normalized(); @@ -1200,7 +1201,10 @@ impl LayoutSystem for ScrollingLayoutSystem { let Some(state) = self.layout_state(layout) else { return false; }; - !state.fullscreen.is_empty() || !state.fullscreen_within_gaps.is_empty() + let Some(selected) = state.selected else { + return false; + }; + state.fullscreen.contains(&selected) || state.fullscreen_within_gaps.contains(&selected) } fn join_selection_with_direction(&mut self, layout: LayoutId, direction: Direction) { @@ -2013,4 +2017,66 @@ mod tests { after.origin.x ); } + + #[test] + fn fullscreen_window_returns_to_normal_bounds_when_unfocused() { + let mut system = ScrollingLayoutSystem::default(); + let layout = system.create_layout(); + + let w1 = wid(1, 100); + let w2 = wid(1, 101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen it + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_of_selection(layout); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, screen, "focused fullscreen window should cover screen"); + + // Move focus to w2 + system.select_window(layout, w2); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + let w2_frame = frames.get(&w2).copied().expect("w2 missing"); + + assert_ne!(w1_frame, screen, "unfocused fullscreen window should return to normal bounds"); + assert!(w1_frame.size.width < 1000.0, "w1 should be smaller than screen"); + assert!(w2_frame.size.width > 0.0, "w2 should have some space"); + } + + #[test] + fn fullscreen_window_re_expands_when_refocused() { + let mut system = ScrollingLayoutSystem::default(); + let layout = system.create_layout(); + + let w1 = wid(1, 100); + let w2 = wid(1, 101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen it + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_of_selection(layout); + + // Move focus away and back + system.select_window(layout, w2); + system.select_window(layout, w1); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, screen, "refocused fullscreen window should cover screen again"); + } } diff --git a/src/layout_engine/systems/traditional.rs b/src/layout_engine/systems/traditional.rs index 82e6f705..158a3b68 100644 --- a/src/layout_engine/systems/traditional.rs +++ b/src/layout_engine/systems/traditional.rs @@ -143,6 +143,7 @@ impl TraditionalLayoutSystem { pub(crate) fn calculate_layout_for_node( &self, node: NodeId, + focused_node: NodeId, screen: CGRect, rect: CGRect, stack_offset: f64, @@ -157,6 +158,7 @@ impl TraditionalLayoutSystem { &self.tree.map, &self.tree.data.window, &self.tree.data.selection, + focused_node, node, rect, screen, @@ -476,11 +478,13 @@ impl LayoutSystem for TraditionalLayoutSystem { ) -> Vec<(WindowId, CGRect)> { let mut sizes = vec![]; let tiling_area = compute_tiling_area(screen, gaps); + let focused = self.selection(layout); self.tree.data.layout.apply_with_gaps( &self.tree.map, &self.tree.data.window, &self.tree.data.selection, + focused, self.root(layout), tiling_area, screen, @@ -749,8 +753,12 @@ impl LayoutSystem for TraditionalLayoutSystem { fn has_any_fullscreen_node(&self, layout: LayoutId) -> bool { let root = self.root(layout); + let focused = self.selection(layout); root.traverse_preorder(&self.tree.map) - .any(|node| self.tree.data.layout.is_effectively_fullscreen(node)) + .any(|node| { + self.tree.data.layout.is_effectively_fullscreen(node) + && (node == focused || focused.ancestors(&self.tree.map).any(|a| a == node)) + }) } fn join_selection_with_direction(&mut self, layout: LayoutId, direction: Direction) { @@ -1275,11 +1283,13 @@ impl TraditionalLayoutSystem { let map = &self.tree.map; let tiling_area = compute_tiling_area(screen, gaps); + let focused = self.selection(layout); let mut stack: Vec<(NodeId, CGRect)> = vec![(self.root(layout), tiling_area)]; while let Some((node, rect)) = stack.pop() { - if self.tree.data.layout.is_effectively_fullscreen(node) { + let is_focused = node == focused || focused.ancestors(map).any(|a| a == node); + if self.tree.data.layout.is_effectively_fullscreen(node) && is_focused { continue; } @@ -1315,7 +1325,8 @@ impl TraditionalLayoutSystem { ); for (i, &child) in children.iter().enumerate().rev() { - if self.tree.data.layout.is_effectively_fullscreen(child) { + let child_is_focused = child == focused || focused.ancestors(map).any(|a| a == child); + if self.tree.data.layout.is_effectively_fullscreen(child) && child_is_focused { continue; } let child_rect = layout_res.get_focused_frame_for_index(i, i); @@ -2540,6 +2551,7 @@ impl Layout { map: &NodeMap, window: &WindowIndex, selection: &Selection, + focused_node: NodeId, node: NodeId, rect: CGRect, screen: CGRect, @@ -2552,9 +2564,10 @@ impl Layout { stack_line_vert: crate::common::config::VerticalPlacement, ) { let info = &self.info[node]; - let rect = if info.is_fullscreen { + let is_focused = node == focused_node || focused_node.ancestors(map).any(|a| a == node); + let rect = if info.is_fullscreen && is_focused { screen - } else if info.is_fullscreen_within_gaps { + } else if info.is_fullscreen_within_gaps && is_focused { compute_tiling_area(screen, gaps) } else { rect @@ -2635,6 +2648,7 @@ impl Layout { map, window, selection, + focused_node, child, frame, screen, @@ -2652,6 +2666,7 @@ impl Layout { map, window, selection, + focused_node, node, rect, screen, @@ -2668,6 +2683,7 @@ impl Layout { map, window, selection, + focused_node, node, rect, screen, @@ -2688,6 +2704,7 @@ impl Layout { map: &NodeMap, window: &WindowIndex, selection: &Selection, + focused_node: NodeId, node: NodeId, rect: CGRect, screen: CGRect, @@ -2808,6 +2825,7 @@ impl Layout { map, window, selection, + focused_node, child, child_rect, screen, @@ -4579,6 +4597,7 @@ mod tests { let gaps = crate::common::config::GapSettings::default(); let _ = system.calculate_layout_for_node( + root, root, screen, screen, @@ -5067,4 +5086,104 @@ mod tests { .expect("right node proportion missing"); assert_eq!(before, after); } + + #[test] + fn fullscreen_window_returns_to_normal_bounds_when_unfocused() { + let mut system = TraditionalLayoutSystem::default(); + let layout = system.create_layout(); + let root = system.root(layout); + system.tree.data.layout.set_kind(root, LayoutKind::Horizontal); + + let w1 = w(100); + let w2 = w(101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen it + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_of_selection(layout); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, screen, "focused fullscreen window should cover screen"); + + // Move focus to w2 + system.select_window(layout, w2); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + let w2_frame = frames.get(&w2).copied().expect("w2 missing"); + + assert_ne!(w1_frame, screen, "unfocused fullscreen window should return to normal bounds"); + assert_eq!(w1_frame.size.width + w2_frame.size.width, 1000.0, "windows should share space"); + } + + #[test] + fn fullscreen_window_re_expands_when_refocused() { + let mut system = TraditionalLayoutSystem::default(); + let layout = system.create_layout(); + let root = system.root(layout); + system.tree.data.layout.set_kind(root, LayoutKind::Horizontal); + + let w1 = w(100); + let w2 = w(101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen it + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_of_selection(layout); + + // Move focus away and back + system.select_window(layout, w2); + system.select_window(layout, w1); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, screen, "refocused fullscreen window should cover screen again"); + } + + #[test] + fn fullscreen_within_gaps_only_applies_when_focused() { + let mut system = TraditionalLayoutSystem::default(); + let layout = system.create_layout(); + let root = system.root(layout); + system.tree.data.layout.set_kind(root, LayoutKind::Horizontal); + + let w1 = w(100); + let w2 = w(101); + system.add_window_after_selection(layout, w1); + system.add_window_after_selection(layout, w2); + + let screen = CGRect::new(CGPoint::new(0.0, 0.0), CGSize::new(1000.0, 800.0)); + let gaps = crate::common::config::GapSettings::default(); + + // Focus w1 and fullscreen within gaps + system.select_window(layout, w1); + let _ = system.toggle_fullscreen_within_gaps_of_selection(layout); + + let tiling = crate::layout_engine::utils::compute_tiling_area(screen, &gaps); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_eq!(w1_frame, tiling, "focused fullscreen-within-gaps should cover tiling area"); + + // Move focus to w2 + system.select_window(layout, w2); + + let frames = system.calculate_layout(layout, screen, 0.0, &HashMap::default(), &gaps, 0.0, Default::default(), Default::default()); + let frames: HashMap = frames.into_iter().collect(); + let w1_frame = frames.get(&w1).copied().expect("w1 missing"); + assert_ne!(w1_frame, tiling, "unfocused fullscreen-within-gaps should return to normal bounds"); + } }