feature: working window traversal algorithm minus some pain points

src/miracle_window_management_policy.cpp

@@ -32,7 +32,7 @@ MiracleWindowManagementPolicy::MiracleWindowManagementPolicy(
     miral::ExternalClientLauncher const& external_client_launcher,
     miral::InternalClientLauncher const& internal_client_launcher)
     : miral::MinimalWindowManager(tools),
-      tree{geom::Size{1280, 1016}}, // TODO: Don't hardcode! Ask the compositor
+      tree{geom::Size{1280, 1016}, tools}, // TODO: Don't hardcode! Ask the compositor
       window_manager_tools{tools},
       external_client_launcher{external_client_launcher},
       internal_client_launcher{internal_client_launcher}
@@ -75,40 +75,48 @@ bool MiracleWindowManagementPolicy::handle_keyboard_event(MirKeyboardEvent const
         {
             if (modifiers & mir_input_event_modifier_shift)
             {
-                if (tree.try_move_active_window(WindowMoveDirection::up))
+                if (tree.try_move_active_window(Direction::up))
                     return true;
             }
-            else if (tree.try_resize_active_window(WindowResizeDirection::up))
+            else if (tree.try_resize_active_window(Direction::up))
+                return true;
+            else if (tree.try_select_next(Direction::up))
                 return true;
         }
         else if (scan_code == KEY_DOWN)
         {
             if (modifiers & mir_input_event_modifier_shift)
             {
-                if (tree.try_move_active_window(WindowMoveDirection::down))
+                if (tree.try_move_active_window(Direction::down))
                     return true;
             }
-            else if (tree.try_resize_active_window(WindowResizeDirection::down))
+            else if (tree.try_resize_active_window(Direction::down))
+                return true;
+            else if (tree.try_select_next(Direction::down))
                 return true;
         }
         else if (scan_code == KEY_LEFT)
         {
             if (modifiers & mir_input_event_modifier_shift)
             {
-                if (tree.try_move_active_window(WindowMoveDirection::left))
+                if (tree.try_move_active_window(Direction::left))
                     return true;
             }
-            else if (tree.try_resize_active_window(WindowResizeDirection::left))
+            else if (tree.try_resize_active_window(Direction::left))
+                return true;
+            else if (tree.try_select_next(Direction::left))
                 return true;
         }
         else if (scan_code == KEY_RIGHT)
         {
             if (modifiers & mir_input_event_modifier_shift)
             {
-                if (tree.try_move_active_window(WindowMoveDirection::right))
+                if (tree.try_move_active_window(Direction::right))
                     return true;
             }
-            else if (tree.try_resize_active_window(WindowResizeDirection::right))
+            else if (tree.try_resize_active_window(Direction::right))
+                return true;
+            else if (tree.try_select_next(Direction::right))
                 return true;
         }
     }
@@ -130,7 +138,6 @@ void MiracleWindowManagementPolicy::handle_window_ready(miral::WindowInfo &windo
     // The new placement has been confirmed. We can now add the window into the pending position
     // in the tree. This comes _after_ place_new_window has been called.
     tree.confirm(window_info.window());
-    tools.select_active_window(window_info.window());
 }
 
 void MiracleWindowManagementPolicy::advise_focus_gained(const miral::WindowInfo &window_info)

src/node.cpp

@@ -16,6 +16,7 @@
 
 #include "node.h"
 #include <cmath>
+#include <iostream>
 
 using namespace miracle;
 
@@ -48,7 +49,7 @@ geom::Rectangle Node::new_node_position(int index)
     if (index < 0)
         index = sub_nodes.size();
 
