Improve window state handling

src/api/window.rs

@@ -94,26 +94,21 @@ impl window_service_server::WindowService for WindowService {
             window_loc.x = x.unwrap_or(window_loc.x);
             window_loc.y = y.unwrap_or(window_loc.y);
 
+            // TODO: window.geometry.size or space.elem_geo
             let mut window_size = window.geometry().size;
             window_size.w = width.unwrap_or(window_size.w);
             window_size.h = height.unwrap_or(window_size.h);
 
             window.with_state_mut(|state| {
-                use crate::window::window_state::FloatingOrTiled;
+                state.floating_loc = Some(window_loc.to_f64());
-                state.floating_or_tiled = match state.floating_or_tiled {
+                state.floating_size = Some(window_size);
-                    FloatingOrTiled::Floating { .. } => FloatingOrTiled::Floating {
-                        loc: window_loc.to_f64(),
-                        size: window_size,
-                    },
-                    FloatingOrTiled::Tiled(_) => {
-                        FloatingOrTiled::Tiled(Some((window_loc.to_f64(), window_size)))
-                    }
-                }
             });
 
-            for output in state.pinnacle.space.outputs_for_element(&window) {
+            if window.with_state(|state| state.floating_or_tiled.is_floating()) {
-                state.pinnacle.request_layout(&output);
+                window.change_geometry(window_loc.to_f64(), window_size);
-                state.schedule_render(&output);
+                if let Some(toplevel) = window.toplevel() {
+                    toplevel.send_pending_configure();
+                }
             }
         })
         .await
@@ -150,11 +145,11 @@ impl window_service_server::WindowService for WindowService {
             };
 
             match fullscreen {
-                Some(fullscreen) => state.set_window_fullscreen(&window, fullscreen),
+                Some(fullscreen) => state.set_window_fullscreen_and_layout(&window, fullscreen),
                 None => {
                     let is_fullscreen = window
                         .with_state(|win_state| win_state.fullscreen_or_maximized.is_fullscreen());
-                    state.set_window_fullscreen(&window, !is_fullscreen);
+                    state.set_window_fullscreen_and_layout(&window, !is_fullscreen);
                 }
             }
         })
@@ -192,11 +187,11 @@ impl window_service_server::WindowService for WindowService {
             };
 
             match maximized {
-                Some(maximized) => state.set_window_maximized(&window, maximized),
+                Some(maximized) => state.set_window_maximized_and_layout(&window, maximized),
                 None => {
                     let is_maximized = window
                         .with_state(|win_state| win_state.fullscreen_or_maximized.is_maximized());
-                    state.set_window_maximized(&window, !is_maximized);
+                    state.set_window_maximized_and_layout(&window, !is_maximized);
                 }
             }
         })
@@ -226,26 +221,22 @@ impl window_service_server::WindowService for WindowService {
                 return;
             };
 
+            let floating = match set_or_toggle {
+                SetOrToggle::Unspecified => unreachable!(),
+                SetOrToggle::Set => true,
+                SetOrToggle::Unset => false,
+                SetOrToggle::Toggle => {
+                    window.with_state(|state| !state.floating_or_tiled.is_floating())
+                }
+            };
+
             let output = window.output(&state.pinnacle);
 
             if let Some(output) = output.as_ref() {
                 state.capture_snapshots_on_output(output, [window.clone()]);
             }
 
