panes: adopt zipper based bintree for managing panes

This simplifies the logic in the tab.rs module

bintree/src/lib.rs

@@ -142,6 +142,24 @@ where
     }
 }
 
+pub struct ParentIterator<'a, L, N> {
+    path: &'a Path<L, N>,
+}
+
+impl<'a, L, N> std::iter::Iterator for ParentIterator<'a, L, N> {
+    type Item = &'a Option<N>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        match self.path {
+            Path::Top => None,
+            Path::Left { data, up, .. } | Path::Right { data, up, .. } => {
+                self.path = &*up;
+                Some(data)
+            }
+        }
+    }
+}
+
 impl<L, N> Tree<L, N> {
     /// Construct a new empty tree
     pub fn new() -> Self {
@@ -182,6 +200,22 @@ impl<L, N> Cursor<L, N> {
         }
     }
 
+    /// Returns true if the current position is the left child of its parent
+    pub fn is_left(&self) -> bool {
+        match &*self.path {
+            Path::Left { .. } => true,
+            _ => false,
+        }
+    }
+
+    /// Returns true if the current position is the right child of its parent
+    pub fn is_right(&self) -> bool {
+        match &*self.path {
+            Path::Right { .. } => true,
+            _ => false,
+        }
+    }
+
     /// If the current position is the root of the empty tree,
     /// assign an initial leaf value.
     /// Consumes the cursor and returns a new cursor representing
@@ -216,6 +250,13 @@ impl<L, N> Cursor<L, N> {
         }
     }
 
+    /// Return an iterator that will visit the chain of nodes leading
+    /// to the root from the current position and yield their node
+    /// data at each step of iteration.
+    pub fn path_to_root(&self) -> ParentIterator<L, N> {
+        ParentIterator { path: &*self.path }
+    }
+
     /// If the current position is not a leaf node, assign the
     /// node data to the supplied value.
     /// Consumes the cursor and returns a new cursor representing the
@@ -368,6 +409,20 @@ impl<L, N> Cursor<L, N> {
         }
     }
 