-    if (direction == NodeDirection::horizontal)
+    if (direction == NodeLayoutDirection::horizontal)
     {
         auto width_per_item = area.size.width.as_int() / static_cast<float>(sub_nodes.size() + 1);
         auto new_size = geom::Size{geom::Width{width_per_item}, area.size.height};
@@ -125,7 +126,7 @@ void Node::add_node(std::shared_ptr<Node> node)
 
 void Node::redistribute_size()
 {
-    if (direction == NodeDirection::horizontal)
+    if (direction == NodeLayoutDirection::horizontal)
     {
         int total_width = 0;
         for (auto node : sub_nodes)
@@ -179,7 +180,7 @@ void Node::set_rectangle(geom::Rectangle target_rect)
         // neighbor takes up the remaining 600px, horizontally).
         // We need to look at the target dimension and scale everyone relative to that.
         // However, the "non-main-axis" dimension will be consistent across each node.
-        if (direction == NodeDirection::horizontal)
+        if (direction == NodeLayoutDirection::horizontal)
         {
             for (size_t idx = 0; idx < sub_nodes.size(); idx++)
             {
@@ -324,3 +325,47 @@ void Node::insert_node(std::shared_ptr<Node> node, int index)
     node->set_rectangle(position);
     sub_nodes.insert(sub_nodes.begin() + index, node);
 }
+
+int Node::get_index_of_node(std::shared_ptr<Node> node)
+{
+    for (int i = 0; i < sub_nodes.size(); i++)
+        if (sub_nodes[i] == node)
+            return i;
+
+    return -1;
+}
+
+int Node::num_nodes()
+{
+    return sub_nodes.size();
+}
+
+std::shared_ptr<Node> Node::node_at(int i)
+{
+    if (i < 0 || i >= num_nodes())
+        return nullptr;
+
+    return sub_nodes[i];
+}
+
+std::shared_ptr<Node> Node::find_first_window_child()
+{
+    if (is_window())
+        return shared_from_this();
+    
+    for (auto node : sub_nodes)
+    {
+        if (node->is_window())
+            return node;
+    }
+
+    for (auto node : sub_nodes)
+    {
+        auto first_child = node->find_first_window_child();
+        if (first_child)
+            return nullptr;
+    }
+
+    std::cerr << "Cannot discover a first child for this lane\n";
+    return nullptr;
+}

src/node.h

@@ -33,7 +33,7 @@ enum class NodeState
     lane
 };
 
-enum class NodeDirection
+enum class NodeLayoutDirection
 {
     horizontal,
     vertical
@@ -77,18 +77,23 @@ public:
     bool is_root() { return parent == nullptr; }
     bool is_window() { return state == NodeState::window; }
     bool is_lane() { return state == NodeState::lane; }
-    NodeDirection get_direction() { return direction; }
+    NodeLayoutDirection get_direction() { return direction; }
     miral::Window& get_window() { return window; }
     std::vector<std::shared_ptr<Node>>& get_sub_nodes() { return sub_nodes; }
-    void set_direction(NodeDirection in_direction) { direction = in_direction; }
+    void set_direction(NodeLayoutDirection in_direction) { direction = in_direction; }
+
+    int get_index_of_node(std::shared_ptr<Node>);
+    int num_nodes();
+    std::shared_ptr<Node> node_at(int i);
 
     void to_lane();
+    std::shared_ptr<Node> find_first_window_child();
 
 private:
     miral::Window window;
     std::vector<std::shared_ptr<Node>> sub_nodes;
     NodeState state;
-    NodeDirection direction = NodeDirection::horizontal;
+    NodeLayoutDirection direction = NodeLayoutDirection::horizontal;
     geom::Rectangle area;
 };
 }

src/window_tree.cpp

@@ -22,8 +22,9 @@
 
 using namespace miracle;
 
-WindowTree::WindowTree(geom::Size default_size)
+WindowTree::WindowTree(geom::Size default_size, const miral::WindowManagerTools & tools)
     : root_lane{std::make_shared<Node>(geom::Rectangle{geom::Point{}, default_size})},
+      tools{tools},
       active_lane{root_lane},
       size{default_size}
 {
@@ -44,6 +45,7 @@ void WindowTree::confirm(miral::Window &window)
         geom::Rectangle{window.top_left(), window.size()},
         active_lane,
         window));
+    tools.select_active_window(window);
 }
 
 void WindowTree::toggle_resize_mode()
@@ -61,7 +63,7 @@ void WindowTree::toggle_resize_mode()
     is_resizing = true;
 }
 