-            match set_or_toggle {
+            state.pinnacle.set_window_floating(&window, floating);
-                SetOrToggle::Set => {
-                    if !window.with_state(|state| state.floating_or_tiled.is_floating()) {
-                        window.toggle_floating();
-                    }
-                }
-                SetOrToggle::Unset => {
-                    if window.with_state(|state| state.floating_or_tiled.is_floating()) {
-                        window.toggle_floating();
-                    }
-                }
-                SetOrToggle::Toggle => window.toggle_floating(),
-                SetOrToggle::Unspecified => unreachable!(),
-            }
 
             let Some(output) = output else {
                 return;
@@ -795,8 +786,8 @@ impl From<WindowRule> for crate::window::rules::WindowRule {
             false => Some(rule.tags.into_iter().map(TagId).collect::<Vec<_>>()),
         };
         let floating_or_tiled = rule.floating.map(|floating| match floating {
-            true => crate::window::rules::FloatingOrTiled::Floating,
+            true => crate::window::window_state::FloatingOrTiled::Floating,
-            false => crate::window::rules::FloatingOrTiled::Tiled,
+            false => crate::window::window_state::FloatingOrTiled::Tiled,
         });
         let size = rule.width.and_then(|w| {
             rule.height.and_then(|h| {

src/grab/move_grab.rs

@@ -21,7 +21,7 @@ use tracing::{debug, warn};
 
 use crate::{
     state::{State, WithState},
-    window::{window_state::FloatingOrTiled, WindowElement},
+    window::WindowElement,
 };
 
 /// Data for moving a window.
@@ -70,11 +70,39 @@ impl PointerGrab<State> for MoveSurfaceGrab {
             }
         }
 
-        let is_tiled = self
+        let can_move = self.window.with_state(|state| {
-            .window
+            state.floating_or_tiled.is_floating() && state.fullscreen_or_maximized.is_neither()
-            .with_state(|state| state.floating_or_tiled.is_tiled());
+        });
 
-        if is_tiled {
+        if can_move {
+            let delta = event.location - self.start_data.location;
+            let new_loc = self.initial_window_loc.to_f64() + delta;
+            // FIXME: space maps locs as i32 not f64
+            state
+                .pinnacle
+                .space
+                .map_element(self.window.clone(), new_loc.to_i32_round(), true);
+
+            self.window.with_state_mut(|state| {
+                state.floating_loc = Some(new_loc);
+            });
+
+            if let Some(surface) = self.window.x11_surface() {
+                if !surface.is_override_redirect() {
+                    let geo = surface.geometry();
+                    // FIXME: prolly not fixable but xwayland configures with loc i32 not f64
+                    let new_geo = Rectangle::from_loc_and_size(new_loc.to_i32_round(), geo.size);
+                    surface
+                        .configure(new_geo)
+                        .expect("failed to configure x11 win");
+                }
+            }
+
+            let outputs = state.pinnacle.space.outputs_for_element(&self.window);
+            for output in outputs {
+                state.schedule_render(&output);
+            }
+        } else {
             // 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
@@ -114,43 +142,6 @@ impl PointerGrab<State> for MoveSurfaceGrab {
                     .pinnacle
                     .swap_window_positions(&self.window, &window_under);
             }
-        } else {
-            let delta = event.location - self.start_data.location;
-            let new_loc = self.initial_window_loc.to_f64() + delta;
-            // FIXME: space maps locs as i32 not f64
-            state
-                .pinnacle
-                .space
-                .map_element(self.window.clone(), new_loc.to_i32_round(), true);
-
-            let size = state
-                .pinnacle
-                .space
-                .element_geometry(&self.window)
-                .expect("window wasn't mapped")
-                .size;
-
-            self.window.with_state_mut(|state| {
-                if state.floating_or_tiled.is_floating() {
-                    state.floating_or_tiled = FloatingOrTiled::Floating { loc: new_loc, size };
-                }
-            });
-
-            if let Some(surface) = self.window.x11_surface() {
-                if !surface.is_override_redirect() {
-                    let geo = surface.geometry();
-                    // FIXME: prolly not fixable but xwayland configures with loc i32 not f64
-                    let new_geo = Rectangle::from_loc_and_size(new_loc.to_i32_round(), geo.size);
-                    surface
-                        .configure(new_geo)
-                        .expect("failed to configure x11 win");
-                }
-            }
-
-            let outputs = state.pinnacle.space.outputs_for_element(&self.window);
-            for output in outputs {
-                state.schedule_render(&output);
-            }
         }
     }
 

src/grab/resize_grab.rs

@@ -22,7 +22,7 @@ use smithay::{
 
 use crate::{
     state::{Pinnacle, State, WithState},
-    window::{window_state::FloatingOrTiled, WindowElement},
+    window::WindowElement,
 };
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -451,19 +451,8 @@ impl Pinnacle {
             window_loc.y = new_y;
         }
 
-        let size = self
-            .space
-            .element_geometry(&window)
-            .expect("called element_geometry on unmapped window")
-            .size;
-
         window.with_state_mut(|state| {
-            if state.floating_or_tiled.is_floating() {
+            state.floating_loc = Some(window_loc);
-                state.floating_or_tiled = FloatingOrTiled::Floating {
-                    loc: window_loc,
-                    size,
-                };
-            }
         });
 
         if new_loc.0.is_some() || new_loc.1.is_some() {

src/handlers.rs

@@ -73,7 +73,7 @@ use smithay::{
         },
         shell::{
             wlr_layer::{self, Layer, LayerSurfaceData, WlrLayerShellHandler, WlrLayerShellState},
-            xdg::{PopupSurface, XdgPopupSurfaceData, XdgToplevelSurfaceData},
+            xdg::{PopupSurface, SurfaceCachedState, XdgPopupSurfaceData, XdgToplevelSurfaceData},
         },
         shm::{ShmHandler, ShmState},
         tablet_manager::TabletSeatHandler,
@@ -103,6 +103,7 @@ use crate::{
         screencopy::{Screencopy, ScreencopyHandler},
     },
     state::{ClientState, Pinnacle, State, WithState},
+    window::window_state::FloatingOrTiled,
 };
 
 impl BufferHandler for State {
@@ -213,6 +214,39 @@ impl CompositorHandler for State {
                         self.capture_snapshots_on_output(output, []);
                     }
 
+                    unmapped_window.with_state_mut(|state| {
+                        if state.floating_size.is_none() {
+                            state.floating_size = Some(unmapped_window.geometry().size);
+                        }
+                    });
+
+                    // Float windows if necessary
+                    if let Some(toplevel) = unmapped_window.toplevel() {
+                        let has_parent = toplevel.parent().is_some();
+                        let (min_size, max_size) =
+                            compositor::with_states(toplevel.wl_surface(), |states| {
+                                let mut guard = states.cached_state.get::<SurfaceCachedState>();
+                                let state = guard.current();
+                                (state.min_size, state.max_size)
+                            });
+
+                        let requests_constrained_size = min_size.w > 0
+                            && min_size.h > 0
+                            && (min_size.w == max_size.w || min_size.h == max_size.h);
+
+                        let should_float = has_parent || requests_constrained_size;
+
+                        if should_float {
+                            unmapped_window.with_state_mut(|state| {
+                                state.floating_or_tiled = FloatingOrTiled::Floating
+                            });
+                        }
+                    }
+
+                    if unmapped_window.with_state(|state| state.floating_or_tiled.is_floating()) {
+                        self.pinnacle.set_window_floating(&unmapped_window, true);
+                    }
+
                     self.pinnacle
                         .unmapped_windows
                         .retain(|win| win != unmapped_window);
@@ -224,8 +258,27 @@ impl CompositorHandler for State {
                         if unmapped_window.is_on_active_tag() {
                             self.update_keyboard_focus(&focused_output);
 
-                            self.pinnacle.begin_layout_transaction(&focused_output);
+                            if unmapped_window.with_state(|state| {
-                            self.pinnacle.request_layout(&focused_output);
+                                state.floating_or_tiled.is_floating()
+                                    && state.fullscreen_or_maximized.is_neither()
+                            }) {
+                                // TODO: make this sync with commit
+                                let loc = unmapped_window
+                                    .with_state(|state| state.floating_loc)
+                                    .unwrap();
+                                self.pinnacle.space.map_element(
+                                    unmapped_window.clone(),
+                                    loc.to_i32_round(),
+                                    true,
+                                );
+                                unmapped_window
+                                    .toplevel()
+                                    .expect("unreachable")
+                                    .send_pending_configure();
+                            } else {
+                                self.pinnacle.begin_layout_transaction(&focused_output);
+                                self.pinnacle.request_layout(&focused_output);
+                            }
 
                             // It seems wlcs needs immediate frame sends for client tests to work
                             #[cfg(feature = "testing")]
@@ -242,6 +295,17 @@ impl CompositorHandler for State {
                         unmapped_window.place_on_output(&output);
                     }
                     self.pinnacle.apply_window_rules(&unmapped_window);
+                    if unmapped_window.with_state(|state| {
+                        state.floating_or_tiled.is_floating()
+                            && state.fullscreen_or_maximized.is_neither()
+                    }) {
+                        if let Some(size) = unmapped_window.with_state(|state| state.floating_size)
+                        {
+                            if let Some(toplevel) = unmapped_window.toplevel() {
+                                toplevel.with_pending_state(|state| state.size = Some(size));
+                            }
+                        }
+                    }
                     // Still unmapped
                     unmapped_window.on_commit();
                     self.pinnacle.ensure_initial_configure(surface);

src/handlers/foreign_toplevel.rs

@@ -60,7 +60,7 @@ impl ForeignToplevelHandler for State {
             return;
         };
 
-        self.set_window_fullscreen(&window, true);
+        self.set_window_fullscreen_and_layout(&window, true);
     }
 
     fn unset_fullscreen(&mut self, wl_surface: WlSurface) {
@@ -68,7 +68,7 @@ impl ForeignToplevelHandler for State {
             return;
         };
 
-        self.set_window_fullscreen(&window, false);
+        self.set_window_fullscreen_and_layout(&window, false);
     }
 
     fn set_maximized(&mut self, wl_surface: WlSurface) {
@@ -76,7 +76,7 @@ impl ForeignToplevelHandler for State {
             return;
         };
 
-        self.set_window_maximized(&window, true);
+        self.set_window_maximized_and_layout(&window, true);
     }
 
     fn unset_maximized(&mut self, wl_surface: WlSurface) {
@@ -84,7 +84,7 @@ impl ForeignToplevelHandler for State {
             return;
         };
 
-        self.set_window_maximized(&window, false);
+        self.set_window_maximized_and_layout(&window, false);
     }
 
     fn set_minimized(&mut self, wl_surface: WlSurface) {

src/handlers/window.rs

@@ -1,22 +1,13 @@
-use crate::{
+use crate::{state::State, window::WindowElement};
-    state::{State, WithState},
-    window::WindowElement,
-};
 
 impl State {
-    pub fn set_window_maximized(&mut self, window: &WindowElement, maximized: bool) {
+    pub fn set_window_maximized_and_layout(&mut self, window: &WindowElement, maximized: bool) {
         let output = window.output(&self.pinnacle);
         if let Some(output) = output.as_ref() {
             self.capture_snapshots_on_output(output, [window.clone()]);
         }
 
-        if maximized {
+        self.pinnacle.set_window_maximized(window, maximized);
-            if !window.with_state(|state| state.fullscreen_or_maximized.is_maximized()) {
-                window.toggle_maximized();
-            }
-        } else if window.with_state(|state| state.fullscreen_or_maximized.is_maximized()) {
-            window.toggle_maximized();
-        }
 
         if let Some(output) = output {
             self.pinnacle.begin_layout_transaction(&output);
@@ -26,19 +17,13 @@ impl State {
         }
     }
 
-    pub fn set_window_fullscreen(&mut self, window: &WindowElement, fullscreen: bool) {
+    pub fn set_window_fullscreen_and_layout(&mut self, window: &WindowElement, fullscreen: bool) {
         let output = window.output(&self.pinnacle);
         if let Some(output) = output.as_ref() {
             self.capture_snapshots_on_output(output, [window.clone()]);
         }
 
-        if fullscreen {
+        self.pinnacle.set_window_fullscreen(window, fullscreen);
-            if !window.with_state(|state| state.fullscreen_or_maximized.is_fullscreen()) {
-                window.toggle_fullscreen();
-            }
-        } else if window.with_state(|state| state.fullscreen_or_maximized.is_fullscreen()) {
-            window.toggle_fullscreen();
-        }
 
         if let Some(output) = window.output(&self.pinnacle) {
             self.pinnacle.begin_layout_transaction(&output);

src/handlers/xdg_shell.rs

@@ -231,7 +231,7 @@ impl XdgShellHandler for State {
                 return;
             };
 
-            self.set_window_fullscreen(&window, true);
+            self.set_window_fullscreen_and_layout(&window, true);
         }
 
         surface.send_configure();
@@ -246,7 +246,7 @@ impl XdgShellHandler for State {
             return;
         };
 
-        self.set_window_fullscreen(&window, false);
+        self.set_window_fullscreen_and_layout(&window, false);
     }
 
     fn maximize_request(&mut self, surface: ToplevelSurface) {
@@ -254,7 +254,7 @@ impl XdgShellHandler for State {
             return;
         };
 
-        self.set_window_maximized(&window, true);
+        self.set_window_maximized_and_layout(&window, true);
     }
 
     fn unmaximize_request(&mut self, surface: ToplevelSurface) {
@@ -262,7 +262,7 @@ impl XdgShellHandler for State {
             return;
         };
 
-        self.set_window_maximized(&window, false);
+        self.set_window_maximized_and_layout(&window, false);
     }
 
     fn minimize_request(&mut self, _surface: ToplevelSurface) {

src/handlers/xwayland.rs

@@ -3,7 +3,7 @@
 use std::{process::Stdio, time::Duration};
 
 use smithay::{
-    desktop::Window,
+    desktop::{space::SpaceElement, Window},
     input::pointer::CursorIcon,
     utils::{Logical, Point, Rectangle, Size, SERIAL_COUNTER},
     wayland::selection::{
@@ -27,7 +27,7 @@ use tracing::{debug, error, trace, warn};
 use crate::{
     focus::keyboard::KeyboardFocusTarget,
     state::{Pinnacle, State, WithState},
-    window::{window_state::FloatingOrTiled, WindowElement},
+    window::WindowElement,
 };
 
 impl XwmHandler for State {
@@ -47,8 +47,14 @@ impl XwmHandler for State {
             return;
         }
 
+        surface.set_mapped(true).expect("failed to map x11 window");
         let window = WindowElement::new(Window::new_x11_window(surface));
-        let bbox = window.bbox();
+
+        if let Some(output) = self.pinnacle.focused_output() {
+            window.place_on_output(output);
+        }
+
+        self.pinnacle.apply_window_rules(&window);
 
         let output_size = self
             .pinnacle
@@ -63,13 +69,17 @@ impl XwmHandler for State {
             .map(|op| op.current_location())
             .unwrap_or((0, 0).into());
 
+        let size = window
+            .with_state(|state| state.floating_size)
+            .unwrap_or(window.bbox().size);
+
         // Center the popup in the middle of the output.
         // Once I find a way to get an X11Surface's parent it will be centered on the parent if
         // applicable.
         // FIXME: loc is i32
         let loc: Point<i32, Logical> = (
-            output_loc.x + output_size.w / 2 - bbox.size.w / 2,
+            output_loc.x + output_size.w / 2 - size.w / 2,
-            output_loc.y + output_size.h / 2 - bbox.size.h / 2,
+            output_loc.y + output_size.h / 2 - size.h / 2,
         )
             .into();
 
@@ -77,30 +87,27 @@ impl XwmHandler for State {
             unreachable!()
         };
 
-        surface.set_mapped(true).expect("failed to map x11 window");
+        let geo = Rectangle::from_loc_and_size(loc, size);
-
-        let bbox = Rectangle::from_loc_and_size(loc, bbox.size);
 
         surface
-            .configure(bbox)
+            .configure(geo)
             .expect("failed to configure x11 window");
 
-        if let Some(output) = self.pinnacle.focused_output() {
+        let will_float = should_float(surface)
-            window.place_on_output(output);
+            || window.with_state(|state| state.floating_or_tiled.is_floating());
-        }
 
-        if should_float(surface) {
+        if will_float {
             window.with_state_mut(|state| {
-                state.floating_or_tiled = FloatingOrTiled::Floating {
+                if state.floating_loc.is_none() {
-                    loc: bbox.loc.to_f64(),
+                    state.floating_loc = Some(geo.loc.to_f64());
-                    size: bbox.size,
+                }
+                if state.floating_size.is_none() {
+                    tracing::info!(?geo.size);
+                    state.floating_size = Some(geo.size);
                 }
             });
-            self.pinnacle.space.map_element(window.clone(), loc, true);
         }
 
-        // TODO: do snapshot and transaction here BUT ONLY IF TILED AND ON ACTIVE TAG
-
         let output = window.output(&self.pinnacle);
 
         if let Some(output) = output.as_ref() {
@@ -110,15 +117,18 @@ impl XwmHandler for State {
         self.pinnacle.windows.push(window.clone());
         self.pinnacle.raise_window(window.clone(), true);
 
-        self.pinnacle.apply_window_rules(&window);
-
         if window.is_on_active_tag() {
             if let Some(output) = output {
                 output.with_state_mut(|state| state.focus_stack.set_focus(window.clone()));
                 self.update_keyboard_focus(&output);
 
-                self.pinnacle.begin_layout_transaction(&output);
+                if will_float {
-                self.pinnacle.request_layout(&output);
+                    self.pinnacle.set_window_floating(&window, true);
+                    self.pinnacle.space.map_element(window.clone(), loc, true);
+                } else {
+                    self.pinnacle.begin_layout_transaction(&output);
+                    self.pinnacle.request_layout(&output);
+                }
             }
         }
     }
@@ -184,7 +194,8 @@ impl XwmHandler for State {
         _reorder: Option<Reorder>,
     ) {
         trace!("XwmHandler::configure_request");
-        let floating_or_override_redirect = self
+        tracing::info!(?x, ?y, ?w, ?h);
+        let should_configure = self
             .pinnacle
             .windows
             .iter()
@@ -193,9 +204,11 @@ impl XwmHandler for State {
                 win.is_x11_override_redirect()
                     || win.with_state(|state| state.floating_or_tiled.is_floating())
             })
-            .unwrap_or(false);
+            .unwrap_or(true);
+        // If we unwrap_or here then the window hasn't requested a map yet.
+        // In that case, grant the configure. Xterm wants this to map properly, for example.
 
-        if floating_or_override_redirect {
+        if should_configure {
             let mut geo = window.geometry();
 
             if let Some(x) = x {
@@ -211,6 +224,8 @@ impl XwmHandler for State {
                 geo.size.h = h as i32;
             }
 
+            tracing::info!(?geo, "configure_request");
+
             if let Err(err) = window.configure(geo) {
                 error!("Failed to configure x11 win: {err}");
             }
@@ -248,7 +263,7 @@ impl XwmHandler for State {
             return;
         };
 
-        self.set_window_maximized(&window, true);
+        self.set_window_maximized_and_layout(&window, true);
     }
 
     fn unmaximize_request(&mut self, _xwm: XwmId, window: X11Surface) {
@@ -259,7 +274,7 @@ impl XwmHandler for State {
             return;
         };
 
-        self.set_window_maximized(&window, false);
+        self.set_window_maximized_and_layout(&window, false);
     }
 
     fn fullscreen_request(&mut self, _xwm: XwmId, window: X11Surface) {
@@ -270,7 +285,7 @@ impl XwmHandler for State {
             return;
         };
 
-        self.set_window_fullscreen(&window, true);
+        self.set_window_fullscreen_and_layout(&window, true);
     }
 
     fn unfullscreen_request(&mut self, _xwm: XwmId, window: X11Surface) {
@@ -281,7 +296,7 @@ impl XwmHandler for State {
             return;
         };
 
-        self.set_window_fullscreen(&window, true);
+        self.set_window_fullscreen_and_layout(&window, true);
     }
 
     fn resize_request(

src/layout.rs

@@ -17,10 +17,7 @@ use tracing::warn;
 use crate::{
     output::OutputName,
     state::{Pinnacle, State, WithState},
-    window::{
+    window::{window_state::FullscreenOrMaximized, WindowElement},
-        window_state::{FloatingOrTiled, FullscreenOrMaximized},
-        WindowElement,
-    },
 };
 
 use self::transaction::LayoutTransaction;
@@ -44,6 +41,13 @@ impl Pinnacle {
         });
 
         for win in to_unmap {
+            if win.with_state(|state| {
+                state.floating_or_tiled.is_floating() && state.fullscreen_or_maximized.is_neither()
+            }) {
+                if let Some(loc) = self.space.element_location(&win) {
+                    win.with_state_mut(|state| state.floating_loc = Some(loc.to_f64()));
+                }
+            }
             self.space.unmap_elem(&win);
         }
 
@@ -71,33 +75,33 @@ impl Pinnacle {
 
         for (win, geo) in zipped.by_ref() {
             win.change_geometry(geo.loc.to_f64(), geo.size);
+            self.space.map_element(win, geo.loc, false);
         }
 
         let (remaining_wins, _remaining_geos) = zipped.unzip::<_, _, Vec<_>, Vec<_>>();
 
         for win in remaining_wins {
             assert!(win.with_state(|state| state.floating_or_tiled.is_floating()));
-            win.toggle_floating();
+            self.set_window_floating(&win, true);
+            if let Some(toplevel) = win.toplevel() {
+                toplevel.send_pending_configure();
+            }
+            // TODO: will prolly need to map here
         }
 
         for window in windows_on_foc_tags.iter() {
             match window.with_state(|state| state.fullscreen_or_maximized) {
                 FullscreenOrMaximized::Fullscreen => {
                     window.change_geometry(output_geo.loc.to_f64(), output_geo.size);
+                    self.space
+                        .map_element(window.clone(), output_geo.loc, false);
                 }
                 FullscreenOrMaximized::Maximized => {
-                    window.change_geometry(
+                    let loc = output_geo.loc + non_exclusive_geo.loc;
-                        (output_geo.loc + non_exclusive_geo.loc).to_f64(),
+                    window.change_geometry(loc.to_f64(), non_exclusive_geo.size);
-                        non_exclusive_geo.size,
+                    self.space.map_element(window.clone(), loc, false);
-                    );
-                }
-                FullscreenOrMaximized::Neither => {
-                    if let FloatingOrTiled::Floating { loc, size } =
-                        window.with_state(|state| state.floating_or_tiled)
-                    {
-                        window.change_geometry(loc, size);
-                    }
                 }
+                FullscreenOrMaximized::Neither => (),
             }
         }
 
@@ -110,10 +114,17 @@ impl Pinnacle {
                 }
             }
 
-            // TODO: get rid of target_loc
+            let floating_loc = win
-            let loc = win.with_state_mut(|state| state.target_loc.take());
+                .with_state(|state| {
-            if let Some(loc) = loc {
+                    let should_map = state.floating_or_tiled.is_floating()
-                self.space.map_element(win.clone(), loc, false);
+                        && state.fullscreen_or_maximized.is_neither();
+
+                    should_map.then_some(state.floating_loc)
+                })
+                .flatten();
+            if let Some(loc) = floating_loc {
+                self.space
+                    .map_element(win.clone(), loc.to_i32_round(), false);
             }
         }
 

src/output.rs

@@ -22,7 +22,6 @@ use crate::{
     render::util::snapshot::OutputSnapshots,
     state::{Pinnacle, State, WithState},
     tag::Tag,
-    window::window_state::FloatingOrTiled,
 };
 
 /// A unique identifier for an output.
@@ -223,21 +222,17 @@ impl Pinnacle {
 
                 let output_loc = output.current_location();
 
-                // FIXME: get everything out of this with_state
+                let mut loc = self.space.element_location(&win).unwrap_or(output_loc);
-                win.with_state_mut(|state| {
-                    let FloatingOrTiled::Floating { loc, size: _ } = &mut state.floating_or_tiled
-                    else {
-                        unreachable!()
-                    };
 
-                    let mut loc_relative_to_output = *loc - output_loc.to_f64();
+                // FIXME: space maps in i32
-                    loc_relative_to_output = loc_relative_to_output.upscale(pos_multiplier);
+                let mut loc_relative_to_output = loc - output_loc;
+                loc_relative_to_output = loc_relative_to_output
+                    .to_f64()
+                    .upscale(pos_multiplier)
+                    .to_i32_round();
 
-                    *loc = loc_relative_to_output + output_loc.to_f64();
+                loc = loc_relative_to_output + output_loc;
-                    // FIXME: f64 -> i32
+                self.space.map_element(win.clone(), loc, false);
-                    self.space
-                        .map_element(win.clone(), loc.to_i32_round(), false);
-                });
             }
         }
 

src/window.rs

@@ -73,10 +73,10 @@ impl WindowElement {
             }
         }
 
-        self.with_state_mut(|state| {
+        // self.with_state_mut(|state| {
-            // FIXME: f64 -> i32, also remove target loc
+        //     // FIXME: f64 -> i32, also remove target loc
-            state.target_loc = Some(new_loc.to_i32_round());
+        //     state.target_loc = Some(new_loc.to_i32_round());
-        });
+        // });
     }
 
     /// Get this window's class (app id in Wayland but hey old habits die hard).

src/window/rules.rs

@@ -11,7 +11,7 @@ use smithay::{
 use crate::{
     handlers::decoration::KdeDecorationObject,
     state::{Pinnacle, WithState},
-    window::window_state,
+    window::window_state::FloatingOrTiled,
 };
 
 use super::WindowElement;
@@ -164,13 +164,6 @@ pub struct WindowRule {
     pub decoration_mode: Option<DecorationMode>,
 }
 
-// TODO: just skip serializing fields on the other FloatingOrTiled
-#[derive(Debug, Copy, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
-pub enum FloatingOrTiled {
-    Floating,
-    Tiled,
-}
-
 impl Pinnacle {
     pub fn apply_window_rules(&mut self, window: &WindowElement) {
         tracing::debug!("Applying window rules");
@@ -207,23 +200,14 @@ impl Pinnacle {
                     window.with_state_mut(|state| state.tags.clone_from(&tags));
                 }
 
-                if let Some(floating_or_tiled) = floating_or_tiled {
-                    match floating_or_tiled {
-                        FloatingOrTiled::Floating => {
-                            if window.with_state(|state| state.floating_or_tiled.is_tiled()) {
-                                window.toggle_floating();
-                            }
-                        }
-                        FloatingOrTiled::Tiled => {
-                            if window.with_state(|state| state.floating_or_tiled.is_floating()) {
-                                window.toggle_floating();
-                            }
-                        }
-                    }
-                }
-
                 if let Some(fs_or_max) = fullscreen_or_maximized {
-                    window.with_state_mut(|state| state.fullscreen_or_maximized = *fs_or_max);
+                    match fs_or_max {
+                        FullscreenOrMaximized::Neither => (), // TODO: is this branch needed?
+                        FullscreenOrMaximized::Fullscreen => {
+                            self.set_window_fullscreen(window, true)
+                        }
+                        FullscreenOrMaximized::Maximized => self.set_window_maximized(window, true),
+                    }
                 }
 
                 if let Some((w, h)) = size {
@@ -231,53 +215,20 @@ impl Pinnacle {
                     window_size.w = u32::from(*w) as i32;
                     window_size.h = u32::from(*h) as i32;
 
-                    match window.with_state(|state| state.floating_or_tiled) {
+                    window.with_state_mut(|state| {
-                        window_state::FloatingOrTiled::Floating { loc, mut size } => {
+                        state.floating_size = Some(window_size);
-                            size = (u32::from(*w) as i32, u32::from(*h) as i32).into();
+                    });
-                            window.with_state_mut(|state| {
-                                state.floating_or_tiled =
-                                    window_state::FloatingOrTiled::Floating { loc, size }
-                            });
-                        }
-                        window_state::FloatingOrTiled::Tiled(mut rect) => {
-                            if let Some((_, size)) = rect.as_mut() {
-                                *size = (u32::from(*w) as i32, u32::from(*h) as i32).into();
-                            }
-                            window.with_state_mut(|state| {
-                                state.floating_or_tiled = window_state::FloatingOrTiled::Tiled(rect)
-                            });
-                        }
-                    }
                 }
 
+                // FIXME: make this f64
                 if let Some(location) = location {
-                    match window.with_state(|state| state.floating_or_tiled) {
+                    window.with_state_mut(|state| {
-                        window_state::FloatingOrTiled::Floating { mut loc, size } => {
+                        state.floating_loc = Some(Point::from(*location).to_f64());
-                            // FIXME: make window rule f64
+                    });
-                            loc = Point::from(*location).to_f64();
+                }
-                            window.with_state_mut(|state| {
-                                state.floating_or_tiled =
-                                    window_state::FloatingOrTiled::Floating { loc, size }
-                            });
-                            // FIXME: space maps as i32
-                            self.space
-                                .map_element(window.clone(), loc.to_i32_round(), false);
-                        }
-                        window_state::FloatingOrTiled::Tiled(rect) => {
-                            // If the window is tiled, don't set the size. Instead, set
-                            // what the size will be when it gets set to floating.
-                            let rect = rect.unwrap_or_else(|| {
-                                let size = window.geometry().size;
-                                // FIXME: i32 -> f64
-                                (Point::from(*location).to_f64(), size)
-                            });
 
-                            window.with_state_mut(|state| {
+                if let Some(floating_or_tiled) = floating_or_tiled {
-                                state.floating_or_tiled =
+                    window.with_state_mut(|state| state.floating_or_tiled = *floating_or_tiled);
-                                    window_state::FloatingOrTiled::Tiled(Some(rect))
-                            });
-                        }
-                    }
                 }
 
                 if let Some(decoration_mode) = decoration_mode {

src/window/window_state.rs

@@ -8,6 +8,7 @@ use smithay::{
     utils::{Logical, Point, Serial, Size},
     wayland::compositor::HookId,
 };
+use tracing::warn;
 
 use crate::{
     layout::transaction::LayoutSnapshot,
@@ -61,133 +62,11 @@ pub struct WindowElementState {
     pub snapshot: Option<LayoutSnapshot>,
     pub snapshot_hook_id: Option<HookId>,
     pub decoration_mode: Option<DecorationMode>,
+    pub floating_loc: Option<Point<f64, Logical>>,
+    pub floating_size: Option<Size<i32, Logical>>,
 }
 
 impl WindowElement {
-    /// 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 { loc, size } => {
-                self.with_state_mut(|state| {
-                    state.floating_or_tiled = FloatingOrTiled::Tiled(Some((loc, size)))
-                });
-                self.set_tiled_states();
-            }
-            FloatingOrTiled::Tiled(prev_rect) => {
-                // FIXME: is using window geometry here right?
-                let (prev_loc, prev_size) = prev_rect.unwrap_or_else(|| {
-                    let geo = self.geometry();
-                    (geo.loc.to_f64(), geo.size)
-                });
-
-                self.with_state_mut(|state| {
-                    state.floating_or_tiled = FloatingOrTiled::Floating {
-                        loc: prev_loc,
-                        size: prev_size,
-                    };
-                });
-
-                // TODO: maybe move this into update_windows
-                self.change_geometry(prev_loc, prev_size);
-                self.set_floating_states();
-            }
-        }
-    }
-
-    /// 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 => {
-                self.with_state_mut(|state| {
-                    state.fullscreen_or_maximized = FullscreenOrMaximized::Fullscreen;
-                });
-
-                match self.underlying_surface() {
-                    WindowSurface::Wayland(toplevel) => {
-                        toplevel.with_pending_state(|state| {
-                            state.states.unset(xdg_toplevel::State::Maximized);
-                            state.states.set(xdg_toplevel::State::Fullscreen);
-                            state.states.set(xdg_toplevel::State::TiledTop);
-                            state.states.set(xdg_toplevel::State::TiledLeft);
-                            state.states.set(xdg_toplevel::State::TiledBottom);
-                            state.states.set(xdg_toplevel::State::TiledRight);
-                        });
-                    }
-                    WindowSurface::X11(surface) => {
-                        if !surface.is_override_redirect() {
-                            surface
-                                .set_maximized(false)
-                                .expect("failed to set x11 win to maximized");
-                            surface
-                                .set_fullscreen(true)
-                                .expect("failed to set x11 win to not fullscreen");
-                        }
-                    }
-                }
-            }
-            FullscreenOrMaximized::Fullscreen => {
-                self.with_state_mut(|state| {
-                    state.fullscreen_or_maximized = FullscreenOrMaximized::Neither;
-                });
-
-                match self.with_state(|state| state.floating_or_tiled) {
-                    FloatingOrTiled::Floating { loc, size } => {
-                        self.change_geometry(loc, size);
-                        self.set_floating_states();
-                    }
-                    FloatingOrTiled::Tiled(_) => self.set_tiled_states(),
-                }
-            }
-        }
-    }
-
-    /// 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 => {
-                self.with_state_mut(|state| {
-                    state.fullscreen_or_maximized = FullscreenOrMaximized::Maximized;
-                });
-
-                match self.underlying_surface() {
-                    WindowSurface::Wayland(toplevel) => {
-                        toplevel.with_pending_state(|state| {
-                            state.states.set(xdg_toplevel::State::Maximized);
-                            state.states.unset(xdg_toplevel::State::Fullscreen);
-                            state.states.set(xdg_toplevel::State::TiledTop);
-                            state.states.set(xdg_toplevel::State::TiledLeft);
-                            state.states.set(xdg_toplevel::State::TiledBottom);
-                            state.states.set(xdg_toplevel::State::TiledRight);
-                        });
-                    }
-                    WindowSurface::X11(surface) => {
-                        if !surface.is_override_redirect() {
-                            surface
-                                .set_maximized(true)
-                                .expect("failed to set x11 win to maximized");
-                            surface
-                                .set_fullscreen(false)
-                                .expect("failed to set x11 win to not fullscreen");
-                        }
-                    }
-                }
-            }
-            FullscreenOrMaximized::Maximized => {
-                self.with_state_mut(|state| {
-                    state.fullscreen_or_maximized = FullscreenOrMaximized::Neither;
-                });
-
-                match self.with_state(|state| state.floating_or_tiled) {
-                    FloatingOrTiled::Floating { loc, size } => {
-                        self.change_geometry(loc, size);
-                        self.set_floating_states();
-                    }
-                    FloatingOrTiled::Tiled(_) => self.set_tiled_states(),
-                }
-            }
-        }
-    }
-
     /// Unsets maximized and fullscreen states for both wayland and xwayland windows
     /// and unsets tiled states for wayland windows.
     fn set_floating_states(&self) {
@@ -243,19 +122,181 @@ impl WindowElement {
     }
 }
 
+impl Pinnacle {
+    pub fn set_window_floating(&self, window: &WindowElement, floating: bool) {
+        // If the window is fullscreen or maximized, simply mark it as floating or tiled
+        // and don't set floating or tiled states to prevent stuff like decorations
+        // appearing in fullscreen mode.
+        if window.with_state(|state| !state.fullscreen_or_maximized.is_neither()) {
+            window.with_state_mut(|state| {
+                state.floating_or_tiled = match floating {
+                    true => FloatingOrTiled::Floating,
+                    false => FloatingOrTiled::Tiled,
+                }
+            });
+            return;
+        }
+
+        if floating {
+            let size = window
+                .with_state(|state| state.floating_size)
+                .unwrap_or_else(|| window.geometry().size);
+            let loc = window
+                .with_state(|state| state.floating_loc)
+                .or_else(|| self.space.element_location(window).map(|loc| loc.to_f64()))
+                .or_else(|| {
+                    self.focused_output().map(|op| {
+                        let op_geo = self
+                            .space
+                            .output_geometry(op)
+                            .expect("focused output wasn't mapped");
+
+                        let x = op_geo.loc.x + op_geo.size.w / 2 - (size.w / 2);
+                        let y = op_geo.loc.y + op_geo.size.h / 2 - (size.h / 2);
+
+                        (x as f64, y as f64).into()
+                    })
+                })
+                .unwrap_or_default();
+
+            window.with_state_mut(|state| {
+                state.floating_size = Some(size);
+                state.floating_loc = Some(loc);
+                state.floating_or_tiled = FloatingOrTiled::Floating;
+            });
+
+            window.change_geometry(loc, size);
+            window.set_floating_states();
+        } else {
+            let geo = self.space.element_geometry(window);
+
+            window.with_state_mut(|state| {
+                if let Some(geo) = geo {
+                    state.floating_size.replace(geo.size);
+                    state.floating_loc.replace(geo.loc.to_f64()); // FIXME: i32 -> f64
+                }
+                state.floating_or_tiled = FloatingOrTiled::Tiled;
+            });
+            window.set_tiled_states();
+        }
+    }
+
+    pub fn set_window_maximized(&self, window: &WindowElement, maximized: bool) {
+        if maximized {
+            // We only want to update the stored floating geometry when exiting floating mode.
+            if window.with_state(|state| {
+                state.floating_or_tiled.is_floating() && state.fullscreen_or_maximized.is_neither()
+            }) {
+                let geo = self.space.element_geometry(window);
+
+                if let Some(geo) = geo {
+                    window.with_state_mut(|state| {
+                        state.floating_size.replace(geo.size);
+                        state.floating_loc.replace(geo.loc.to_f64()); // FIXME: i32 -> f64
+                    });
+                }
+            }
+
+            window.with_state_mut(|state| {
+                state.fullscreen_or_maximized = FullscreenOrMaximized::Maximized;
+            });
+
+            match window.underlying_surface() {
+                WindowSurface::Wayland(toplevel) => {
+                    toplevel.with_pending_state(|state| {
+                        state.states.set(xdg_toplevel::State::Maximized);
+                        state.states.unset(xdg_toplevel::State::Fullscreen);
+                        state.states.set(xdg_toplevel::State::TiledTop);
+                        state.states.set(xdg_toplevel::State::TiledLeft);
+                        state.states.set(xdg_toplevel::State::TiledBottom);
+                        state.states.set(xdg_toplevel::State::TiledRight);
+                    });
+                }
+                WindowSurface::X11(surface) => {
+                    if !surface.is_override_redirect() {
+                        if let Err(err) = surface.set_maximized(true) {
+                            warn!("Failed to set xwayland window to maximized: {err}");
+                        }
+                        if let Err(err) = surface.set_fullscreen(false) {
+                            warn!("Failed to unset xwayland window fullscreen: {err}");
+                        }
+                    }
+                }
+            }
+        } else {
+            window.with_state_mut(|state| {
+                state.fullscreen_or_maximized = FullscreenOrMaximized::Neither;
+            });
+
+            match window.with_state(|state| state.floating_or_tiled) {
+                FloatingOrTiled::Floating => self.set_window_floating(window, true),
+                FloatingOrTiled::Tiled => window.set_tiled_states(),
+            }
+        }
+    }
+
+    pub fn set_window_fullscreen(&self, window: &WindowElement, fullscreen: bool) {
+        if fullscreen {
+            // We only want to update the stored floating geometry when exiting floating mode.
+            if window.with_state(|state| {
+                state.floating_or_tiled.is_floating() && state.fullscreen_or_maximized.is_neither()
+            }) {
+                let geo = self.space.element_geometry(window);
+
+                if let Some(geo) = geo {
+                    window.with_state_mut(|state| {
+                        state.floating_size.replace(geo.size);
+                        state.floating_loc.replace(geo.loc.to_f64()); // FIXME: i32 -> f64
+                    });
+                }
+            }
+
+            window.with_state_mut(|state| {
+                state.fullscreen_or_maximized = FullscreenOrMaximized::Fullscreen;
+            });
+
+            match window.underlying_surface() {
+                WindowSurface::Wayland(toplevel) => {
+                    toplevel.with_pending_state(|state| {
+                        state.states.unset(xdg_toplevel::State::Maximized);
+                        state.states.set(xdg_toplevel::State::Fullscreen);
+                        state.states.set(xdg_toplevel::State::TiledTop);
+                        state.states.set(xdg_toplevel::State::TiledLeft);
+                        state.states.set(xdg_toplevel::State::TiledBottom);
+                        state.states.set(xdg_toplevel::State::TiledRight);
+                    });
+                }
+                WindowSurface::X11(surface) => {
+                    if !surface.is_override_redirect() {
+                        if let Err(err) = surface.set_maximized(false) {
+                            warn!("Failed to unset xwayland window maximized: {err}");
+                        }
+                        if let Err(err) = surface.set_fullscreen(true) {
+                            warn!("Failed to set xwayland window to fullscreen: {err}");
+                        }
+                    }
+                }
+            }
+        } else {
+            window.with_state_mut(|state| {
+                state.fullscreen_or_maximized = FullscreenOrMaximized::Neither;
+            });
+
+            match window.with_state(|state| state.floating_or_tiled) {
+                FloatingOrTiled::Floating => self.set_window_floating(window, true),
+                FloatingOrTiled::Tiled => window.set_tiled_states(),
+            }
+        }
+    }
+}
+
 /// Whether a window is floating or tiled
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum FloatingOrTiled {
-    /// The window is floating with the specified geometry.
+    /// The window is floating.
-    Floating {
+    Floating,
-        loc: Point<f64, Logical>,
-        size: Size<i32, Logical>,
-    },
     /// The window is tiled.
-    ///
+    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<(Point<f64, Logical>, Size<i32, Logical>)>),
 }
 
 impl FloatingOrTiled {
@@ -264,7 +305,7 @@ impl FloatingOrTiled {
     /// [`Floating`]: FloatingOrTiled::Floating
     #[must_use]
     pub fn is_floating(&self) -> bool {
-        matches!(self, Self::Floating { .. })
+        matches!(self, Self::Floating)
     }
 
     /// Returns `true` if the floating or tiled is [`Tiled`].
@@ -272,7 +313,7 @@ impl FloatingOrTiled {
     /// [`Tiled`]: FloatingOrTiled::Tiled
     #[must_use]
     pub fn is_tiled(&self) -> bool {
-        matches!(self, Self::Tiled(..))
+        matches!(self, Self::Tiled)
     }
 }
 
@@ -313,9 +354,10 @@ impl WindowElementState {
     pub fn new() -> Self {
         Self {
             id: WindowId::next(),
-            // loc_request_state: LocationRequestState::Idle,
             tags: vec![],
-            floating_or_tiled: FloatingOrTiled::Tiled(None),
+            floating_or_tiled: FloatingOrTiled::Tiled,
+            floating_loc: None,
+            floating_size: None,
             fullscreen_or_maximized: FullscreenOrMaximized::Neither,
             target_loc: None,
             minimized: false,

wlcs_pinnacle/src/main_loop.rs

@@ -3,7 +3,6 @@ use std::{path::PathBuf, sync::Arc, time::Duration};
 use pinnacle::{
     state::{ClientState, State, WithState},
     tag::TagId,
-    window::window_state::FloatingOrTiled,
 };
 use smithay::{
     backend::input::{ButtonState, DeviceCapability, InputEvent},
@@ -119,29 +118,17 @@ fn handle_event(event: WlcsEvent, state: &mut State) {
                 .cloned();
 
             if let Some(window) = window {
+                window.with_state_mut(|state| {
+                    state.floating_loc = Some(location.to_f64());
+                });
+
+                state.pinnacle.set_window_floating(&window, true);
+
                 state
                     .pinnacle
                     .space
                     .map_element(window.clone(), location, false);
 
-                let size = state
-                    .pinnacle
-                    .space
-                    .element_geometry(&window)
-                    .expect("window to be positioned was not mapped")
-                    .size;
-
-                if window.with_state(|state| state.floating_or_tiled.is_tiled()) {
-                    window.toggle_floating();
-                }
-
-                window.with_state_mut(|state| {
-                    state.floating_or_tiled = FloatingOrTiled::Floating {
-                        loc: location.to_f64(),
-                        size,
-                    };
-                });
-
                 for output in state.pinnacle.space.outputs_for_element(&window) {
                     state.schedule_render(&output);
                 }