Merge pull request #87 from pinnacle-comp/fix_stuff

Update keyboard focus on tag change and position Wayland popups correctly

Cargo.toml

@@ -10,8 +10,9 @@ repository = "https://github.com/pinnacle-comp/pinnacle/"
 keywords = ["wayland", "compositor", "smithay", "lua"]
 
 [dependencies]
-tracing = "0.1.37"
-tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "b8c45cc" }
+tracing-subscriber = { git = "https://github.com/tokio-rs/tracing", rev = "b8c45cc", features = ["env-filter"] }
+tracing-appender = { git = "https://github.com/tokio-rs/tracing", rev = "b8c45cc" }
 smithay = { git = "https://github.com/Smithay/smithay", rev = "1a61e1c", features = ["desktop", "wayland_frontend"] }
 smithay-drm-extras = { git = "https://github.com/Smithay/smithay", optional = true }
 thiserror = "1.0.48"
@@ -31,7 +32,6 @@ clap = { version = "4.4.2", features = ["derive"] }
 xkbcommon = "0.6.0"
 xdg = "2.5.2"
 lazy_static = "1.4.0"
-tracing-appender = "0.2.2"
 walkdir = "2.4.0"
 sysinfo = "0.29.10"
 

src/backend/udev.rs

@@ -1452,15 +1452,21 @@ fn render_surface<'a>(
     pointer_location: Point<f64, Logical>,
     clock: &Clock<Monotonic>,
 ) -> Result<bool, SwapBuffersError> {
-    let pending_win_count = windows
+    let pending_wins = windows
         .iter()
         .filter(|win| win.alive())
         .filter(|win| win.with_state(|state| !state.loc_request_state.is_idle()))
-        .count() as u32;
+        .map(|win| {
+            (
+                win.class().unwrap_or("None".to_string()),
+                win.title().unwrap_or("None".to_string()),
+                win.with_state(|state| state.loc_request_state.clone()),
+            )
+        })
+        .collect::<Vec<_>>();
 
-    tracing::debug!("pending_win_count is {pending_win_count}");
-
-    if pending_win_count > 0 {
+    if !pending_wins.is_empty() {
+        tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
         for win in windows.iter() {
             win.send_frame(output, clock.now(), Some(Duration::ZERO), |_, _| {
                 Some(output.clone())

src/backend/winit.rs

@@ -235,18 +235,22 @@ pub fn run_winit() -> anyhow::Result<()> {
 
                 pointer_element.set_status(state.cursor_status.clone());
 
-                // TODO: make a pending_windows state, when pending_windows increases,
-                // |     pause rendering.
-                // |     If it goes down, push a frame, then repeat until no pending_windows are left.
-
-                let pending_win_count = state
+                let pending_wins = state
                     .windows
                     .iter()
                     .filter(|win| win.alive())
                     .filter(|win| win.with_state(|state| !state.loc_request_state.is_idle()))
-                    .count() as u32;
+                    .map(|win| {
+                        (
+                            win.class().unwrap_or("None".to_string()),
+                            win.title().unwrap_or("None".to_string()),
+                            win.with_state(|state| state.loc_request_state.clone()),
+                        )
+                    })
+                    .collect::<Vec<_>>();
 
-                if pending_win_count > 0 {
+                if !pending_wins.is_empty() {
+                    tracing::debug!("Skipping frame, waiting on {pending_wins:?}");
                     for win in state.windows.iter() {
                         win.send_frame(
                             &output,

src/focus.rs

@@ -9,7 +9,7 @@ use smithay::{
     },
     output::Output,
     reexports::wayland_server::{protocol::wl_surface::WlSurface, Resource},
-    utils::IsAlive,
+    utils::{IsAlive, SERIAL_COUNTER},
     wayland::seat::WaylandFocus,
 };
 
@@ -26,7 +26,7 @@ pub struct FocusState {
 
 impl State {
     /// Get the currently focused window on `output`, if any.
-    pub fn current_focus(&mut self, output: &Output) -> Option<WindowElement> {
+    pub fn focused_window(&mut self, output: &Output) -> Option<WindowElement> {
         self.focus_state.focus_stack.retain(|win| win.alive());
 
         let mut windows = self.focus_state.focus_stack.iter().rev().filter(|win| {
@@ -41,6 +41,27 @@ impl State {
 
         windows.next().cloned()
     }
+
+    /// Update the focus. This will raise the current focus and activate it,
+    /// as well as setting the keyboard focus to it.
+    pub fn update_focus(&mut self, output: &Output) {
+        let current_focus = self.focused_window(output);
+
+        if let Some(win) = &current_focus {
+            self.space.raise_element(win, true);
+            if let WindowElement::Wayland(w) = win {
+                w.toplevel().send_configure();
+            }
+        }
+
+        self.seat.get_keyboard().expect("no keyboard").set_focus(
+            self,
+            current_focus.map(|win| win.into()),
+            SERIAL_COUNTER.next_serial(),
+        );
+
+        // TODO: if there already is a visible focused window, don't do anything
+    }
 }
 
 impl FocusState {

src/handlers/xdg_shell.rs

@@ -7,13 +7,16 @@ use smithay::{
     input::{pointer::Focus, Seat},
     output::Output,
     reexports::{
-        wayland_protocols::xdg::shell::server::xdg_toplevel::{self, ResizeEdge},
+        wayland_protocols::xdg::shell::server::{
+            xdg_positioner::{Anchor, ConstraintAdjustment, Gravity},
+            xdg_toplevel::{self, ResizeEdge},
+        },
         wayland_server::{
             protocol::{wl_output::WlOutput, wl_seat::WlSeat, wl_surface::WlSurface},
             Resource,
         },
     },
-    utils::{Serial, SERIAL_COUNTER},
+    utils::{Logical, Point, Rectangle, Serial, SERIAL_COUNTER},
     wayland::{
         compositor::{self},
         shell::xdg::{
@@ -133,7 +136,7 @@ impl XdgShellHandler for State {
 
         if let Some(output) = window.output(self) {
             self.update_windows(&output);
-            let focus = self.current_focus(&output).map(FocusTarget::Window);
+            let focus = self.focused_window(&output).map(FocusTarget::Window);
             if let Some(FocusTarget::Window(win)) = &focus {
                 tracing::debug!("Focusing on prev win");
                 self.space.raise_element(win, true);
@@ -148,7 +151,493 @@ impl XdgShellHandler for State {
         }
     }
 
-    fn new_popup(&mut self, surface: PopupSurface, _positioner: PositionerState) {
+    // this is 500 lines there has to be a shorter way to do this
+    fn new_popup(&mut self, surface: PopupSurface, mut positioner: PositionerState) {
+        tracing::debug!(?positioner.constraint_adjustment, ?positioner.gravity);
+        let output_rect = self
+            .focus_state
+            .focused_output
+            .as_ref()
+            .or_else(|| self.space.outputs().next())
+            .and_then(|op| self.space.output_geometry(op));
+
+        /// Horizontal direction
+        #[derive(Clone, Copy, Debug, PartialEq, Eq)]
+        enum DirH {
+            Left,
+            Right,
+        }
+
+        /// Vertical direction
+        #[derive(Clone, Copy, Debug, PartialEq, Eq)]
+        enum DirV {
+            Top,
+            Bottom,
+        }
+
+        let mut is_subpopup = false;
+
+        // We have to go through this really verbose way of getting the location of the popup in
+        // the global space.
+        //
+        // The location from PopupSurface.with_pending_state's state.geometry.loc is relative to
+        // its parent. When its parent is a window, we can simply add the window's location to the
+        // popup's to get its global location.
+        //
+        // However, if its parent is another popup, we need to step upwards and grab the location
+        // from each popup, aggregating them into one global space location.
+        let global_loc = {
+            let mut surf = surface.get_parent_surface();
+            let mut loc = surface.with_pending_state(|state| state.geometry.loc);
+            tracing::debug!(?loc);
+
+            while let Some(s) = &surf {
+                if let Some(popup) = self.popup_manager.find_popup(s) {
+                    // popup.geometry() doesn't return the right location, so we dive into the
+                    // PopupSurface's state to grab it.
+                    let PopupKind::Xdg(popup_surf) = &popup;
+                    let l = popup_surf.with_pending_state(|state| state.geometry.loc);
+                    tracing::debug!(loc = ?l, "parent is popup");
+                    loc += l;
+                    is_subpopup = true;
+                    surf = popup_surf.get_parent_surface();
+                } else if let Some(win) = self.window_for_surface(s) {
+                    // Once we reach a window, we can stop as windows are already in the global space.
+                    tracing::debug!("parent is window");
+                    loc += self
+                        .space
+                        .element_location(&win)
+                        .unwrap_or_else(|| (0, 0).into());
+                    tracing::debug!(?loc);
+                    break;
+                }
+            }
+
+            loc
+        };
+
+        let mut popup_rect_global = {
+            let mut rect = positioner.get_geometry();
+            rect.loc = global_loc;
+            rect
+        };
+
+        tracing::debug!(?global_loc);
+
+        // The final rectangle that the popup is set to needs to be relative to its parent,
+        // so we store its old location here and subtract it at the end.
+        let loc_diff = global_loc - surface.with_pending_state(|state| state.geometry.loc);
+
+        if let Some(output_rect) = output_rect {
+            // Check if the rect is constrained horizontally, and if so, which side.
+            let constrained_x = |rect: Rectangle<i32, Logical>| -> Option<DirH> {
+                tracing::debug!(?rect, ?output_rect);
+
+                if rect.loc.x < output_rect.loc.x {
+                    Some(DirH::Left)
+                } else if rect.loc.x + rect.size.w > output_rect.loc.x + output_rect.size.w {
+                    Some(DirH::Right)
+                } else {
+                    None
+                }
+            };
+
+            // Check if the rect is constrained vertically, and if so, which side.
+            let constrained_y = |rect: Rectangle<i32, Logical>| -> Option<DirV> {
+                tracing::debug!(?rect, ?output_rect);
+
+                if rect.loc.y < output_rect.loc.y {
+                    Some(DirV::Top)
+                } else if rect.loc.y + rect.size.h > output_rect.loc.y + output_rect.size.h {
+                    Some(DirV::Bottom)
+                } else {
+                    None
+                }
+            };
+
+            // We're going to need to position popups such that they stay fully onscreen.
+            // We can use the provided `positioner.constraint_adjustment` to get hints on how
+            // the popups want to be relocated.
+
+            let output_left_x = output_rect.loc.x;
+            let output_right_x = output_rect.loc.x + output_rect.size.w;
+            let output_top_y = output_rect.loc.y;
+            let output_bottom_y = output_rect.loc.y + output_rect.size.h;
+
+            // The popup is flowing offscreen in the horizontal direction.
+            if let Some(constrain_dir) = constrained_x(popup_rect_global) {
+                tracing::debug!("Popup was constrained on the x axis, repositioning");
+                let gravity = match positioner.gravity {
+                    Gravity::Left | Gravity::TopLeft | Gravity::BottomLeft => DirH::Left,
+                    _ => DirH::Right,
+                };
+
+                tracing::debug!(?gravity);
+                tracing::debug!(?constrain_dir);
+
+                'block: {
+                    // If the constraint_adjustment has SlideX, we attempt to slide the popup
+                    // towards the direction specified by positioner.gravity until  TODO:
+                    if positioner
+                        .constraint_adjustment
+                        .contains(ConstraintAdjustment::SlideX)
+                        && !is_subpopup
+                    // If it's a subpopup, flip instead of slide. This makes
+                    // stuff like Firefox nested dropdowns more intuitive.
+                    {
+                        tracing::debug!("Attempting to slide popup X");
+                        // Slide towards the gravity until the opposite edge is unconstrained or the
+                        // same edge is constrained
+                        match (&constrain_dir, &gravity) {
+                            (DirH::Left, DirH::Right) => {
+                                let len_until_same_edge_constrained = output_right_x
+                                    - (popup_rect_global.loc.x + popup_rect_global.size.w);
+                                let len_until_opp_edge_unconstrained =
+                                    output_left_x - popup_rect_global.loc.x;
+
+                                popup_rect_global.loc.x += i32::min(
+                                    len_until_same_edge_constrained,
+                                    len_until_opp_edge_unconstrained,
+                                )
+                                .max(0);
+                                tracing::debug!(
+                                    ?popup_rect_global,
+                                    "Constrained SlideX left right"
+                                );
+                            }
+                            (DirH::Right, DirH::Left) => {
+                                let len_until_same_edge_constrained =
+                                    popup_rect_global.loc.x - output_left_x;
+                                let len_until_opp_edge_unconstrained = (popup_rect_global.loc.x
+                                    + popup_rect_global.size.w)
+                                    - output_right_x;
+
+                                popup_rect_global.loc.x -= i32::min(
+                                    len_until_opp_edge_unconstrained,
+                                    len_until_same_edge_constrained,
+                                )
+                                .max(0);
+                                tracing::debug!(
+                                    ?popup_rect_global,
+                                    "Constrained SlideX right left"
+                                );
+                            }
+                            _ => (),
+                        };
+
+                        if constrained_x(popup_rect_global).is_none() {
+                            break 'block;
+                        }
+
+                        // Do the same but in the other direction
+                        match (constrain_dir, gravity) {
+                            (DirH::Right, DirH::Right) => {
+                                let len_until_same_edge_unconstrained =
+                                    popup_rect_global.loc.x - output_left_x;
+                                let len_until_opp_edge_constrained = (popup_rect_global.loc.x
+                                    + popup_rect_global.size.w)
+                                    - output_right_x;
+
+                                popup_rect_global.loc.x -= i32::min(
+                                    len_until_opp_edge_constrained,
+                                    len_until_same_edge_unconstrained,
+                                )
+                                .max(0);
+                                tracing::debug!(
+                                    ?popup_rect_global,
+                                    "Constrained SlideX right right"
+                                );
+                            }
+                            (DirH::Left, DirH::Left) => {
+                                let len_until_same_edge_unconstrained = output_right_x
+                                    - (popup_rect_global.loc.x + popup_rect_global.size.w);
+                                let len_until_opp_edge_constrained =
+                                    output_left_x - popup_rect_global.loc.x;
+
+                                popup_rect_global.loc.x += i32::min(
+                                    len_until_same_edge_unconstrained,
+                                    len_until_opp_edge_constrained,
+                                )
+                                .max(0);
+                                tracing::debug!(?popup_rect_global, "Constrained SlideX left left");
+                            }
+                            _ => (),
+                        };
+
+                        if constrained_x(popup_rect_global).is_none() {
+                            break 'block;
+                        }
+                    }
+
+                    // If the above didn't bring the popup onscreen or if it's a nested popup, flip it.
+                    if positioner
+                        .constraint_adjustment
+                        .contains(ConstraintAdjustment::FlipX)
+                    {
+                        tracing::debug!("Attempting to flip popup X");
+                        let old_gravity = positioner.gravity;
+                        positioner.gravity = match positioner.gravity {
+                            Gravity::Left => Gravity::Right,
+                            Gravity::Right => Gravity::Left,
+                            Gravity::TopLeft => Gravity::TopRight,
+                            Gravity::BottomLeft => Gravity::BottomRight,
+                            Gravity::TopRight => Gravity::TopLeft,
+                            Gravity::BottomRight => Gravity::BottomLeft,
+                            rest => rest,
+                        };
+
+                        let old_anchor = positioner.anchor_edges;
+                        positioner.anchor_edges = match positioner.anchor_edges {
+                            Anchor::Left => Anchor::Right,
+                            Anchor::Right => Anchor::Left,
+                            Anchor::TopLeft => Anchor::TopRight,
+                            Anchor::BottomLeft => Anchor::BottomRight,
+                            Anchor::TopRight => Anchor::TopLeft,
+                            Anchor::BottomRight => Anchor::BottomLeft,
+                            rest => rest,
+                        };
+
+                        let mut relative_geo = positioner.get_geometry();
+                        relative_geo.loc += loc_diff;
+                        tracing::debug!(?relative_geo, "FlipX");
+                        if constrained_x(relative_geo).is_none() {
+                            popup_rect_global = relative_geo;
+                            break 'block;
+                        }
+
+                        // The protocol states that if flipping it didn't bring it onscreen,
+                        // then it should just stay at its unflipped state.
+                        positioner.gravity = old_gravity;
+                        positioner.anchor_edges = old_anchor;
+                    }
+
+                    // Finally, if flipping it failed, resize it to fit.
+                    if positioner
+                        .constraint_adjustment
+                        .contains(ConstraintAdjustment::ResizeX)
+                    {
+                        tracing::debug!("Resizing popup X");
+                        // Slice off the left side
+                        if popup_rect_global.loc.x < output_left_x {
+                            let len_to_slice = output_left_x - popup_rect_global.loc.x;
+                            let new_top_left: Point<i32, Logical> = (
+                                popup_rect_global.loc.x + len_to_slice,
+                                popup_rect_global.loc.y,
+                            )
+                                .into();
+                            let bottom_right: Point<i32, Logical> = (
+                                popup_rect_global.loc.x + popup_rect_global.size.w,
+                                popup_rect_global.loc.y + popup_rect_global.size.h,
+                            )
+                                .into();
+                            popup_rect_global =
+                                Rectangle::from_extemities(new_top_left, bottom_right);
+                        }
+
+                        // Slice off the right side
+                        if popup_rect_global.loc.x + popup_rect_global.size.w > output_right_x {
+                            let len_to_slice = (popup_rect_global.loc.x + popup_rect_global.size.w)
+                                - output_right_x;
+                            let top_left = popup_rect_global.loc;
+                            let new_bottom_right: Point<i32, Logical> = (
+                                popup_rect_global.loc.x + popup_rect_global.size.w - len_to_slice,
+                                popup_rect_global.loc.y + popup_rect_global.size.h,
+                            )
+                                .into();
+                            popup_rect_global =
+                                Rectangle::from_extemities(top_left, new_bottom_right);
+                        }
+
+                        if constrained_x(popup_rect_global).is_none() {
+                            break 'block;
+                        }
+                    }
+                }
+            }
+
+            // The popup is flowing offscreen in the vertical direction.
+            if let Some(constrain_dir) = constrained_y(popup_rect_global) {
+                tracing::debug!("Popup was constrained on the y axis, repositioning");
+                let gravity = match positioner.gravity {
+                    Gravity::Top | Gravity::TopLeft | Gravity::TopRight => DirV::Top,
+                    _ => DirV::Bottom,
+                };
+
+                tracing::debug!(?gravity);
+                tracing::debug!(?constrain_dir);
+
+                'block: {
+                    // If the constraint_adjustment has SlideY, we attempt to slide the popup
+                    // towards the direction specified by positioner.gravity until  TODO:
+                    if positioner
+                        .constraint_adjustment
+                        .contains(ConstraintAdjustment::SlideY)
+                        && !is_subpopup
+                    // If it's a subpopup, flip instead of slide. This makes
+                    // stuff like Firefox nested dropdowns more intuitive.
+                    {
+                        // Slide towards the gravity until the opposite edge is unconstrained or the
+                        // same edge is constrained
+                        match (&constrain_dir, &gravity) {
+                            (DirV::Top, DirV::Bottom) => {
+                                let len_until_same_edge_constrained = output_bottom_y
+                                    - (popup_rect_global.loc.y + popup_rect_global.size.h);
+                                let len_until_opp_edge_unconstrained =
+                                    output_top_y - popup_rect_global.loc.y;
+
+                                popup_rect_global.loc.y += i32::min(
+                                    len_until_same_edge_constrained,
+                                    len_until_opp_edge_unconstrained,
+                                )
+                                .max(0);
+                            }
+                            (DirV::Bottom, DirV::Top) => {
+                                let len_until_same_edge_constrained =
+                                    popup_rect_global.loc.y - output_top_y;
+                                let len_until_opp_edge_unconstrained = (popup_rect_global.loc.y
+                                    + popup_rect_global.size.h)
+                                    - output_bottom_y;
+
+                                popup_rect_global.loc.y -= i32::min(
+                                    len_until_opp_edge_unconstrained,
+                                    len_until_same_edge_constrained,
+                                )
+                                .max(0);
+                            }
+                            _ => (),
+                        };
+
+                        if constrained_y(popup_rect_global).is_none() {
+                            break 'block;
+                        }
+
+                        // Do the same but in the other direction
+                        match (constrain_dir, gravity) {
+                            (DirV::Bottom, DirV::Bottom) => {
+                                let len_until_same_edge_unconstrained =
+                                    popup_rect_global.loc.y - output_top_y;
+                                let len_until_opp_edge_constrained = (popup_rect_global.loc.y
+                                    + popup_rect_global.size.h)
+                                    - output_bottom_y;
+
+                                popup_rect_global.loc.y -= i32::min(
+                                    len_until_opp_edge_constrained,
+                                    len_until_same_edge_unconstrained,
+                                )
+                                .max(0);
+                            }
+                            (DirV::Top, DirV::Top) => {
+                                let len_until_same_edge_unconstrained = output_bottom_y
+                                    - (popup_rect_global.loc.y + popup_rect_global.size.h);
+                                let len_until_opp_edge_constrained =
+                                    output_top_y - popup_rect_global.loc.y;
+
+                                popup_rect_global.loc.y += i32::min(
+                                    len_until_same_edge_unconstrained,
+                                    len_until_opp_edge_constrained,
+                                )
+                                .max(0);
+                            }
+                            _ => (),
+                        };
+
+                        if constrained_y(popup_rect_global).is_none() {
+                            break 'block;
+                        }
+                    }
+
+                    // If the above didn't bring the popup onscreen or if it's a nested popup, flip it.
+                    if positioner
+                        .constraint_adjustment
+                        .contains(ConstraintAdjustment::FlipY)
+                    {
+                        let old_gravity = positioner.gravity;
+                        positioner.gravity = match positioner.gravity {
+                            Gravity::Top => Gravity::Bottom,
+                            Gravity::Bottom => Gravity::Top,
+                            Gravity::TopLeft => Gravity::BottomLeft,
+                            Gravity::BottomLeft => Gravity::TopLeft,
+                            Gravity::TopRight => Gravity::BottomRight,
+                            Gravity::BottomRight => Gravity::TopRight,
+                            rest => rest,
+                        };
+
+                        let old_anchor = positioner.anchor_edges;
+                        positioner.anchor_edges = match positioner.anchor_edges {
+                            Anchor::Top => Anchor::Bottom,
+                            Anchor::Bottom => Anchor::Top,
+                            Anchor::TopLeft => Anchor::BottomLeft,
+                            Anchor::BottomLeft => Anchor::TopLeft,
+                            Anchor::TopRight => Anchor::BottomRight,
+                            Anchor::BottomRight => Anchor::TopRight,
+                            rest => rest,
+                        };
+
+                        let mut geo = positioner.get_geometry();
+                        tracing::debug!(?geo, "Flipped Y geo");
+                        geo.loc += loc_diff;
+                        geo.loc.x = popup_rect_global.loc.x;
+                        tracing::debug!(?geo, "Flipped Y geo global");
+                        if constrained_y(geo).is_none() {
+                            popup_rect_global = geo;
+                            break 'block;
+                        }
+
+                        // The protocol states that if flipping it didn't bring it onscreen,
+                        // then it should just stay at its unflipped state.
+                        positioner.gravity = old_gravity;
+                        positioner.anchor_edges = old_anchor;
+                    }
+
+                    // Finally, if flipping it failed, resize it to fit.
+                    if positioner
+                        .constraint_adjustment
+                        .contains(ConstraintAdjustment::ResizeY)
+                    {
+                        // Slice off the top side
+                        if popup_rect_global.loc.y < output_top_y {
+                            let len_to_slice = output_top_y - popup_rect_global.loc.y;
+                            let new_top_left: Point<i32, Logical> = (
+                                popup_rect_global.loc.x,
+                                popup_rect_global.loc.y + len_to_slice,
+                            )
+                                .into();
+                            let bottom_right: Point<i32, Logical> = (
+                                popup_rect_global.loc.x + popup_rect_global.size.w,
+                                popup_rect_global.loc.y + popup_rect_global.size.h,
+                            )
+                                .into();
+                            popup_rect_global =
+                                Rectangle::from_extemities(new_top_left, bottom_right);
+                        }
+
+                        // Slice off the right side
+                        if popup_rect_global.loc.y + popup_rect_global.size.h > output_bottom_y {
+                            let len_to_slice = (popup_rect_global.loc.y + popup_rect_global.size.h)
+                                - output_bottom_y;
+                            let top_left = popup_rect_global.loc;
+                            let new_bottom_right: Point<i32, Logical> = (
+                                popup_rect_global.loc.x + popup_rect_global.size.w,
+                                popup_rect_global.loc.y + popup_rect_global.size.h - len_to_slice,
+                            )
+                                .into();
+                            popup_rect_global =
+                                Rectangle::from_extemities(top_left, new_bottom_right);
+                        }
+
+                        if constrained_y(popup_rect_global).is_none() {
+                            break 'block;
+                        }
+                    }
+                }
+            }
+        }
+
+        tracing::debug!(?popup_rect_global, "New popup");
+        popup_rect_global.loc -= loc_diff;
+
+        surface.with_pending_state(|state| state.geometry = popup_rect_global);
+
         if let Err(err) = self.popup_manager.track_popup(PopupKind::from(surface)) {
             tracing::warn!("failed to track popup: {}", err);
         }
@@ -190,6 +679,8 @@ impl XdgShellHandler for State {
         positioner: PositionerState,
         token: u32,
     ) {
+        // TODO: reposition logic
+
         surface.with_pending_state(|state| {
             state.geometry = positioner.get_geometry();
             state.positioner = positioner;

src/handlers/xwayland.rs

@@ -200,7 +200,7 @@ impl XwmHandler for CalloopData {
             self.state.space.unmap_elem(&win);
             if let Some(output) = win.output(&self.state) {
                 self.state.update_windows(&output);
-                let focus = self.state.current_focus(&output).map(FocusTarget::Window);
+                let focus = self.state.focused_window(&output).map(FocusTarget::Window);
                 if let Some(FocusTarget::Window(win)) = &focus {
                     tracing::debug!("Focusing on prev win");
                     self.state.space.raise_element(win, true);
@@ -243,7 +243,7 @@ impl XwmHandler for CalloopData {
 
             if let Some(output) = win.output(&self.state) {
                 self.state.update_windows(&output);
-                let focus = self.state.current_focus(&output).map(FocusTarget::Window);
+                let focus = self.state.focused_window(&output).map(FocusTarget::Window);
                 if let Some(FocusTarget::Window(win)) = &focus {
                     tracing::debug!("Focusing on prev win");
                     self.state.space.raise_element(win, true);

src/input.rs

@@ -19,7 +19,7 @@ use smithay::{
     desktop::{layer_map_for_output, space::SpaceElement},
     input::{
         keyboard::{keysyms, FilterResult},
-        pointer::{AxisFrame, ButtonEvent, MotionEvent},
+        pointer::{AxisFrame, ButtonEvent, MotionEvent, RelativeMotionEvent},
     },
     utils::{Logical, Point, SERIAL_COUNTER},
     wayland::{seat::WaylandFocus, shell::wlr_layer},
@@ -526,7 +526,7 @@ impl State {
         if let Some(ptr) = self.seat.get_pointer() {
             ptr.motion(
                 self,
-                surface_under,
+                surface_under.clone(),
                 &MotionEvent {
                     location: self.pointer_location,
                     serial,
@@ -534,15 +534,15 @@ impl State {
                 },
             );
 
-            // ptr.relative_motion(
-            //     self,
-            //     under,
-            //     &RelativeMotionEvent {
-            //         delta: event.delta(),
-            //         delta_unaccel: event.delta_unaccel(),
-            //         utime: event.time(),
-            //     },
-            // )
+            ptr.relative_motion(
+                self,
+                surface_under,
+                &RelativeMotionEvent {
+                    delta: event.delta(),
+                    delta_unaccel: event.delta_unaccel(),
+                    utime: event.time(),
+                },
+            )
         }
     }
 }

src/layout.rs

@@ -53,6 +53,7 @@ impl State {
     /// Compute tiled window locations and sizes, size maximized and fullscreen windows correctly,
     /// and send configures and that cool stuff.
     pub fn update_windows(&mut self, output: &Output) {
+        tracing::debug!("Updating windows");
         let Some(layout) = output.with_state(|state| {
             state.focused_tags().next().map(|tag| tag.layout())
         }) else { return };
@@ -128,11 +129,12 @@ impl State {
                             // (like tiled states) but not commit, so we check for just the size
                             // here
                             if !is_pending {
-                                tracing::debug!("No pending changes");
+                                tracing::debug!("No pending changes, mapping window");
+                                // TODO: map win here, not down there
                                 state.loc_request_state = LocationRequestState::Idle;
                                 non_pending_wins.push((loc, window.clone()));
                             } else {
-                                tracing::debug!("Pending changes");
+                                tracing::debug!("Pending changes, requesting commit");
                                 let serial = win.toplevel().send_configure();
                                 state.loc_request_state =
                                     LocationRequestState::Requested(serial, loc);
@@ -154,14 +156,6 @@ impl State {
             });
         }
 
-        // Pause rendering. Here we'll wait until all windows have ack'ed and committed,
-        // then resume rendering. This prevents flickering because some windows will commit before
-        // others.
-        //
-        // This *will* cause everything to freeze for a few frames, but it shouldn't impact
-        // anything meaningfully.
-        // self.pause_rendering = true;
-
         // schedule on all idle
         self.schedule(
             move |_dt| {
@@ -176,7 +170,6 @@ impl State {
                 for (loc, win) in non_pending_wins {
                     dt.state.space.map_element(win, loc, false);
                 }
-                // dt.state.pause_rendering = false;
             },
         );
     }
@@ -486,49 +479,14 @@ fn corner(layout: &Layout, windows: Vec<WindowElement>, rect: Rectangle<i32, Log
 
 impl State {
     pub fn swap_window_positions(&mut self, win1: &WindowElement, win2: &WindowElement) {
-        let mut elems = self
-            .windows
-            .iter_mut()
-            .filter(|win| *win == win1 || *win == win2);
+        let win1_index = self.windows.iter().position(|win| win == win1);
+        let win2_index = self.windows.iter().position(|win| win == win2);
 
-        let (first, second) = (elems.next(), elems.next());
-
-        if let Some(first) = first {
-            if let Some(second) = second {
-                std::mem::swap(first, second);
+        if let (Some(first), Some(second)) = (win1_index, win2_index) {
+            self.windows.swap(first, second);
+            if let Some(output) = win1.output(self) {
+                self.update_windows(&output);
             }
         }
-
-        let mut same_suggested_size = false;
-
-        if let WindowElement::Wayland(w1) = win1 {
-            if let WindowElement::Wayland(w2) = win2 {
-                if let Some(w1_size) = w1.toplevel().current_state().size {
-                    if let Some(w2_size) = w2.toplevel().current_state().size {
-                        same_suggested_size = w1_size == w2_size;
-                    }
-                }
-            }
-        }
-
-        if same_suggested_size {
-            let win1_loc = self.space.element_location(win1);
-            let win2_loc = self.space.element_location(win2);
-
-            if let Some(win1_loc) = win1_loc {
-                if let Some(win2_loc) = win2_loc {
-                    self.space.map_element(win1.clone(), win2_loc, false);
-                    self.space.map_element(win2.clone(), win1_loc, false);
-                }
-            }
-        } else {
-            // TODO: don't use the focused output, use the outputs the two windows are on
-            let output = self
-                .focus_state
-                .focused_output
-                .clone()
-                .expect("no focused output");
-            self.update_windows(&output);
-        }
     }
 }

src/main.rs

@@ -11,11 +11,9 @@
 // #![deny(unused_imports)] // gonna force myself to keep stuff clean
 #![warn(clippy::unwrap_used)]
 
-use std::path::Path;
-
 use clap::Parser;
+use tracing_appender::rolling::Rotation;
 use tracing_subscriber::{fmt::writer::MakeWriterExt, EnvFilter};
-use walkdir::WalkDir;
 use xdg::BaseDirectories;
 
 mod api;
@@ -63,14 +61,16 @@ struct Args {
     force: bool,
 }
 
-const PINNACLE_LOG_PREFIX: &str = "pinnacle.log";
-
 fn main() -> anyhow::Result<()> {
     let xdg_state_dir = XDG_BASE_DIRS.get_state_home();
 
-    trim_logs(&xdg_state_dir);
+    let appender = tracing_appender::rolling::Builder::new()
+        .rotation(Rotation::HOURLY)
+        .filename_suffix("pinnacle.log")
+        .max_log_files(8)
+        .build(xdg_state_dir)
+        .expect("failed to build file logger");
 
-    let appender = tracing_appender::rolling::hourly(&xdg_state_dir, PINNACLE_LOG_PREFIX);
     let (appender, _guard) = tracing_appender::non_blocking(appender);
     let writer = appender.and(std::io::stdout);
 
@@ -140,41 +140,3 @@ fn main() -> anyhow::Result<()> {
 
     Ok(())
 }
-
-fn trim_logs(log_path: impl AsRef<Path>) {
-    let logs = WalkDir::new(log_path)
-        .sort_by(|a, b| {
-            let a_creation_time = a
-                .metadata()
-                .expect("failed to get log metadata")
-                .created()
-                .expect("failed to get log creation time");
-            let b_creation_time = b
-                .metadata()
-                .expect("failed to get log metadata")
-                .created()
-                .expect("failed to get log creation time");
-
-            a_creation_time.cmp(&b_creation_time)
-        })
-        .contents_first(true)
-        .into_iter()
-        .filter_entry(|entry| {
-            entry.file_type().is_file()
-                && entry
-                    .file_name()
-                    .to_string_lossy()
-                    .starts_with(PINNACLE_LOG_PREFIX)
-        })
-        .filter_map(|dir| dir.ok())
-        .collect::<Vec<_>>();
-
-    // If there are more than 4 logs, delete all but 3
-    if logs.len() > 4 {
-        let num_to_delete = logs.len().saturating_sub(3);
-        for entry in logs.into_iter().take(num_to_delete) {
-            tracing::info!("Deleting {:?}", entry.path());
-            std::fs::remove_file(entry.path()).expect("failed to remove oldest log file");
-        }
-    }
-}

src/state/api_handlers.rs

@@ -237,7 +237,7 @@ impl State {
                     tag.set_active(!tag.active());
                     if let Some(output) = tag.output(self) {
                         self.update_windows(&output);
-                        // self.re_layout(&output);
+                        self.update_focus(&output);
                     }
                 }
             }
@@ -251,7 +251,7 @@ impl State {
                     tag.set_active(true);
                 });
                 self.update_windows(&output);
-                // self.re_layout(&output);
+                self.update_focus(&output);
             }
             Msg::AddTags {
                 output_name,
@@ -297,7 +297,6 @@ impl State {
                 tag.set_layout(layout);
                 let Some(output) = tag.output(self) else { return };
                 self.update_windows(&output);
-                // self.re_layout(&output);
             }
 
             Msg::ConnectForAllOutputs { callback_id } => {
@@ -305,8 +304,8 @@ impl State {
                     .api_state
                     .stream
                     .as_ref()
-                    .expect("Stream doesn't exist");
-                let mut stream = stream.lock().expect("Couldn't lock stream");
+                    .expect("stream doesn't exist");
+                let mut stream = stream.lock().expect("couldn't lock stream");
                 for output in self.space.outputs() {
                     crate::api::send_to_client(
                         &mut stream,
@@ -332,12 +331,12 @@ impl State {
                 }
                 output.change_current_state(None, None, None, Some(loc));
                 self.space.map_output(&output, loc);
-                tracing::debug!("mapping output {} to {loc:?}", output.name());
+                tracing::debug!("Mapping output {} to {loc:?}", output.name());
                 self.update_windows(&output);
-                // self.re_layout(&output);
             }
 
             Msg::Quit => {
+                tracing::info!("Quitting Pinnacle");
                 self.loop_signal.stop();
             }
 
@@ -405,7 +404,7 @@ impl State {
                 });
                 let focused = window.as_ref().and_then(|win| {
                     let output = win.output(self)?;
-                    self.current_focus(&output).map(|foc_win| win == &foc_win)
+                    self.focused_window(&output).map(|foc_win| win == &foc_win)
                 });
                 let floating = window
                     .as_ref()

src/window.rs

@@ -242,6 +242,8 @@ impl WindowElement {
     /// Get the output this window is on.
     ///
     /// This method gets the first tag the window has and returns its output.
+    ///
+    /// This method uses a [`RefCell`].
     pub fn output(&self, state: &State) -> Option<Output> {
         self.with_state(|st| st.tags.first().and_then(|tag| tag.output(state)))
     }

src/window/window_state.rs

@@ -75,29 +75,6 @@ pub struct WindowElementState {
 }
 
 /// The state of a window's resize operation.
-///
-/// A naive implementation of window swapping would probably immediately call
-/// [`space.map_element()`] right after setting its size through [`with_pending_state()`] and
-/// sending a configure event. However, the client will probably not acknowledge the configure
-/// until *after* the window has moved, causing flickering.
-///
-/// To solve this, we need to create two additional steps: [`Requested`] and [`Acknowledged`].
-/// If we need to change a window's location when we change its size, instead of
-/// calling `map_element()`, we change the window's [`WindowState`] and set
-/// its [`resize_state`] to `Requested` with the new position we want.
-///
-/// When the client acks the configure, we can move the state to `Acknowledged` in
-/// [`XdgShellHandler.ack_configure()`]. Finally, in [`CompositorHandler.commit()`], we set the
-/// state back to [`Idle`] and map the window.
-///
-/// [`space.map_element()`]: smithay::desktop::space::Space#method.map_element
-/// [`with_pending_state()`]: smithay::wayland::shell::xdg::ToplevelSurface#method.with_pending_state
-/// [`Idle`]: WindowResizeState::Idle
-/// [`Requested`]: WindowResizeState::Requested
-/// [`Acknowledged`]: WindowResizeState::Acknowledged
-/// [`resize_state`]: WindowState#structfield.resize_state
-/// [`XdgShellHandler.ack_configure()`]: smithay::wayland::shell::xdg::XdgShellHandler#method.ack_configure
-/// [`CompositorHandler.commit()`]: smithay::wayland::compositor::CompositorHandler#tymethod.commit
 #[derive(Debug, Default, Clone)]
 pub enum LocationRequestState {
     /// The window doesn't need to be moved.