-bool WindowTree::try_resize_active_window(miracle::WindowResizeDirection direction)
+bool WindowTree::try_resize_active_window(miracle::Direction direction)
 {
     if (!is_resizing)
         return false;
@@ -80,7 +82,7 @@ bool WindowTree::try_resize_active_window(miracle::WindowResizeDirection directi
 
 void WindowTree::resize_node_internal(
     std::shared_ptr<Node> node,
-    WindowResizeDirection direction,
+    Direction direction,
     int amount)
 {
     auto parent = node->parent;
@@ -90,16 +92,16 @@ void WindowTree::resize_node_internal(
         return;
     }
 
-    bool is_vertical = direction == WindowResizeDirection::up || direction == WindowResizeDirection::down;
-    bool is_main_axis_movement = (is_vertical  && parent->get_direction() == NodeDirection::vertical)
-        || (!is_vertical && parent->get_direction() == NodeDirection::horizontal);
+    bool is_vertical = direction == Direction::up || direction == Direction::down;
+    bool is_main_axis_movement = (is_vertical  && parent->get_direction() == NodeLayoutDirection::vertical)
+        || (!is_vertical && parent->get_direction() == NodeLayoutDirection::horizontal);
     if (!is_main_axis_movement)
     {
         resize_node_internal(parent, direction, amount);
         return;
     }
 
-    bool is_negative = direction == WindowResizeDirection::left || direction == WindowResizeDirection::up;
+    bool is_negative = direction == Direction::left || direction == Direction::up;
     auto resize_amount = is_negative ? -amount : amount;
     auto nodes = parent->get_sub_nodes();
     if (is_vertical)
@@ -144,19 +146,31 @@ void WindowTree::resize_node_internal(
     }
 }
 
+bool WindowTree::try_select_next(miracle::Direction direction)
+{
+    auto window_lane = active_lane->find_node_for_window(active_window);
+    if (!window_lane)
+        return false;
+    auto node = traverse(window_lane, direction);
+    if (!node)
+        return false;
+    tools.select_active_window(node->get_window());
+    return true;
+}
+
 void WindowTree::resize(geom::Size new_size)
 {
     size = new_size;
     // TODO: Resize all windows
 }
 
-bool WindowTree::try_move_active_window(miracle::WindowMoveDirection direction)
+bool WindowTree::try_move_active_window(miracle::Direction direction)
 {
     if (is_resizing)
         return false;
 
-    bool is_vertical = direction == WindowMoveDirection::up || direction == WindowMoveDirection::down;
-    bool is_negative = direction == WindowMoveDirection::up || direction == WindowMoveDirection::left;
+    bool is_vertical = direction == Direction::up || direction == Direction::down;
+    bool is_negative = direction == Direction::up || direction == Direction::left;
 
     int node_index = 0;
     for (; node_index < active_lane->get_sub_nodes().size(); node_index++)
@@ -227,15 +241,15 @@ bool WindowTree::try_move_active_window(miracle::WindowMoveDirection direction)
 
 void WindowTree::request_vertical()
 {
-    handle_direction_request(NodeDirection::vertical);
+    handle_direction_request(NodeLayoutDirection::vertical);
 }
 
 void WindowTree::request_horizontal()
 {
-    handle_direction_request(NodeDirection::horizontal);
+    handle_direction_request(NodeLayoutDirection::horizontal);
 }
 
-void WindowTree::handle_direction_request(NodeDirection direction)
+void WindowTree::handle_direction_request(NodeLayoutDirection direction)
 {
     if (is_resizing)
         return;
@@ -341,3 +355,71 @@ void WindowTree::advise_delete_window(miral::Window& window)
 
     active_lane->redistribute_size();
 }
+
+std::shared_ptr<Node> WindowTree::traverse(std::shared_ptr<Node> from, Direction direction)
+{
+    if (!from->parent)
+    {
+        std::cerr << "Cannot traverse the root node\n";
+        return nullptr;
+    }
+
+    auto parent = from->parent;
+    int index = parent->get_index_of_node(from);
+    auto parent_direction = parent->get_direction();
+
+    bool is_vertical = direction == Direction::up || direction == Direction::down;
+    bool is_negative = direction == Direction::up || direction == Direction::left;
+
+    if (is_vertical && parent_direction == NodeLayoutDirection::vertical
+        || !is_vertical && parent_direction == NodeLayoutDirection::horizontal)
+    {
+        // Simplest case: we're within a lane
+        if (is_negative)
+        {
+            if (index == 0)
+            {
+                // TODO: lazy lazy for readability
+                goto grandparent_route;
+            }
+            else
+                return parent->node_at(index - 1);
+        }
+        else
+        {
+            if (index == parent->num_nodes() - 1)
+            {
+                // TODO: lazy lazy for readability
+                goto grandparent_route;
+            }
+            else
+                return parent->node_at(index + 1);
+        }
+    }
+    else
+    {
+grandparent_route:
+        // Harder case: we need to jump to another lane. The best thing to do here is to
+        // find the first ancestor that matches the direction that we want to travel in.
+        // If  that ancestor cannot be found, then we throw up our hands.
+        auto grandparent = parent->parent;
+        if (!grandparent)
+        {
+            std::cerr << "Parent lane lacks a grandparent. It should AT LEAST be root\n";
+            return nullptr;
+        }
+
+        do {
+            if (grandparent->get_direction() == NodeLayoutDirection::horizontal && !is_vertical
+                || grandparent->get_direction() == NodeLayoutDirection::vertical && is_vertical)
+            {
+                return grandparent->find_first_window_child();
+            }
+
+            grandparent = grandparent->parent;
+        } while (grandparent != nullptr);
+    }
+
+
+    return nullptr;
+}

src/window_tree.h

@@ -24,13 +24,14 @@
 #include <miral/window_specification.h>
 #include <mir/geometry/size.h>
 #include <mir/geometry/rectangle.h>
+#include <miral/window_manager_tools.h>
 
 namespace geom = mir::geometry;
 
 namespace miracle
 {
 
-enum class WindowResizeDirection
+enum class Direction
 {
     up,
     left,
@@ -38,19 +39,12 @@ enum class WindowResizeDirection
     right
 };
 
-enum class WindowMoveDirection
-{
-    up,
-    left,
-    down,
-    right
-};
 
 /// Represents a tiling tree for an output.
 class WindowTree
 {
 public:
-    WindowTree(geom::Size default_size);
+    WindowTree(geom::Size default_size, const miral::WindowManagerTools & tools);
     ~WindowTree() = default;
 
     /// Makes space for the new window and returns its specified spot in the world.
@@ -60,10 +54,11 @@ public:
     void confirm(miral::Window&);
 
     void toggle_resize_mode();
-    bool try_resize_active_window(WindowResizeDirection direction);
+    bool try_resize_active_window(Direction direction);
     void resize(geom::Size new_size);
 
-    bool try_move_active_window(WindowMoveDirection direction);
+    bool try_move_active_window(Direction direction);
+    bool try_select_next(Direction direction);
 
     // Request a change to vertical window placement
     void request_vertical();
@@ -76,14 +71,18 @@ public:
     void advise_delete_window(miral::Window&);
 
 private:
-    void handle_direction_request(NodeDirection direction);
+    miral::WindowManagerTools tools;
     std::shared_ptr<Node> root_lane;
     std::shared_ptr<Node> active_lane;
     miral::Window active_window;
     geom::Size size;
     bool is_resizing = false;
 
-    void resize_node_internal(std::shared_ptr<Node> node, WindowResizeDirection direction, int amount);
+    void handle_direction_request(NodeLayoutDirection direction);
+    void resize_node_internal(std::shared_ptr<Node> node, Direction direction, int amount);
+    /// From the provided node, find the next node in the provided direction.
+    /// This method is guaranteed to return a Window node, not a Lane.
+    std::shared_ptr<Node> traverse(std::shared_ptr<Node> from, Direction direction);
 };
 
 }