Preserve tree stack windows during layout conversion

crates/ferese-core/src/workspace.rs

@@ -67,7 +67,7 @@ impl WorkspaceLayout {
     pub fn preferred_window(&self) -> Option<WindowId> {
         match self {
             Self::Scrolling(layout) => layout.active_window(),
-            Self::Tree(layout) => layout.window_ids().next(),
+            Self::Tree(layout) => layout.preferred_window(),
         }
     }
 
@@ -1483,6 +1483,99 @@ mod tests {
         assert!(workspaces.validate().is_ok());
     }
 
+    fn tree_stack_workspaces(ids: &[WindowId]) -> WorkspaceSet {
+        let mut workspaces = WorkspaceSet::new(
+            LayoutMode::Tree,
+            ColumnWidth::Proportion(0.5),
+            ViewportFocusStrategy::Minimal,
+        );
+        workspaces.insert_window(ids[0], Axis::Horizontal, 0.5).unwrap();
+        for pair in ids.windows(2) {
+            workspaces.insert_window(pair[1], Axis::Horizontal, 0.5).unwrap();
+            workspaces.stack_window(pair[1], pair[0]).unwrap();
+        }
+        workspaces
+    }
+
+    #[test]
+    fn stacked_tree_mode_conversions_preserve_membership_focus_and_order() {
+        let bounds = Rect::new(0.0, 0.0, 1_000.0, 800.0);
+        let all_ids = [WindowId(30), WindowId(10), WindowId(20), WindowId(5), WindowId(40)];
+        for count in [2, 3, 5] {
+            let ids = &all_ids[..count];
+            let expected = ids.iter().copied().collect::<HashSet<_>>();
+            for focused in ids {
+                // Fresh maps have independent hash seeds; structural ordering
+                // must give the same result for each conversion.
+                for _ in 0..8 {
+                    let mut workspaces = tree_stack_workspaces(ids);
+                    workspaces.focus_window(*focused).unwrap();
+                    // A saved valid focus may refer to an inactive stack member.
+                    let WorkspaceLayout::Tree(tree) = &mut workspaces.active_mut().layout else {
+                        panic!()
+                    };
+                    tree.activate_window(ids[1]).unwrap();
+                    assert_eq!(tree.geometry(bounds).unwrap().len(), 1);
+                    assert_eq!(workspaces.active().last_focused, Some(*focused));
+                    assert!(workspaces.validate().is_ok());
+
+                    for mode in [LayoutMode::Scrolling, LayoutMode::Tree, LayoutMode::Scrolling] {
+                        assert!(workspaces.set_active_layout_mode(mode, bounds).unwrap());
+                        let windows = workspaces.active().layout.window_ids().collect::<Vec<_>>();
+                        assert_eq!(windows.len(), ids.len());
+                        assert_eq!(windows.iter().copied().collect::<HashSet<_>>(), expected);
+                        assert_eq!(workspaces.active().last_focused, Some(*focused));
+                        assert!(workspaces.validate().is_ok());
+                        if let WorkspaceLayout::Scrolling(layout) = &workspaces.active().layout {
+                            assert_eq!(windows, ids);
+                            assert_eq!(layout.active_window(), Some(*focused));
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn nested_tree_splits_and_stacks_survive_round_trip_conversion() {
+        let bounds = Rect::new(0.0, 0.0, 1_000.0, 800.0);
+        let mut workspaces = tree_stack_workspaces(&[WindowId(30), WindowId(10), WindowId(20)]);
+        workspaces.focus_window(WindowId(10)).unwrap();
+        workspaces.insert_window(WindowId(5), Axis::Vertical, 0.5).unwrap();
+        workspaces.insert_window(WindowId(40), Axis::Horizontal, 0.5).unwrap();
+        workspaces.insert_window(WindowId(50), Axis::Vertical, 0.5).unwrap();
+        workspaces.stack_window(WindowId(50), WindowId(40)).unwrap();
+        workspaces.focus_window(WindowId(40)).unwrap();
+        let before = workspaces.active().layout.window_ids().collect::<HashSet<_>>();
+        assert_eq!(before.len(), 6);
+        assert!(workspaces.active().layout.geometry(bounds).unwrap().len() < before.len());
+        for mode in [LayoutMode::Scrolling, LayoutMode::Tree, LayoutMode::Scrolling] {
+            assert!(workspaces.set_active_layout_mode(mode, bounds).unwrap());
+            let windows = workspaces.active().layout.window_ids().collect::<Vec<_>>();
+            assert_eq!(windows.len(), before.len());
+            assert_eq!(windows.into_iter().collect::<HashSet<_>>(), before);
+            assert_eq!(workspaces.active().last_focused, Some(WindowId(40)));
+            assert!(workspaces.validate().is_ok());
+            if let WorkspaceLayout::Scrolling(layout) = &workspaces.active().layout {
+                assert_eq!(layout.active_window(), Some(WindowId(40)));
+            }
+        }
+    }
+
+    #[test]
+    fn tree_preferred_window_is_deterministic_without_a_saved_focus() {
+        for _ in 0..32 {
+            let mut workspaces = tree_stack_workspaces(&[WindowId(30), WindowId(20), WindowId(40)]);
+            workspaces.active_mut().last_focused = None;
+            assert_eq!(workspaces.active().layout.preferred_window(), Some(WindowId(40)));
+            workspaces.insert_window(WindowId(10), Axis::Horizontal, 0.5).unwrap();
+            workspaces.remove_window(WindowId(10)).unwrap();
+            assert_eq!(workspaces.active().last_focused, Some(WindowId(40)));
+            assert_eq!(workspaces.active().layout.preferred_window(), Some(WindowId(40)));
+            assert!(workspaces.validate().is_ok());
+        }
+    }
+
     #[test]
     fn scrolling_column_commands_preserve_workspace_state() {
         let mut workspaces = WorkspaceSet::default();

crates/ferese-layout/src/lib.rs

@@ -165,15 +165,23 @@ impl LayoutTree {
         self.windows.keys().copied()
     }
 
+    pub fn preferred_window(&self) -> Option<WindowId> {
+        let leaf = self.first_window(self.root?).ok()?;
+        match self.nodes.get(&leaf)? {
+            Node::Window(window) => Some(*window),
+            _ => None,
+        }
+    }
+
+    /// Include every structural leaf, even inactive stack members. Preserve
+    /// stack child order when their positions coincide.
     pub fn window_ids_in_reading_order(&self, bounds: Rect) -> Result<Vec<WindowId>, LayoutError> {
-        let geometry = self.geometry(bounds)?;
+        self.validate()?;
-        let mut windows = geometry.into_iter().collect::<Vec<_>>();
+        let mut windows = Vec::with_capacity(self.windows.len());
-        windows.sort_by(|(left_window, left), (right_window, right)| {
+        if let Some(root) = self.root {
-            left.x
+            self.collect_window_positions(root, bounds, &mut windows)?;
-                .total_cmp(&right.x)
+        }
-                .then_with(|| left.y.total_cmp(&right.y))
+        windows.sort_by(|(_, left), (_, right)| left.x.total_cmp(&right.x).then_with(|| left.y.total_cmp(&right.y)));
-                .then_with(|| left_window.0.cmp(&right_window.0))
-        });
         Ok(windows.into_iter().map(|(window, _)| window).collect())
     }
 
@@ -624,6 +632,34 @@ impl LayoutTree {
         id
     }
 
+    fn collect_window_positions(
+        &self,
+        node: NodeId,
+        bounds: Rect,
+        windows: &mut Vec<(WindowId, Rect)>,
+    ) -> Result<(), LayoutError> {
+        match self.nodes.get(&node) {
+            Some(Node::Window(window)) => windows.push((*window, bounds)),
+            Some(Node::Split {
+                axis,
+                ratio,
+                first,
+                second,
+            }) => {
+                let (first_bounds, second_bounds) = split_rect(bounds, *axis, *ratio);
+                self.collect_window_positions(*first, first_bounds, windows)?;
+                self.collect_window_positions(*second, second_bounds, windows)?;
+            }
+            Some(Node::Stack { children, .. }) => {
+                for child in children {
+                    self.collect_window_positions(*child, bounds, windows)?;
+                }
+            }
+            None => return Err(LayoutError::InvalidTree("node is missing")),
+        }
+        Ok(())
+    }
+
     fn first_window(&self, node: NodeId) -> Result<NodeId, LayoutError> {
         match self.nodes.get(&node) {
             Some(Node::Window(_)) => Ok(node),
@@ -1232,6 +1268,99 @@ mod tests {
         assert!(tree.validate().is_ok());
     }
 
+    #[test]
+    fn structural_reading_order_includes_all_stack_members_independently_of_activation() {
+        let mut tree = LayoutTree::default();
+        let ids = [WindowId(30), WindowId(10), WindowId(20), WindowId(5)];
+        tree.insert(ids[0], None, Axis::Horizontal, 0.5).unwrap();
+        for pair in ids.windows(2) {
+            tree.insert(pair[1], Some(pair[0]), Axis::Horizontal, 0.5).unwrap();
+            tree.stack_window(pair[1], pair[0]).unwrap();
+        }
+        let bounds = Rect::new(0.0, 0.0, 1_000.0, 800.0);
+        for active in ids {
+            tree.activate_window(active).unwrap();
+            assert_eq!(
+                tree.geometry(bounds).unwrap().keys().copied().collect::<Vec<_>>(),
+                vec![active]
+            );
+            assert_eq!(tree.window_ids_in_reading_order(bounds).unwrap(), ids);
+            assert_eq!(tree.preferred_window(), Some(active));
+            assert!(tree.validate().is_ok());
+        }
+    }
+
+    #[test]
+    fn flat_stack_reading_order_preserves_every_inactive_member() {
+        let mut tree = LayoutTree::default();
+        let ids = [WindowId(900), WindowId(7), WindowId(400), WindowId(10)];
+        let children = ids
+            .iter()
+            .map(|window| {
+                let leaf = tree.insert_node(Node::Window(*window));
+                tree.windows.insert(*window, leaf);
+                leaf
+            })
+            .collect::<Vec<_>>();
+        let root = tree.insert_node(Node::Stack {
+            children: children.clone(),
+            active: 2,
+        });
+        for child in children {
+            tree.parents.insert(child, root);
+        }
+        tree.root = Some(root);
+        let bounds = Rect::new(0.0, 0.0, 1_000.0, 800.0);
+        assert!(tree.validate().is_ok());
+        assert_eq!(tree.geometry(bounds).unwrap().len(), 1);
+        assert_eq!(tree.preferred_window(), Some(ids[2]));
+        assert_eq!(tree.window_ids_in_reading_order(bounds).unwrap(), ids);
+    }
+
+    #[test]
+    fn structural_reading_order_retains_xy_order_through_splits_and_stacks() {
+        let mut tree = LayoutTree::default();
+        tree.insert(WindowId(40), None, Axis::Horizontal, 0.5).unwrap();
+        tree.insert(WindowId(10), Some(WindowId(40)), Axis::Vertical, 0.5)
+            .unwrap();
+        tree.insert(WindowId(30), Some(WindowId(40)), Axis::Horizontal, 0.5)
+            .unwrap();
+        tree.insert(WindowId(20), Some(WindowId(40)), Axis::Vertical, 0.5)
+            .unwrap();
+        tree.stack_window(WindowId(20), WindowId(40)).unwrap();
+        tree.insert(WindowId(50), Some(WindowId(10)), Axis::Horizontal, 0.5)
+            .unwrap();
+        let bounds = Rect::new(25.0, 40.0, 1_000.0, 800.0);
+        let expected = [40, 20, 10, 30, 50].map(WindowId);
+        assert_eq!(tree.window_ids_in_reading_order(bounds).unwrap(), expected);
+        assert_eq!(tree.geometry(bounds).unwrap().len(), 4);
+        let collapsed = tree.window_ids_in_reading_order(Rect::default()).unwrap();
+        assert_eq!(collapsed.len(), expected.len());
+        assert_eq!(
+            collapsed.into_iter().collect::<HashSet<_>>(),
+            expected.into_iter().collect()
+        );
+        assert!(tree.validate().is_ok());
+    }
+
+    #[test]
+    fn tree_preference_follows_first_split_branch_and_active_stack_children() {
+        let mut tree = LayoutTree::default();
+        assert_eq!(tree.preferred_window(), None);
+        assert!(tree.window_ids_in_reading_order(Rect::default()).unwrap().is_empty());
+        tree.insert(WindowId(30), None, Axis::Horizontal, 0.5).unwrap();
+        tree.insert(WindowId(10), Some(WindowId(30)), Axis::Horizontal, 0.5)
+            .unwrap();
+        tree.insert(WindowId(20), Some(WindowId(30)), Axis::Vertical, 0.5)
+            .unwrap();
+        tree.stack_window(WindowId(20), WindowId(30)).unwrap();
+        assert_eq!(tree.preferred_window(), Some(WindowId(20)));
+        tree.activate_window(WindowId(30)).unwrap();
+        assert_eq!(tree.preferred_window(), Some(WindowId(30)));
+        tree.activate_window(WindowId(10)).unwrap();
+        assert_eq!(tree.preferred_window(), Some(WindowId(30)));
+    }
+
     #[test]
     fn activating_a_hidden_stack_child_changes_visible_geometry() {
         let mut tree = LayoutTree::default();