Clean up stuff and add more docs

src/api.rs

@@ -55,6 +55,10 @@ use self::msg::{Msg, OutgoingMsg};
 
 pub const SOCKET_NAME: &str = "pinnacle_socket";
 
+/// Handle a config process.
+///
+/// `stream` is the incoming stream where messages will be received,
+/// and `sender` sends decoded messages to the main state's handler.
 fn handle_client(
     mut stream: UnixStream,
     sender: Sender<Msg>,
@@ -85,13 +89,16 @@ fn handle_client(
     }
 }
 
+/// A socket source for an event loop that will listen for config processes.
 pub struct PinnacleSocketSource {
+    /// The socket listener
     socket: Generic<UnixListener>,
+    /// The sender that will send messages from clients to the main event loop.
     sender: Sender<Msg>,
 }
 
 impl PinnacleSocketSource {
-    /// Create a loop source that listens for connections to the provided socket_dir.
+    /// Create a loop source that listens for connections to the provided `socket_dir`.
     /// This will also set PINNACLE_SOCKET for use in API implementations.
     pub fn new(sender: Sender<Msg>, socket_dir: &Path) -> anyhow::Result<Self> {
         tracing::debug!("Creating socket source for dir {socket_dir:?}");
@@ -128,7 +135,7 @@ impl PinnacleSocketSource {
                 }
             }
         } else {
-            // If there are, remove them all
+            // If there aren't, remove them all
             for file in std::fs::read_dir(socket_dir)?
                 .filter_map(|entry| entry.ok())
                 .filter(|entry| entry.file_name().to_string_lossy().starts_with(SOCKET_NAME))
@@ -155,6 +162,7 @@ impl PinnacleSocketSource {
     }
 }
 
+/// Send a message to a client.
 pub fn send_to_client(
     stream: &mut UnixStream,
     msg: &OutgoingMsg,
@@ -171,12 +179,14 @@ pub fn send_to_client(
             return Ok(()); // TODO:
         }
     }
+
     if let Err(err) = stream.write_all(msg.as_slice()) {
         if err.kind() == io::ErrorKind::BrokenPipe {
             // TODO: something
             return Ok(()); // TODO:
         }
-    };
+    }
+
     Ok(())
 }
 

src/api/handlers.rs