+    /// Move to the nth (preorder) leaf from the current position.
+    pub fn go_to_nth_leaf(mut self, n: usize) -> Result<Self, Self> {
+        let mut next = 0;
+        loop {
+            if self.is_leaf() {
+                if next == n {
+                    return Ok(self);
+                }
+                next += 1;
+            }
+            self = self.preorder_next()?;
+        }
+    }
+
     /// Consume the cursor and return the root of the Tree
     pub fn tree(mut self) -> Tree<L, N> {
         loop {

pty/src/lib.rs

@@ -62,7 +62,7 @@ pub mod ssh;
 pub mod serial;
 
 /// Represents the size of the visible display area in the pty
-#[derive(Debug, Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(feature = "serde_support", derive(Serialize, Deserialize))]
 pub struct PtySize {
     /// The number of lines of text

src/mux/tab.rs

@@ -2,6 +2,7 @@ use crate::mux::domain::DomainId;
 use crate::mux::renderable::Renderable;
 use crate::mux::Mux;
 use async_trait::async_trait;
+use bintree::Tree;
 use downcast_rs::{impl_downcast, Downcast};
 use portable_pty::PtySize;
 use serde::{Deserialize, Serialize};
@@ -92,7 +93,7 @@ pub struct SearchResult {
 /// At this time only a single pane is supported
 pub struct Tab {
     id: TabId,
-    pane: RefCell<Option<PaneNode>>,
+    pane: RefCell<Option<Tree<Rc<dyn Pane>, SplitDirectionAndSize>>>,
     size: RefCell<PtySize>,
     active: RefCell<usize>,
 }
@@ -117,133 +118,28 @@ pub struct PositionedPane {
     pub pane: Rc<dyn Pane>,
 }
 
-/// A tab contains a tree of PaneNode's.
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
-#[derive(Clone)]
+pub enum SplitDirection {
-enum PaneNode {
+    Horizontal,
-    /// This node is filled with a single Pane
+    Vertical,
-    Single(Rc<dyn Pane>),
-    /// This node is split horizontally in two.
-    HorizontalSplit {
-        left: Box<PaneNode>,
-        left_width: usize,
-        right: Box<PaneNode>,
-    },
-    /// This node is split vertically in two.
-    VerticalSplit {
-        top: Box<PaneNode>,
-        top_height: usize,
-        bottom: Box<PaneNode>,
-    },
 }
 
-impl PaneNode {
+/// The size is of the (first, second) child of the split
-    /// Returns true if this node or any of its children are
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
-    /// alive.  Stops evaluating as soon as it identifies that
+struct SplitDirectionAndSize {
-    /// something is alive.
+    direction: SplitDirection,
-    pub fn is_alive(&self) -> bool {
+    /// Offset relative to container
-        match self {
+    left: usize,
-            PaneNode::Single(p) => !p.is_dead(),
+    top: usize,
-            PaneNode::HorizontalSplit { left, right, .. } => left.is_alive() || right.is_alive(),
+    first: PtySize,
-            PaneNode::VerticalSplit { top, bottom, .. } => top.is_alive() || bottom.is_alive(),
+    second: PtySize,
-        }
-    }
-
-    /// Returns a ref to the PaneNode::Single that contains a pane
-    /// given its topological index.
-    /// The if topological index is invalid, returns None.
-    fn node_by_index_mut(
-        &mut self,
-        wanted_index: usize,
-        current_index: &mut usize,
-    ) -> Option<&mut Self> {
-        match self {
-            PaneNode::Single(_) => {
-                if wanted_index == *current_index {
-                    Some(self)
-                } else {
-                    *current_index += 1;
-                    None
-                }
-            }
-            PaneNode::HorizontalSplit { left, right, .. } => {
-                if let Some(found) = left.node_by_index_mut(wanted_index, current_index) {
-                    Some(found)
-                } else {
-                    right.node_by_index_mut(wanted_index, current_index)
-                }
-            }
-            PaneNode::VerticalSplit { top, bottom, .. } => {
-                if let Some(found) = top.node_by_index_mut(wanted_index, current_index) {
-                    Some(found)
-                } else {
-                    bottom.node_by_index_mut(wanted_index, current_index)
-                }
-            }
-        }
-    }
-
-    /// Recursively Walk to compute the positioning information
-    fn walk(
-        &self,
-        active_index: usize,
-        x: usize,
-        y: usize,
-        width: usize,
-        height: usize,
-        panes: &mut Vec<PositionedPane>,
-    ) {
-        match self {
-            PaneNode::Single(p) => {
-                let index = panes.len();
-                panes.push(PositionedPane {
-                    index,
-                    is_active: index == active_index,
-                    left: x,
-                    top: y,
-                    width,
-                    height,
-                    pane: Rc::clone(p),
-                });
-            }
-            PaneNode::HorizontalSplit {
-                left,
-                left_width,
-                right,
-            } => {
-                left.walk(active_index, x, y, *left_width, height, panes);
-                right.walk(
-                    active_index,
-                    x + *left_width,
-                    y,
-                    width.saturating_sub(*left_width),
-                    height,
-                    panes,
-                );
-            }
-            PaneNode::VerticalSplit {
-                top,
-                top_height,
-                bottom,
-            } => {
-                top.walk(active_index, x, y, width, *top_height, panes);
-                bottom.walk(
-                    active_index,
-                    x,
-                    y + *top_height,
-                    width,
-                    height.saturating_sub(*top_height),
-                    panes,
-                );
-            }
-        }
-    }
 }
 
 impl Tab {
     pub fn new(size: &PtySize) -> Self {
         Self {
             id: TAB_ID.fetch_add(1, ::std::sync::atomic::Ordering::Relaxed),
-            pane: RefCell::new(None),
+            pane: RefCell::new(Some(Tree::new())),
             size: RefCell::new(*size),
             active: RefCell::new(0),
         }
@@ -253,17 +149,56 @@ impl Tab {
     /// list of PositionedPane instances along with their positioning information.
     pub fn iter_panes(&self) -> Vec<PositionedPane> {
         let mut panes = vec![];
-        let size = *self.size.borrow();
+        let active_idx = *self.active.borrow();
+        let mut root = self.pane.borrow_mut();
+        let mut cursor = root.take().unwrap().cursor();
 
-        if let Some(pane) = self.pane.borrow().as_ref() {
+        loop {
-            pane.walk(
+            if cursor.is_leaf() {
-                *self.active.borrow(),
+                let index = panes.len();
-                0,
+                let mut left = 0usize;
-                0,
+                let mut top = 0usize;
-                size.cols as _,
+                let mut parent_size = None;
-                size.rows as _,
+                let is_second = cursor.is_right();
-                &mut panes,
+                for node in cursor.path_to_root() {
-            );
+                    if let Some(node) = node {
+                        if parent_size.is_none() {
+                            parent_size.replace(if is_second { node.second } else { node.first });
+                            if is_second {
+                                match node.direction {
+                                    SplitDirection::Vertical => top += node.first.rows as usize + 1,
+                                    SplitDirection::Horizontal => {
+                                        left += node.first.cols as usize + 1
+                                    }
+                                }
+                            }
+                        }
+                        left += node.left;
+                        top += node.top;
+                    }
+                }
+
+                let pane = Rc::clone(cursor.leaf_mut().unwrap());
+                let dims = parent_size.unwrap_or_else(|| *self.size.borrow());
+
+                panes.push(PositionedPane {
+                    index,
+                    is_active: index == active_idx,
+                    left,
+                    top,
+                    width: dims.cols as _,
+                    height: dims.rows as _,
+                    pane,
+                });
+            }
+
+            match cursor.preorder_next() {
+                Ok(c) => cursor = c,
+                Err(c) => {
+                    root.replace(c.tree());
+                    break;
+                }
+            }
         }
 
         panes
@@ -274,12 +209,15 @@ impl Tab {
     }
 
     pub fn is_dead(&self) -> bool {
-        if let Some(pane) = self.pane.borrow().as_ref() {
+        let panes = self.iter_panes();
-            !pane.is_alive()
+        let mut dead_count = 0;
-        } else {
+        for pos in &panes {
-            true
+            if pos.pane.is_dead() {
+                dead_count += 1;
             }
         }
+        dead_count == panes.len()
+    }
 
     pub fn get_active_pane(&self) -> Option<Rc<dyn Pane>> {
         self.iter_panes()
@@ -292,9 +230,10 @@ impl Tab {
     /// This is suitable when creating a new tab and then assigning
     /// the initial pane
     pub fn assign_pane(&self, pane: &Rc<dyn Pane>) {
-        self.pane
+        match Tree::new().cursor().assign_top(Rc::clone(pane)) {
-            .borrow_mut()
+            Ok(c) => *self.pane.borrow_mut() = Some(c.tree()),
-            .replace(PaneNode::Single(Rc::clone(pane)));
+            Err(_) => panic!("tried to assign root pane to non-empty tree"),
+        }
     }
 
     fn cell_dimensions(&self) -> PtySize {
@@ -312,23 +251,48 @@ impl Tab {
     /// The intent is to call this prior to spawning the new pane so that
     /// you can create it with the correct size.
     /// May return None if the specified pane_index is invalid.
-    pub fn compute_split_size(
+    fn compute_split_size(
         &self,
         pane_index: usize,
         direction: SplitDirection,
-    ) -> Option<PtySize> {
+    ) -> Option<SplitDirectionAndSize> {
         let cell_dims = self.cell_dimensions();
 
         self.iter_panes().iter().nth(pane_index).map(|pos| {
-            let (width, height) = match direction {
+            fn split_dimension(dim: usize) -> (usize, usize) {
-                SplitDirection::Horizontal => (pos.width / 2, pos.height),
+                let halved = dim / 2;
-                SplitDirection::Vertical => (pos.width, pos.height / 2),
+                if halved * 2 == dim {
+                    // Was an even size; we need to allow 1 cell to render
+                    // the split UI, so make the newly created leaf slightly
+                    // smaller
+                    (halved, halved.saturating_sub(1))
+                } else {
+                    (halved, halved)
+                }
+            }
+
+            let ((width1, width2), (height1, height2)) = match direction {
+                SplitDirection::Horizontal => {
+                    (split_dimension(pos.width), (pos.height, pos.height))
+                }
+                SplitDirection::Vertical => ((pos.width, pos.width), split_dimension(pos.height)),
             };
-            PtySize {
+            SplitDirectionAndSize {
-                rows: height as _,
+                direction,
-                cols: width as _,
+                left: pos.left,
-                pixel_width: cell_dims.pixel_width * width as u16,
+                top: pos.top,
-                pixel_height: cell_dims.pixel_height * height as u16,
+                first: PtySize {
+                    rows: height1 as _,
+                    cols: width1 as _,
+                    pixel_height: cell_dims.pixel_height * height1 as u16,
+                    pixel_width: cell_dims.pixel_width * width1 as u16,
+                },
+                second: PtySize {
+                    rows: height2 as _,
+                    cols: width2 as _,
+                    pixel_height: cell_dims.pixel_height * height2 as u16,
+                    pixel_width: cell_dims.pixel_width * width2 as u16,
+                },
             }
         })
     }
@@ -343,62 +307,41 @@ impl Tab {
         direction: SplitDirection,
         pane: Rc<dyn Pane>,
     ) -> anyhow::Result<usize> {
-        let new_size = self
+        let split_info = self
             .compute_split_size(pane_index, direction)
             .ok_or_else(|| anyhow::anyhow!("invalid pane_index {}; cannot split!", pane_index))?;
 
-        pane.resize(new_size.clone())?;
+        let mut root = self.pane.borrow_mut();
-        let new_pane = Box::new(PaneNode::Single(pane));
+        let mut cursor = root.take().unwrap().cursor();
 
-        if let Some(root_node) = self.pane.borrow_mut().as_mut() {
+        match cursor.go_to_nth_leaf(pane_index) {
-            let mut active = 0;
+            Ok(c) => cursor = c,
-            let node = root_node
+            Err(c) => {
-                .node_by_index_mut(pane_index, &mut active)
+                root.replace(c.tree());
-                .ok_or_else(|| {
+                anyhow::bail!("invalid pane_index {}; cannot split!", pane_index);
-                    anyhow::anyhow!("invalid pane_index {}; cannot split!", pane_index)
-                })?;
-
-            let prior_node = match node {
-                PaneNode::Single(orig_pane) => {
-                    orig_pane.resize(new_size)?;
-                    Box::new(PaneNode::Single(Rc::clone(orig_pane)))
             }
-                _ => unreachable!("impossible PaneNode variant returned from node_by_index_mut"),
         };
 
-            match direction {
+        pane.resize(split_info.second.clone())?;
-                SplitDirection::Horizontal => {
-                    *node = PaneNode::HorizontalSplit {
-                        left: prior_node,
-                        right: new_pane,
-                        left_width: new_size.cols as _,
-                    }
-                }
 
-                SplitDirection::Vertical => {
+        match cursor.split_leaf_and_insert_right(pane) {
-                    *node = PaneNode::VerticalSplit {
+            Ok(c) => cursor = c,
-                        top: prior_node,
+            Err(c) => {
-                        bottom: new_pane,
+                root.replace(c.tree());
-                        top_height: new_size.rows as _,
+                anyhow::bail!("invalid pane_index {}; cannot split!", pane_index);
-                    }
-                }
             }
+        };
 
-            let new_index = pane_index + 1;
+        // cursor now points to the newly created split node;
+        // we need to populate its split information
+        match cursor.assign_node(Some(split_info)) {
+            Err(c) | Ok(c) => root.replace(c.tree()),
+        };
 
-            *self.active.borrow_mut() = new_index;
+        *self.active.borrow_mut() = pane_index + 1;
 
-            Ok(new_index)
+        Ok(pane_index + 1)
-        } else {
-            anyhow::bail!("no panes have been assigned; cannot split!");
     }
-    }
-}
-
-#[derive(Debug, Clone, Copy, Eq, PartialEq)]
-pub enum SplitDirection {
-    Horizontal,
-    Vertical,
 }
 
 /// A Pane represents a view on a terminal
@@ -558,27 +501,53 @@ mod test {
             .unwrap();
         assert_eq!(
             horz_size,
-            PtySize {
+            SplitDirectionAndSize {
+                direction: SplitDirection::Horizontal,
+                left: 0,
+                top: 0,
+                first: PtySize {
                     rows: 24,
                     cols: 40,
                     pixel_width: 400,
                     pixel_height: 600
+                },
+                second: PtySize {
+                    rows: 24,
+                    cols: 39,
+                    pixel_width: 390,
+                    pixel_height: 600
+                },
             }
         );
 
         let vert_size = tab.compute_split_size(0, SplitDirection::Vertical).unwrap();
         assert_eq!(
             vert_size,
-            PtySize {
+            SplitDirectionAndSize {
+                direction: SplitDirection::Vertical,
+                left: 0,
+                top: 0,
+                first: PtySize {
                     rows: 12,
                     cols: 80,
                     pixel_width: 800,
                     pixel_height: 300
+                },
+                second: PtySize {
+                    rows: 11,
+                    cols: 80,
+                    pixel_width: 800,
+                    pixel_height: 275
+                }
             }
         );
 
         let new_index = tab
-            .split_and_insert(0, SplitDirection::Horizontal, FakePane::new(2, horz_size))
+            .split_and_insert(
+                0,
+                SplitDirection::Horizontal,
+                FakePane::new(2, horz_size.second),
+            )
             .unwrap();
         assert_eq!(new_index, 1);
 
@@ -595,15 +564,19 @@ mod test {
 
         assert_eq!(1, panes[1].index);
         assert_eq!(true, panes[1].is_active);
-        assert_eq!(40, panes[1].left);
+        assert_eq!(41, panes[1].left);
         assert_eq!(0, panes[1].top);
-        assert_eq!(40, panes[1].width);
+        assert_eq!(39, panes[1].width);
         assert_eq!(24, panes[1].height);
         assert_eq!(2, panes[1].pane.pane_id());
 
         let vert_size = tab.compute_split_size(0, SplitDirection::Vertical).unwrap();
         let new_index = tab
-            .split_and_insert(0, SplitDirection::Vertical, FakePane::new(3, vert_size))
+            .split_and_insert(
+                0,
+                SplitDirection::Vertical,
+                FakePane::new(3, vert_size.second),
+            )
             .unwrap();
         assert_eq!(new_index, 1);
 
@@ -621,16 +594,16 @@ mod test {
         assert_eq!(1, panes[1].index);
         assert_eq!(true, panes[1].is_active);
         assert_eq!(0, panes[1].left);
-        assert_eq!(12, panes[1].top);
+        assert_eq!(13, panes[1].top);
         assert_eq!(40, panes[1].width);
-        assert_eq!(12, panes[1].height);
+        assert_eq!(11, panes[1].height);
         assert_eq!(3, panes[1].pane.pane_id());
 
         assert_eq!(2, panes[2].index);
         assert_eq!(false, panes[2].is_active);
-        assert_eq!(40, panes[2].left);
+        assert_eq!(41, panes[2].left);
         assert_eq!(0, panes[2].top);
-        assert_eq!(40, panes[2].width);
+        assert_eq!(39, panes[2].width);
         assert_eq!(24, panes[2].height);
         assert_eq!(2, panes[2].pane.pane_id());
     }