@@ -23,8 +23,10 @@ use crate::{
 use crate::state::{State, WithState};
 
 impl State {
+    /// Handle a client message.
     pub fn handle_msg(&mut self, msg: Msg) {
-        // tracing::debug!("Got {msg:?}");
+        tracing::trace!("Got {msg:?}");
+
         match msg {
             Msg::SetKeybind {
                 key,
@@ -452,6 +454,7 @@ impl State {
         }
     }
 
+    /// Handle a client request.
     fn handle_request(&mut self, request_id: RequestId, request: Request) {
         let stream = self
             .api_state
@@ -663,6 +666,8 @@ impl State {
         }
     }
 
+    // Welcome to indentation hell
+    /// Handle a received spawn command by spawning the command and hooking up any callbacks.
     pub fn handle_spawn(&self, command: Vec<String>, callback_id: Option<CallbackId>) {
         let mut command = command.into_iter();
         let Some(program) = command.next() else {
@@ -700,7 +705,7 @@ impl State {
         else {
             // TODO: notify user that program doesn't exist
             tracing::warn!(
-                "tried to run {}, but it doesn't exist",
+                "Tried to run {}, but it doesn't exist",
                 program.to_string_lossy()
             );
             return;
@@ -720,17 +725,19 @@ impl State {
                     while let Some(line) = reader.next().await {
                         match line {
                             Ok(line) => {
+                                let msg = OutgoingMsg::CallCallback {
+                                    callback_id,
+                                    args: Some(Args::Spawn {
+                                        stdout: Some(line),
+                                        stderr: None,
+                                        exit_code: None,
+                                        exit_msg: None,
+                                    }),
+                                };
+
                                 crate::api::send_to_client(
                                     &mut stream_out.lock().expect("Couldn't lock stream"),
-                                    &OutgoingMsg::CallCallback {
+                                    &msg,
-                                        callback_id,
-                                        args: Some(Args::Spawn {
-                                            stdout: Some(line),
-                                            stderr: None,
-                                            exit_code: None,
-                                            exit_msg: None,
-                                        }),
-                                    },
                                 )
                                 .expect("Send to client failed"); // TODO: notify instead of crash
                             }
@@ -752,17 +759,19 @@ impl State {
                     while let Some(line) = reader.next().await {
                         match line {
                             Ok(line) => {
+                                let msg = OutgoingMsg::CallCallback {
+                                    callback_id,
+                                    args: Some(Args::Spawn {
+                                        stdout: None,
+                                        stderr: Some(line),
+                                        exit_code: None,
+                                        exit_msg: None,
+                                    }),
+                                };
+
                                 crate::api::send_to_client(
                                     &mut stream_err.lock().expect("Couldn't lock stream"),
-                                    &OutgoingMsg::CallCallback {
+                                    &msg,
-                                        callback_id,
-                                        args: Some(Args::Spawn {
-                                            stdout: None,
-                                            stderr: Some(line),
-                                            exit_code: None,
-                                            exit_msg: None,
-                                        }),
-                                    },
                                 )
                                 .expect("Send to client failed"); // TODO: notify instead of crash
                             }
@@ -779,17 +788,19 @@ impl State {
             let future = async move {
                 match child.status().await {
                     Ok(exit_status) => {
+                        let msg = OutgoingMsg::CallCallback {
+                            callback_id,
+                            args: Some(Args::Spawn {
+                                stdout: None,
+                                stderr: None,
+                                exit_code: exit_status.code(),
+                                exit_msg: Some(exit_status.to_string()),
+                            }),
+                        };
+
                         crate::api::send_to_client(
                             &mut stream_exit.lock().expect("Couldn't lock stream"),
-                            &OutgoingMsg::CallCallback {
+                            &msg,
-                                callback_id,
-                                args: Some(Args::Spawn {
-                                    stdout: None,
-                                    stderr: None,
-                                    exit_code: exit_status.code(),
-                                    exit_msg: Some(exit_status.to_string()),
-                                }),
-                            },
                         )
                         .expect("Send to client failed"); // TODO: notify instead of crash
                     }

src/backend.rs

@@ -85,6 +85,8 @@ pub trait BackendData: 'static {
     fn early_import(&mut self, surface: &WlSurface);
 }
 
+/// Update surface primary scanout outputs and send frames and dmabuf feedback to visible windows
+/// and layers.
 pub fn post_repaint(
     output: &Output,
     render_element_states: &RenderElementStates,

src/backend/udev.rs

@@ -51,7 +51,6 @@ use smithay::{
         ash::vk::ExtPhysicalDeviceDrmFn,
         calloop::{EventLoop, LoopHandle, RegistrationToken},
         drm::{
-            self,
             control::{connector, crtc, ModeTypeFlags},
             Device,
         },
@@ -104,9 +103,9 @@ struct UdevOutputData {
     device_id: DrmNode,
     /// The [Crtc][crtc::Handle] the output is pushing to
     crtc: crtc::Handle,
-    mode: Option<drm::control::Mode>,
 }
 
+// TODO: document desperately
 pub struct Udev {
     pub session: LibSeatSession,
     display_handle: DisplayHandle,
@@ -133,11 +132,11 @@ impl Backend {
 }
 
 impl Udev {
+    /// Schedule a new render that will cause the compositor to redraw everything.
     pub fn schedule_render(&mut self, loop_handle: &LoopHandle<CalloopData>, output: &Output) {
         let Some(surface) = render_surface_for_output(output, &mut self.backends) else {
             return;
         };
-        // tracing::debug!(state = ?surface.render_state, "scheduling render");
 
         match &surface.render_state {
             RenderState::Idle => {
@@ -162,10 +161,10 @@ impl State {
     /// This will first clear the overlay plane to prevent any lingering artifacts,
     /// then switch the vt.
     ///
-    /// Yells at you when called on the winit backend. Bad developer. Very bad.
+    /// Does nothing when called on the winit backend.
     pub fn switch_vt(&mut self, vt: i32) {
         match &mut self.backend {
-            Backend::Winit(_) => tracing::error!("Called `switch_vt` on winit"),
+            Backend::Winit(_) => (),
             Backend::Udev(udev) => {
                 for backend in udev.backends.values_mut() {
                     for surface in backend.surfaces.values_mut() {
@@ -532,6 +531,7 @@ pub fn run_udev() -> anyhow::Result<()> {
     Ok(())
 }
 
+// TODO: document desperately
 struct UdevBackendData {
     surfaces: HashMap<crtc::Handle, RenderSurface>,
     gbm: GbmDevice<DrmDeviceFd>,
@@ -888,29 +888,9 @@ impl State {
         output.change_current_state(Some(wl_mode), None, None, Some(position));
         self.space.map_output(&output, position);
 
-        // The preferred mode with the highest refresh rate
-        // Logic from niri
-        let mode = connector
-            .modes()
-            .iter()
-            .max_by(|mode1, mode2| {
-                let mode1_preferred = mode1.mode_type().contains(ModeTypeFlags::PREFERRED);
-                let mode2_preferred = mode2.mode_type().contains(ModeTypeFlags::PREFERRED);
-
-                use std::cmp::Ordering;
-
-                match (mode1_preferred, mode2_preferred) {
-                    (true, false) => Ordering::Greater,
-                    (false, true) => Ordering::Less,
-                    _ => mode1.vrefresh().cmp(&mode2.vrefresh()),
-                }
-            })
-            .copied();
-
         output.user_data().insert_if_missing(|| UdevOutputData {
             crtc,
             device_id: node,
-            mode,
         });
 
         let allocator = GbmAllocator::new(
@@ -1332,11 +1312,7 @@ fn render_surface_for_output<'a>(
     output: &Output,
     backends: &'a mut HashMap<DrmNode, UdevBackendData>,
 ) -> Option<&'a mut RenderSurface> {
-    let UdevOutputData {
+    let UdevOutputData { device_id, crtc } = output.user_data().get()?;
-        device_id,
-        crtc,
-        mode: _,
-    } = output.user_data().get()?;
 
     backends
         .get_mut(device_id)

src/backend/winit.rs

@@ -222,7 +222,7 @@ pub fn run_winit() -> anyhow::Result<()> {
                         state.process_input_event(input_evt);
                     }
                     WinitEvent::Refresh => {
-                        state.render_window(&output);
+                        state.render_winit_window(&output);
                     }
                 });
 
@@ -233,7 +233,7 @@ pub fn run_winit() -> anyhow::Result<()> {
                     }
                 };
 
-                state.render_window(&output);
+                state.render_winit_window(&output);
 
                 TimeoutAction::ToDuration(Duration::from_micros(((1.0 / 144.0) * 1000000.0) as u64))
             });
@@ -260,7 +260,7 @@ pub fn run_winit() -> anyhow::Result<()> {
 }
 
 impl State {
-    fn render_window(&mut self, output: &Output) {
+    fn render_winit_window(&mut self, output: &Output) {
         let winit = self.backend.winit_mut();
 
         let pending_wins = self

src/config.rs

@@ -27,6 +27,8 @@ use crate::{
 
 const DEFAULT_SOCKET_DIR: &str = "/tmp";
 
+/// The metaconfig struct containing what to run, what envs to run it with, various keybinds, and
+/// the target socket directory.
 #[derive(serde::Deserialize, Debug)]
 pub struct Metaconfig {
     pub command: Vec<String>,
@@ -42,6 +44,7 @@ pub struct Keybind {
     pub key: Key,
 }
 
+// TODO: accept xkbcommon names instead
 #[derive(serde::Deserialize, Debug, Clone, Copy)]
 #[serde(rename_all = "snake_case")]
 #[repr(u32)]
@@ -116,10 +119,14 @@ pub enum Key {
     Escape = keysyms::KEY_Escape,
 }
 
+/// The current state of configuration.
 #[derive(Default, Debug)]
 pub struct Config {
+    /// Window rules and conditions on when those rules should apply
     pub window_rules: Vec<(WindowRuleCondition, WindowRule)>,
+    /// All callbacks that should be run when outputs are connected
     pub output_callback_ids: Vec<CallbackId>,
+    /// Saved states when outputs are disconnected
     pub connector_saved_states: HashMap<OutputName, ConnectorSavedState>,
 }
 
@@ -127,7 +134,9 @@ pub struct Config {
 /// connector, the saved state will apply to restore its state.
 #[derive(Debug, Default, Clone)]
 pub struct ConnectorSavedState {
+    /// The old location
     pub loc: Point<i32, Logical>,
+    /// The output's previous tags
     pub tags: Vec<Tag>,
 }
 
@@ -173,8 +182,8 @@ impl State {
         self.input_state.libinput_settings.clear();
         self.config.window_rules.clear();
 
-        tracing::debug!("Killing old config");
         if let Some(channel) = self.api_state.kill_channel.as_ref() {
+            tracing::debug!("Killing old config");
             if let Err(err) = futures_lite::future::block_on(channel.send(())) {
                 tracing::warn!("failed to send kill ping to config future: {err}");
             }
@@ -235,13 +244,13 @@ impl State {
 
         let mut command = metaconfig.command.iter();
 
-        let arg1 = command
+        let arg0 = command
             .next()
             .context("command in metaconfig.toml was empty")?;
 
         let command = command.collect::<Vec<_>>();
 
-        tracing::debug!(arg1, ?command);
+        tracing::debug!(arg0, ?command);
 
         let envs = metaconfig
             .envs
@@ -268,7 +277,7 @@ impl State {
 
         tracing::debug!("Config envs are {envs:?}");
 
-        let mut child = async_process::Command::new(arg1)
+        let mut child = async_process::Command::new(arg0)
             .args(command)
             .envs(envs)
             .current_dir(config_dir)
@@ -305,6 +314,7 @@ impl State {
 
         self.input_state.reload_keybind = Some(reload_keybind);
         self.input_state.kill_keybind = Some(kill_keybind);
+
         self.api_state.socket_token = Some(socket_token);
 
         let (kill_channel, future_channel) = async_channel::unbounded::<()>();

src/focus.rs

@@ -20,7 +20,9 @@ use crate::{
 
 #[derive(Default)]
 pub struct FocusState {
+    /// The ordering of window focus
     pub focus_stack: Vec<WindowElement>,
+    /// The focused output, currently defined to be the one the pointer is on.
     pub focused_output: Option<Output>,
 }
 
@@ -89,6 +91,7 @@ impl FocusState {
     }
 }
 
+/// Different focusable objects.
 #[derive(Debug, Clone, PartialEq)]
 pub enum FocusTarget {
     Window(WindowElement),

src/grab/move_grab.rs

@@ -24,14 +24,17 @@ use crate::{
     },
 };
 
-pub struct MoveSurfaceGrab<S: SeatHandler> {
+/// Data for moving a window.
-    pub start_data: GrabStartData<S>,
+pub struct MoveSurfaceGrab {
+    pub start_data: GrabStartData<State>,
+    /// The window being moved
     pub window: WindowElement,
     pub initial_window_loc: Point<i32, Logical>,
+    /// Which button initiated the grab
     pub button_used: u32,
 }
 
-impl PointerGrab<State> for MoveSurfaceGrab<State> {
+impl PointerGrab<State> for MoveSurfaceGrab {
     fn frame(&mut self, data: &mut State, handle: &mut PointerInnerHandle<'_, State>) {
         handle.frame(data);
     }
@@ -60,14 +63,11 @@ impl PointerGrab<State> for MoveSurfaceGrab<State> {
                 .expect("failed to raise x11 win");
         }
 
-        // tracing::info!("window geo is: {:?}", self.window.geometry());
+        let is_tiled = self
-        // tracing::info!("loc is: {:?}", data.space.element_location(&self.window));
-
-        let tiled = self
             .window
             .with_state(|state| state.floating_or_tiled.is_tiled());
 
-        if tiled {
+        if is_tiled {
             // INFO: this is being used instead of space.element_under(event.location) because that
             // |     uses the bounding box, which is different from the actual geometry
             let window_under = state
@@ -258,6 +258,7 @@ impl PointerGrab<State> for MoveSurfaceGrab<State> {
     }
 }
 
+/// The application initiated a move grab e.g. when you drag a titlebar.
 pub fn move_request_client(
     state: &mut State,
     surface: &WlSurface,
@@ -290,6 +291,7 @@ pub fn move_request_client(
     }
 }
 
+/// The compositor initiated a move grab e.g. you hold the mod key and drag.
 pub fn move_request_server(
     state: &mut State,
     surface: &WlSurface,

src/grab/resize_grab.rs

@@ -44,8 +44,8 @@ impl From<xdg_toplevel::ResizeEdge> for ResizeEdge {
     }
 }
 
-pub struct ResizeSurfaceGrab<S: SeatHandler> {
+pub struct ResizeSurfaceGrab {
-    start_data: GrabStartData<S>,
+    start_data: GrabStartData<State>,
     window: WindowElement,
 
     edges: ResizeEdge,
@@ -56,9 +56,9 @@ pub struct ResizeSurfaceGrab<S: SeatHandler> {
     button_used: u32,
 }
 
-impl<S: SeatHandler> ResizeSurfaceGrab<S> {
+impl ResizeSurfaceGrab {
     pub fn start(
-        start_data: GrabStartData<S>,
+        start_data: GrabStartData<State>,
         window: WindowElement,
         edges: ResizeEdge,
         initial_window_rect: Rectangle<i32, Logical>,
@@ -82,7 +82,7 @@ impl<S: SeatHandler> ResizeSurfaceGrab<S> {
     }
 }
 
-impl PointerGrab<State> for ResizeSurfaceGrab<State> {
+impl PointerGrab<State> for ResizeSurfaceGrab {
     fn frame(&mut self, data: &mut State, handle: &mut PointerInnerHandle<'_, State>) {
         handle.frame(data);
     }
@@ -427,6 +427,7 @@ pub fn handle_commit(state: &mut State, surface: &WlSurface) -> Option<()> {
     Some(())
 }
 
+/// The application requests a resize e.g. when you drag the edges of a window.
 pub fn resize_request_client(
     state: &mut State,
     surface: &WlSurface,
@@ -476,6 +477,7 @@ pub fn resize_request_client(
     }
 }
 
+/// The compositor requested a resize e.g. you hold the mod key and right-click drag.
 pub fn resize_request_server(
     state: &mut State,
     surface: &WlSurface,

src/handlers.rs

@@ -99,7 +99,8 @@ impl CompositorHandler for State {
     }
 
     fn commit(&mut self, surface: &WlSurface) {
-        // tracing::debug!("commit on surface {surface:?}");
+        tracing::trace!("commit on surface {surface:?}");
+
         X11Wm::commit_hook::<CalloopData>(surface);
 
         utils::on_commit_buffer_handler::<Self>(surface);
@@ -112,7 +113,6 @@ impl CompositorHandler for State {
 
         if !compositor::is_sync_subsurface(surface) {
             if let Some(win @ WindowElement::Wayland(window)) = &self.window_for_surface(&root) {
-                // tracing::debug!("window commit thing {:?}", win.class());
                 window.on_commit();
                 win.with_state(|state| {
                     if let LocationRequestState::Acknowledged(new_pos) = state.loc_request_state {
@@ -130,37 +130,37 @@ impl CompositorHandler for State {
 
         crate::grab::resize_grab::handle_commit(self, surface);
 
-        // `surface` is a root window
+        let output = if let Some(output) = self
-        let Some(output) = self
             .window_for_surface(surface)
             .and_then(|win| win.output(self))
-            .or_else(|| {
+        {
-                // `surface` is a descendant of a root window
+            output // surface is a window
-                self.window_for_surface(&root)
+        } else if let Some(output) = self
-                    .and_then(|win| win.output(self))
+            .window_for_surface(&root)
+            .and_then(|win| win.output(self))
+        {
+            output // root is a window
+        } else if let Some(output) = self
+            .popup_manager
+            .find_popup(surface)
+            .and_then(|popup| find_popup_root_surface(&popup).ok())
+            .and_then(|surf| self.window_for_surface(&surf))
+            .and_then(|win| win.output(self))
+        {
+            output // surface is a popup
+        } else if let Some(output) = self
+            .space
+            .outputs()
+            .find(|op| {
+                let layer_map = layer_map_for_output(op);
+                layer_map
+                    .layer_for_surface(surface, WindowSurfaceType::ALL)
+                    .is_some()
             })
-            .or_else(|| {
+            .cloned()
-                // `surface` is a popup
+        {
-                self.popup_manager
+            output // surface is a layer surface
-                    .find_popup(surface)
+        } else {
-                    .and_then(|popup| find_popup_root_surface(&popup).ok())
-                    .and_then(|surf| self.window_for_surface(&surf))
-                    .and_then(|win| win.output(self))
-            })
-            .or_else(|| {
-                // `surface` is a layer surface
-                self.space
-                    .outputs()
-                    .find(|op| {
-                        let layer_map = layer_map_for_output(op);
-                        layer_map
-                            .layer_for_surface(surface, WindowSurfaceType::ALL)
-                            .is_some()
-                    })
-                    .cloned()
-            })
-        // TODO: cursor surface and dnd icon
-        else {
             return;
         };
 
@@ -355,6 +355,7 @@ delegate_viewporter!(State);
 
 impl FractionalScaleHandler for State {
     fn new_fractional_scale(&mut self, surface: WlSurface) {
+        // comment yanked from anvil
         // Here we can set the initial fractional scale
         //
         // First we look if the surface already has a primary scan-out output, if not

src/input.rs

@@ -38,7 +38,9 @@ pub struct InputState {
     pub mousebinds: HashMap<(ModifierMask, u32, MouseEdge), CallbackId>,
     pub reload_keybind: Option<(ModifierMask, Keysym)>,
     pub kill_keybind: Option<(ModifierMask, Keysym)>,
+    /// User defined libinput settings that will be applied
     pub libinput_settings: Vec<LibinputSetting>,
+    /// All libinput devices that have been connected
     pub libinput_devices: Vec<input::Device>,
 }
 
@@ -50,6 +52,7 @@ impl InputState {
 
 #[derive(Debug)]
 enum KeyAction {
+    /// Call a callback from a config process
     CallCallback(CallbackId),
     Quit,
     SwitchVt(i32),
@@ -101,58 +104,46 @@ impl State {
             })
         });
 
-        // I think I'm going a bit too far with the functional stuff
+        if let Some(window) = top_fullscreen_window {
+            Some((FocusTarget::from(window.clone()), output_geo.loc))
+        } else if let (Some(layer), _) | (None, Some(layer)) = (
+            layers.layer_under(wlr_layer::Layer::Overlay, point),
+            layers.layer_under(wlr_layer::Layer::Top, point),
+        ) {
+            let layer_loc = layers.layer_geometry(layer).expect("no layer geo").loc;
+            Some((FocusTarget::from(layer.clone()), output_geo.loc + layer_loc))
+        } else if let Some(ret) = self
+            .space
+            .elements()
+            .rev()
+            .filter(|win| win.is_on_active_tag(self.space.outputs()))
+            .find_map(|win| {
+                let loc = self
+                    .space
+                    .element_location(win)
+                    .expect("called elem loc on unmapped win")
+                    - win.geometry().loc;
 
-        // The topmost fullscreen window
+                win.is_in_input_region(&(point - loc.to_f64()))
-        top_fullscreen_window
+                    .then(|| (win.clone().into(), loc))
-            .map(|window| (FocusTarget::from(window.clone()), output_geo.loc))
-            .or_else(|| {
-                // The topmost layer surface in Overlay or Top
-                layers
-                    .layer_under(wlr_layer::Layer::Overlay, point)
-                    .or_else(|| layers.layer_under(wlr_layer::Layer::Top, point))
-                    .map(|layer| {
-                        let layer_loc = layers.layer_geometry(layer).expect("no layer geo").loc;
-                        (FocusTarget::from(layer.clone()), output_geo.loc + layer_loc)
-                    })
-            })
-            .or_else(|| {
-                // The topmost window
-                self.space
-                    .elements()
-                    .rev()
-                    .filter(|win| win.is_on_active_tag(self.space.outputs()))
-                    .find_map(|win| {
-                        let loc = self
-                            .space
-                            .element_location(win)
-                            .expect("called elem loc on unmapped win")
-                            - win.geometry().loc;
-
-                        if win.is_in_input_region(&(point - loc.to_f64())) {
-                            Some((win.clone().into(), loc))
-                        } else {
-                            None
-                        }
-                    })
-            })
-            .or_else(|| {
-                // The topmost layer surface in Bottom or Background
-                layers
-                    .layer_under(wlr_layer::Layer::Bottom, point)
-                    .or_else(|| layers.layer_under(wlr_layer::Layer::Background, point))
-                    .map(|layer| {
-                        let layer_loc = layers.layer_geometry(layer).expect("no layer geo").loc;
-                        (FocusTarget::from(layer.clone()), output_geo.loc + layer_loc)
-                    })
             })
+        {
+            Some(ret)
+        } else if let (Some(layer), _) | (None, Some(layer)) = (
+            layers.layer_under(wlr_layer::Layer::Overlay, point),
+            layers.layer_under(wlr_layer::Layer::Top, point),
+        ) {
+            let layer_loc = layers.layer_geometry(layer).expect("no layer geo").loc;
+            Some((FocusTarget::from(layer.clone()), output_geo.loc + layer_loc))
+        } else {
+            None
+        }
     }
 
     fn keyboard<I: InputBackend>(&mut self, event: I::KeyboardKeyEvent) {
         let serial = SERIAL_COUNTER.next_serial();
         let time = event.time_msec();
         let press_state = event.state();
-        let mut move_mode = false;
 
         let reload_keybind = self.input_state.reload_keybind;
         let kill_keybind = self.input_state.kill_keybind;
@@ -196,16 +187,15 @@ impl State {
                         modifier_mask.push(Modifier::Super);
                     }
                     let modifier_mask = ModifierMask::from(modifier_mask);
+
                     let raw_sym = keysym.raw_syms().iter().next();
                     let mod_sym = keysym.modified_sym();
 
                     let cb_id_mod = state.input_state.keybinds.get(&(modifier_mask, mod_sym));
 
-                    let cb_id_raw = if let Some(raw_sym) = raw_sym {
+                    let cb_id_raw = raw_sym.and_then(|raw_sym| {
                         state.input_state.keybinds.get(&(modifier_mask, *raw_sym))
-                    } else {
+                    });
-                        None
-                    };
 
                     match (cb_id_mod, cb_id_raw) {
                         (Some(cb_id), _) | (None, Some(cb_id)) => {
@@ -214,9 +204,9 @@ impl State {
                         (None, None) => (),
                     }
 
-                    if Some((modifier_mask, mod_sym)) == kill_keybind {
+                    if kill_keybind == Some((modifier_mask, mod_sym)) {
                         return FilterResult::Intercept(KeyAction::Quit);
-                    } else if Some((modifier_mask, mod_sym)) == reload_keybind {
+                    } else if reload_keybind == Some((modifier_mask, mod_sym)) {
                         return FilterResult::Intercept(KeyAction::ReloadConfig);
                     } else if let mut vt @ keysyms::KEY_XF86Switch_VT_1
                         ..=keysyms::KEY_XF86Switch_VT_12 = keysym.modified_sym().raw()
@@ -227,16 +217,6 @@ impl State {
                     }
                 }
 
-                if keysym.modified_sym() == Keysym::Control_L {
-                    match press_state {
-                        KeyState::Pressed => {
-                            move_mode = true;
-                        }
-                        KeyState::Released => {
-                            move_mode = false;
-                        }
-                    }
-                }
                 FilterResult::Forward
             },
         );
@@ -256,18 +236,17 @@ impl State {
                 }
             }
             Some(KeyAction::SwitchVt(vt)) => {
-                if self.backend.is_udev() {
+                self.switch_vt(vt);
-                    self.switch_vt(vt);
-                }
             }
             Some(KeyAction::Quit) => {
+                tracing::info!("Quitting Pinnacle");
                 self.loop_signal.stop();
             }
             Some(KeyAction::ReloadConfig) => {
                 self.start_config(crate::config::get_config_dir())
                     .expect("failed to restart config");
             }
-            None => {}
+            None => (),
         }
     }
 
@@ -283,17 +262,17 @@ impl State {
 
         let pointer_loc = pointer.current_location();
 
-        let edge = match button_state {
+        let mouse_edge = match button_state {
             ButtonState::Released => MouseEdge::Release,
             ButtonState::Pressed => MouseEdge::Press,
         };
         let modifier_mask = ModifierMask::from(keyboard.modifier_state());
 
         // If any mousebinds are detected, call the config's callback and return.
-        if let Some(&callback_id) = self
+        if let Some(&callback_id) =
-            .input_state
+            self.input_state
-            .mousebinds
+                .mousebinds
-            .get(&(modifier_mask, button, edge))
+                .get(&(modifier_mask, button, mouse_edge))
         {
             if let Some(stream) = self.api_state.stream.as_ref() {
                 crate::api::send_to_client(
@@ -310,22 +289,20 @@ impl State {
 
         // If the button was clicked, focus on the window below if exists, else
         // unfocus on windows.
-        if ButtonState::Pressed == button_state {
+        if button_state == ButtonState::Pressed {
             if let Some((focus, _)) = self.surface_under(pointer_loc) {
                 // Move window to top of stack.
                 if let FocusTarget::Window(window) = &focus {
                     self.space.raise_element(window, true);
                     if let WindowElement::X11(surface) = &window {
-                        if !surface.is_override_redirect() {
+                        self.xwm
-                            self.xwm
+                            .as_mut()
-                                .as_mut()
+                            .expect("no xwm")
-                                .expect("no xwm")
+                            .raise_window(surface)
-                                .raise_window(surface)
+                            .expect("failed to raise x11 win");
-                                .expect("failed to raise x11 win");
+                        surface
-                            surface
+                            .set_activated(true)
-                                .set_activated(true)
+                            .expect("failed to set x11 win to activated");
-                                .expect("failed to set x11 win to activated");
-                        }
                     }
                 }
 
@@ -371,7 +348,6 @@ impl State {
             }
         };
 
-        // Send the button event to the client.
         pointer.button(
             self,
             &ButtonEvent {
@@ -391,14 +367,10 @@ impl State {
             .amount(Axis::Horizontal)
             .unwrap_or_else(|| event.amount_discrete(Axis::Horizontal).unwrap_or(0.0) * 3.0);
 
-        let mut vertical_amount = event
+        let vertical_amount = event
             .amount(Axis::Vertical)
             .unwrap_or_else(|| event.amount_discrete(Axis::Vertical).unwrap_or(0.0) * 3.0);
 
-        if self.backend.is_winit() {
-            vertical_amount = -vertical_amount;
-        }
-
         let horizontal_amount_discrete = event.amount_discrete(Axis::Horizontal);
         let vertical_amount_discrete = event.amount_discrete(Axis::Vertical);
 
@@ -422,18 +394,15 @@ impl State {
             frame = frame.stop(Axis::Vertical);
         }
 
-        // tracing::debug!(
-        //     "axis on current focus: {:?}",
-        //     self.seat.get_pointer().unwrap().current_focus()
-        // );
-
         let pointer = self.seat.get_pointer().expect("Seat has no pointer");
 
         pointer.axis(self, frame);
         pointer.frame(self);
     }
 
-    /// Clamp pointer coordinates inside outputs
+    /// Clamp pointer coordinates inside outputs.
+    ///
+    /// This returns the nearest point inside an output.
     fn clamp_coords(&self, pos: Point<f64, Logical>) -> Point<f64, Logical> {
         if self.space.outputs().next().is_none() {
             return pos;
@@ -467,6 +436,7 @@ impl State {
         let Some(output) = self.space.outputs().next() else {
             return;
         };
+
         let output_geo = self
             .space
             .output_geometry(output)
@@ -475,8 +445,6 @@ impl State {
         let serial = SERIAL_COUNTER.next_serial();
         let pointer = self.seat.get_pointer().expect("Seat has no pointer"); // FIXME: handle err
 
-        // tracing::info!("pointer_loc: {:?}", pointer_loc);
-
         self.pointer_location = pointer_loc;
 
         match self.focus_state.focused_output {
@@ -495,13 +463,9 @@ impl State {
             }
         }
 
-        let surface_under_pointer = self.surface_under(pointer_loc);
-
-        // tracing::debug!("surface_under_pointer: {surface_under_pointer:?}");
-        // tracing::debug!("pointer focus: {:?}", pointer.current_focus());
         pointer.motion(
             self,
-            surface_under_pointer,
+            self.surface_under(pointer_loc),
             &MotionEvent {
                 location: pointer_loc,
                 serial,
@@ -537,7 +501,6 @@ impl State {
 
         let surface_under = self.surface_under(self.pointer_location);
 
-        // tracing::info!("{:?}", self.pointer_location);
         if let Some(pointer) = self.seat.get_pointer() {
             pointer.motion(
                 self,

src/layout.rs

@@ -17,8 +17,6 @@ use crate::{
 impl State {
     /// Compute the positions and sizes of tiled windows on
     /// `output` according to the provided [`Layout`].
-    ///
-    /// This will call `request_size_change` on tiled windows.
     fn tile_windows(&self, output: &Output, windows: Vec<WindowElement>, layout: Layout) {
         let Some(rect) = self.space.output_geometry(output).map(|op_geo| {
             let map = layer_map_for_output(output);
@@ -50,7 +48,7 @@ impl State {
         }
     }
 
-    /// Compute tiled window locations and sizes, size maximized and fullscreen windows correctly,
+    /// Compute tiled window locations and sizes, resize maximized and fullscreen windows correctly,
     /// and send configures and that cool stuff.
     pub fn update_windows(&mut self, output: &Output) {
         tracing::debug!("Updating windows");
@@ -97,7 +95,7 @@ impl State {
                 }
                 FullscreenOrMaximized::Maximized => {
                     let map = layer_map_for_output(output);
-                    let geo = if map.layers().peekable().peek().is_none() {
+                    let geo = if map.layers().next().is_none() {
                         // INFO: Sometimes the exclusive zone is some weird number that doesn't match the
                         // |     output res, even when there are no layer surfaces mapped. In this case, we
                         // |     just return the output geometry.
@@ -122,6 +120,7 @@ impl State {
         let mut pending_wins = Vec::<(Point<_, _>, WindowElement)>::new();
         let mut non_pending_wins = Vec::<(Point<_, _>, WindowElement)>::new();
 
+        // TODO: completely refactor how tracking state changes works
         for window in windows_on_foc_tags.iter() {
             window.with_state(|state| {
                 if let LocationRequestState::Sent(loc) = state.loc_request_state {
@@ -133,9 +132,7 @@ impl State {
                             let is_pending = win
                                 .toplevel()
                                 .with_pending_state(|state| state.size != current_state.size);
-                            // for whatever reason vscode on wayland likes to have pending state
+
-                            // (like tiled states) but not commit, so we check for just the size
-                            // here
                             if !is_pending {
                                 tracing::debug!("No pending changes, mapping window");
                                 // TODO: map win here, not down there
@@ -168,7 +165,6 @@ impl State {
             });
         }
 
-        // schedule on all idle
         self.schedule(
             move |_dt| {
                 let all_idle = pending_wins
@@ -301,9 +297,9 @@ fn spiral(windows: Vec<WindowElement>, rect: Rectangle<i32, Logical>) {
     let size = rect.size;
     let loc = rect.loc;
 
-    let mut iter = windows.windows(2).peekable();
+    let mut window_pairs = windows.windows(2).peekable();
 
-    if iter.peek().is_none() {
+    if window_pairs.peek().is_none() {
         if let Some(window) = windows.first() {
             window.change_geometry(Rectangle::from_loc_and_size(loc, size));
         }
@@ -311,7 +307,7 @@ fn spiral(windows: Vec<WindowElement>, rect: Rectangle<i32, Logical>) {
         let mut win1_loc = loc;
         let mut win1_size = size;
 
-        for (i, wins) in iter.enumerate() {
+        for (i, wins) in window_pairs.enumerate() {
             let win1 = &wins[0];
             let win2 = &wins[1];
 
@@ -490,6 +486,7 @@ fn corner(layout: &Layout, windows: Vec<WindowElement>, rect: Rectangle<i32, Log
 }
 
 impl State {
+    /// Swaps two windows in the main window vec and updates all windows.
     pub fn swap_window_positions(&mut self, win1: &WindowElement, win2: &WindowElement) {
         let win1_index = self.windows.iter().position(|win| win == win1);
         let win2_index = self.windows.iter().position(|win| win == win2);

src/output.rs

@@ -12,6 +12,7 @@ use crate::{
 /// A unique identifier for an output.
 ///
 /// An empty string represents an invalid output.
+// TODO: maybe encode that in the type
 #[derive(Debug, Hash, Clone, serde::Serialize, serde::Deserialize, PartialEq, Eq)]
 pub struct OutputName(pub String);
 
@@ -26,6 +27,7 @@ impl OutputName {
     }
 }
 
+/// The state of an output
 #[derive(Default)]
 pub struct OutputState {
     pub tags: Vec<Tag>,

src/render.rs

@@ -141,7 +141,7 @@ where
     }
 }
 
-/// Get the render_elements for the provided tags.
+/// Get render elements for windows on active tags.
 fn tag_render_elements<R>(
     windows: &[WindowElement],
     space: &Space<WindowElement>,
@@ -250,6 +250,7 @@ where
             pointer_element.set_texture(pointer_image.clone());
         }
 
+        // TODO: why is updating the cursor texture in generate_render_elements?
         // draw the cursor as relevant and
         // reset the cursor if the surface is no longer alive
         if let CursorImageStatus::Surface(surface) = &*cursor_status {
@@ -342,11 +343,7 @@ where
                 .map(OutputRenderElements::from),
         );
 
-        output_render_elements.extend(
+        output_render_elements.extend(window_render_elements);
-            window_render_elements
-                .into_iter()
-                .map(OutputRenderElements::from),
-        );
 
         output_render_elements.extend(
             bottom

src/tag.rs

@@ -16,18 +16,22 @@ use crate::{
 
 static TAG_ID_COUNTER: AtomicU32 = AtomicU32::new(0);
 
+/// A unique id for a [`Tag`].
 #[derive(Debug, Hash, PartialEq, Eq, Clone, Copy, serde::Serialize, serde::Deserialize)]
 pub enum TagId {
+    /// The tag given was invalid/nonexistent
     None,
     #[serde(untagged)]
     Some(u32),
 }
 
 impl TagId {
+    /// Get the next available `TagId`.
     fn next() -> Self {
         Self::Some(TAG_ID_COUNTER.fetch_add(1, Ordering::Relaxed))
     }
 
+    /// Get the tag associated with this id.
     pub fn tag(&self, state: &State) -> Option<Tag> {
         state
             .space
@@ -65,6 +69,10 @@ impl PartialEq for TagInner {
 
 impl Eq for TagInner {}
 
+/// A marker for windows.
+///
+/// A window may have 0 or more tags, and you can display 0 or more tags
+/// on each output at a time.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Tag(Rc<RefCell<TagInner>>);
 
@@ -105,6 +113,9 @@ impl Tag {
         })))
     }
 
+    /// Get the output this tag is on.
+    ///
+    /// RefCell Safety: This uses RefCells on every mapped output.
     pub fn output(&self, state: &State) -> Option<Output> {
         state
             .space

src/window.rs

@@ -10,10 +10,10 @@ use smithay::{
         space::SpaceElement,
         utils::{
             send_dmabuf_feedback_surface_tree, send_frames_surface_tree,
-            take_presentation_feedback_surface_tree, under_from_surface_tree,
+            take_presentation_feedback_surface_tree, with_surfaces_surface_tree,
-            with_surfaces_surface_tree, OutputPresentationFeedback,
+            OutputPresentationFeedback,
         },
-        Window, WindowSurfaceType,
+        Window,
     },
     input::{
         keyboard::{KeyboardTarget, KeysymHandle, ModifiersState},
@@ -41,29 +41,18 @@ use self::window_state::{LocationRequestState, WindowElementState};
 
 pub mod window_state;
 
+/// The different types of windows.
 #[derive(Debug, Clone, PartialEq)]
 pub enum WindowElement {
+    /// This is a native Wayland window.
     Wayland(Window),
+    /// This is an Xwayland window.
     X11(X11Surface),
+    /// This is an Xwayland override redirect window, which should not be messed with.
     X11OverrideRedirect(X11Surface),
 }
 
 impl WindowElement {
-    pub fn surface_under(
-        &self,
-        location: Point<f64, Logical>,
-        window_type: WindowSurfaceType,
-    ) -> Option<(WlSurface, Point<i32, Logical>)> {
-        match self {
-            WindowElement::Wayland(window) => window.surface_under(location, window_type),
-            WindowElement::X11(surface) | WindowElement::X11OverrideRedirect(surface) => {
-                surface.wl_surface().and_then(|wl_surf| {
-                    under_from_surface_tree(&wl_surf, location, (0, 0), window_type)
-                })
-            }
-        }
-    }
-
     pub fn with_surfaces<F>(&self, processor: F)
     where
         F: FnMut(&WlSurface, &SurfaceData) + Copy,
@@ -189,7 +178,7 @@ impl WindowElement {
     ///
     /// Xwayland windows will still receive a configure.
     ///
-    /// This method uses a [`RefCell`].
+    /// RefCell Safety: This method uses a [`RefCell`] on this window.
     // TODO: ^ does that make things flicker?
     pub fn change_geometry(&self, new_geo: Rectangle<i32, Logical>) {
         match self {
@@ -256,11 +245,13 @@ impl WindowElement {
     ///
     /// This method gets the first tag the window has and returns its output.
     ///
-    /// This method uses a [`RefCell`].
+    /// RefCell Safety: This method uses a [`RefCell`] on this window and every mapped output.
     pub fn output(&self, state: &State) -> Option<Output> {
         self.with_state(|st| st.tags.first().and_then(|tag| tag.output(state)))
     }
 
+    /// Returns whether or not this window has an active tag.
+    ///
     /// RefCell Safety: This uses RefCells on both `self` and everything in `outputs`.
     pub fn is_on_active_tag<'a>(&self, outputs: impl IntoIterator<Item = &'a Output>) -> bool {
         let tags = outputs
@@ -643,7 +634,7 @@ impl WithState for WindowElement {
     {
         let state = self
             .user_data()
-            .get_or_insert(RefCell::<Self::State>::default);
+            .get_or_insert(|| RefCell::new(WindowElementState::new()));
 
         func(&mut state.borrow_mut())
     }

src/window/window_state.rs

@@ -1,12 +1,9 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 
-use std::{
+use std::sync::atomic::{AtomicU32, Ordering};
-    cell::RefCell,
-    sync::atomic::{AtomicU32, Ordering},
-};
 
 use smithay::{
-    desktop::{space::SpaceElement, Window},
+    desktop::space::SpaceElement,
     reexports::wayland_protocols::xdg::shell::server::xdg_toplevel,
     utils::{Logical, Point, Rectangle, Serial},
 };
@@ -46,26 +43,7 @@ impl WindowId {
     }
 }
 
-#[derive(Debug, Default)]
+/// State of a [`WindowElement`]
-pub struct WindowState {
-    pub minimized: bool,
-}
-
-impl WithState for Window {
-    type State = WindowState;
-
-    fn with_state<F, T>(&self, func: F) -> T
-    where
-        F: FnOnce(&mut Self::State) -> T,
-    {
-        let state = self
-            .user_data()
-            .get_or_insert(RefCell::<Self::State>::default);
-
-        func(&mut state.borrow_mut())
-    }
-}
-
 #[derive(Debug)]
 pub struct WindowElementState {
     /// The id of this window.
@@ -106,7 +84,7 @@ impl LocationRequestState {
 }
 
 impl WindowElement {
-    /// This method uses a [`RefCell`].
+    /// RefCell Safety: This method uses a [`RefCell`] on this window.
     pub fn toggle_floating(&self) {
         match self.with_state(|state| state.floating_or_tiled) {
             FloatingOrTiled::Floating(current_rect) => {
@@ -129,7 +107,7 @@ impl WindowElement {
         }
     }
 
-    /// This method uses a [`RefCell`].
+    /// RefCell Safety: This method uses a [`RefCell`] on this window.
     pub fn toggle_fullscreen(&self) {
         match self.with_state(|state| state.fullscreen_or_maximized) {
             FullscreenOrMaximized::Neither | FullscreenOrMaximized::Maximized => {
@@ -175,7 +153,7 @@ impl WindowElement {
         }
     }
 
-    /// This method uses a [`RefCell`].
+    /// RefCell Safety: This method uses a [`RefCell`] on this window.
     pub fn toggle_maximized(&self) {
         match self.with_state(|state| state.fullscreen_or_maximized) {
             FullscreenOrMaximized::Neither | FullscreenOrMaximized::Fullscreen => {
@@ -221,6 +199,8 @@ impl WindowElement {
         }
     }
 
+    /// Unsets maximized and fullscreen states for both wayland and xwayland windows
+    /// and unsets tiled states for wayland windows.
     fn set_floating_states(&self) {
         match self {
             WindowElement::Wayland(window) => {
@@ -245,6 +225,8 @@ impl WindowElement {
         }
     }
 
+    /// Unsets maximized and fullscreen states for both wayland and xwayland windows
+    /// and sets tiled states for wayland windows.
     fn set_tiled_states(&self) {
         match self {
             WindowElement::Wayland(window) => {
@@ -270,11 +252,15 @@ impl WindowElement {
     }
 }
 
-// You know what they say, the two hardest things in computer science are
+/// Whether a window is floating or tiled
-// cache invalidation and naming things (and off by one errors).
 #[derive(Debug, Clone, Copy)]
 pub enum FloatingOrTiled {
+    /// The window is floating with the specified geometry.
     Floating(Rectangle<i32, Logical>),
+    /// The window is tiled.
+    ///
+    /// The previous geometry it had when it was floating is stored here.
+    /// This is so when it becomes floating again, it returns to this geometry.
     Tiled(Option<Rectangle<i32, Logical>>),
 }
 
@@ -329,8 +315,8 @@ impl FullscreenOrMaximized {
     }
 }
 
-impl Default for WindowElementState {
+impl WindowElementState {
-    fn default() -> Self {
+    pub fn new() -> Self {
         Self {
             id: WindowId::next(),
             loc_request_state: LocationRequestState::Idle,