layout: Refactor window opening targets

niri-visual-tests/src/cases/layout.rs

@@ -3,7 +3,7 @@ use std::time::Duration;
 
 use niri::animation::Clock;
 use niri::layout::scrolling::ColumnWidth;
-use niri::layout::{ActivateWindow, LayoutElement as _, Options};
+use niri::layout::{ActivateWindow, AddWindowTarget, LayoutElement as _, Options};
 use niri::render_helpers::RenderTarget;
 use niri_config::{Color, FloatOrInt, OutputName};
 use smithay::backend::renderer::element::RenderElement;
@@ -170,6 +170,7 @@ impl Layout {
 
         self.layout.add_window(
             window.clone(),
+            AddWindowTarget::Auto,
             width,
             false,
             false,
@@ -194,8 +195,14 @@ impl Layout {
         );
         window.communicate();
 
-        self.layout
+        self.layout.add_window(
-            .add_window_right_of(right_of.id(), window.clone(), width, false, false);
+            window.clone(),
+            AddWindowTarget::NextTo(right_of.id()),
+            width,
+            false,
+            false,
+            ActivateWindow::default(),
+        );
         self.windows.push(window);
     }
 

src/handlers/compositor.rs

@@ -19,7 +19,7 @@ use smithay::{delegate_compositor, delegate_shm};
 
 use super::xdg_shell::add_mapped_toplevel_pre_commit_hook;
 use crate::handlers::XDG_ACTIVATION_TOKEN_TIMEOUT;
-use crate::layout::ActivateWindow;
+use crate::layout::{ActivateWindow, AddWindowTarget};
 use crate::niri::{ClientState, State};
 use crate::utils::send_scale_transform;
 use crate::utils::transaction::Transaction;
@@ -96,7 +96,7 @@ impl CompositorHandler for State {
 
                     let toplevel = window.toplevel().expect("no X11 support");
 
-                    let (rules, width, is_full_width, output, workspace_name) =
+                    let (rules, width, is_full_width, output, workspace_id) =
                         if let InitialConfigureState::Configured {
                             rules,
                             width,
@@ -110,10 +110,12 @@ impl CompositorHandler for State {
                                 output.filter(|o| self.niri.layout.monitor_for_output(o).is_some());
 
                             // Check that the workspace still exists.
-                            let workspace_name = workspace_name
+                            let workspace_id = workspace_name
-                                .filter(|n| self.niri.layout.find_workspace_by_name(n).is_some());
+                                .as_deref()
+                                .and_then(|n| self.niri.layout.find_workspace_by_name(n))
+                                .map(|(_, ws)| ws.id());
 
-                            (rules, width, is_full_width, output, workspace_name)
+                            (rules, width, is_full_width, output, workspace_id)
                         } else {
                             error!("window map must happen after initial configure");
                             (ResolvedWindowRules::empty(), None, false, None, None)
@@ -160,46 +162,24 @@ impl CompositorHandler for State {
                         }
                     };
 
-                    let output = if let Some(p) = parent {
+                    let target = if let Some(p) = &parent {
-                        // Open dialogs immediately to the right of their parent window.
+                        // Open dialogs next to their parent window.
-                        //
+                        AddWindowTarget::NextTo(p)
-                        // FIXME: do we want to use activate here? How do we want things to behave
+                    } else if let Some(id) = workspace_id {
-                        // exactly?
+                        AddWindowTarget::Workspace(id)
-                        self.niri.layout.add_window_right_of(
-                            &p,
-                            mapped,
-                            width,
-                            is_full_width,
-                            is_floating,
-                        )
-                    } else if let Some(workspace_name) = &workspace_name {
-                        self.niri.layout.add_window_to_named_workspace(
-                            workspace_name,
-                            mapped,
-                            width,
-                            is_full_width,
-                            is_floating,
-                            activate,
-                        )
                     } else if let Some(output) = &output {
-                        self.niri.layout.add_window_on_output(
+                        AddWindowTarget::Output(output)
-                            output,
-                            mapped,
-                            width,
-                            is_full_width,
-                            is_floating,
-                            activate,
-                        );
-                        Some(output)
                     } else {
-                        self.niri.layout.add_window(
+                        AddWindowTarget::Auto
-                            mapped,
-                            width,
-                            is_full_width,
-                            is_floating,
-                            activate,
-                        )
                     };
+                    let output = self.niri.layout.add_window(
+                        mapped,
+                        target,
+                        width,
+                        is_full_width,
+                        is_floating,
+                        activate,
+                    );
 
                     if let Some(output) = output.cloned() {
                         self.niri.layout.start_open_animation_for_window(&window);

src/handlers/xdg_shell.rs

@@ -859,7 +859,7 @@ impl State {
                 });
             }
 
-            width = ws.resolve_default_width(rules.default_width);
+            width = ws.resolve_default_width(rules.default_width, is_floating);
 
             let configure_width = if is_full_width {
                 Some(ColumnWidth::Proportion(1.))

src/layout/floating.rs

@@ -428,10 +428,7 @@ impl<W: LayoutElement> FloatingSpace<W> {
         self.bring_up_descendants_of(idx);
     }
 
-    pub fn add_tile_above(&mut self, above: &W::Id, mut tile: Tile<W>) {
+    pub fn add_tile_above(&mut self, above: &W::Id, mut tile: Tile<W>, activate: bool) {
-        // Activate the new window if above was active.
-        let activate = Some(above) == self.active_window_id.as_ref();
-
         let idx = self.idx_of(above).unwrap();
 
         let above_pos = self.data[idx].logical_pos;

src/layout/mod.rs

@@ -35,6 +35,7 @@ use std::mem;
 use std::rc::Rc;
 use std::time::Duration;
 
+use monitor::MonitorAddWindowTarget;
 use niri_config::{
     CenterFocusedColumn, Config, CornerRadius, FloatOrInt, PresetSize, Struts,
     Workspace as WorkspaceConfig,
@@ -48,7 +49,7 @@ use smithay::output::{self, Output};
 use smithay::reexports::wayland_server::protocol::wl_surface::WlSurface;
 use smithay::utils::{Logical, Point, Rectangle, Scale, Serial, Size, Transform};
 use tile::{Tile, TileRenderElement};
-use workspace::WorkspaceId;
+use workspace::{WorkspaceAddWindowTarget, WorkspaceId};
 
 pub use self::monitor::MonitorRenderElement;
 use self::monitor::{Monitor, WorkspaceSwitch};
@@ -424,6 +425,20 @@ pub enum ActivateWindow {
     No,
 }
 
+/// Where to put a newly added window.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub enum AddWindowTarget<'a, W: LayoutElement> {
+    /// No particular preference.
+    #[default]
+    Auto,
+    /// On this output.
+    Output(&'a Output),
+    /// On this workspace.
+    Workspace(WorkspaceId),
+    /// Next to this existing window.
+    NextTo(&'a W::Id),
+}
+
 impl<W: LayoutElement> InteractiveMoveState<W> {
     fn moving(&self) -> Option<&InteractiveMoveData<W>> {
         match self {
@@ -797,71 +812,6 @@ impl<W: LayoutElement> Layout<W> {
         }
     }
 
-    /// Adds a new window to the layout on a specific workspace.
-    pub fn add_window_to_named_workspace(
-        &mut self,
-        workspace_name: &str,
-        window: W,
-        width: Option<ColumnWidth>,
-        is_full_width: bool,
-        is_floating: bool,
-        activate: ActivateWindow,
-    ) -> Option<&Output> {
-        let width = self.resolve_default_width(&window, width);
-
-        match &mut self.monitor_set {
-            MonitorSet::Normal {
-                monitors,
-                active_monitor_idx,
-                ..
-            } => {
-                let (mon_idx, mon, ws_idx) = monitors
-                    .iter_mut()
-                    .enumerate()
-                    .find_map(|(mon_idx, mon)| {
-                        mon.find_named_workspace_index(workspace_name)
-                            .map(move |ws_idx| (mon_idx, mon, ws_idx))
-                    })
-                    .unwrap();
-
-                if activate == ActivateWindow::Yes {
-                    *active_monitor_idx = mon_idx;
-                }
-
-                let activate = activate.map_smart(|| {
-                    // Don't steal focus from an active fullscreen window.
-                    let ws = &mon.workspaces[ws_idx];
-                    if mon_idx == *active_monitor_idx && ws.is_active_fullscreen() {
-                        return false;
-                    }
-
-                    // Don't activate if on a different workspace.
-                    if mon.active_workspace_idx != ws_idx {
-                        return false;
-                    }
-
-                    true
-                });
-
-                mon.add_window(ws_idx, window, activate, width, is_full_width, is_floating);
-                Some(&mon.output)
-            }
-            MonitorSet::NoOutputs { workspaces } => {
-                let ws = workspaces
-                    .iter_mut()
-                    .find(|ws| {
-                        ws.name
-                            .as_ref()
-                            .map_or(false, |name| name.eq_ignore_ascii_case(workspace_name))
-                    })
-                    .unwrap();
-                let activate = activate.map_smart(|| true);
-                ws.add_window(window, activate, width, is_full_width, is_floating);
-                None
-            }
-        }
-    }
-
     pub fn add_column_by_idx(
         &mut self,
         monitor_idx: usize,
@@ -891,12 +841,13 @@ impl<W: LayoutElement> Layout<W> {
     pub fn add_window(
         &mut self,
         window: W,
+        target: AddWindowTarget<W>,
         width: Option<ColumnWidth>,
         is_full_width: bool,
         is_floating: bool,
         activate: ActivateWindow,
     ) -> Option<&Output> {
-        let width = self.resolve_default_width(&window, width);
+        let resolved_width = self.resolve_default_width(&window, width, is_floating);
 
         match &mut self.monitor_set {
             MonitorSet::Normal {
@@ -904,144 +855,138 @@ impl<W: LayoutElement> Layout<W> {
                 active_monitor_idx,
                 ..
             } => {
-                let mon = &mut monitors[*active_monitor_idx];
+                let (mon_idx, target) = match target {
+                    AddWindowTarget::Auto => (*active_monitor_idx, MonitorAddWindowTarget::Auto),
+                    AddWindowTarget::Output(output) => {
+                        let mon_idx = monitors
+                            .iter()
+                            .position(|mon| mon.output == *output)
+                            .unwrap();
 
-                let activate = activate.map_smart(|| {
+                        (mon_idx, MonitorAddWindowTarget::Auto)
-                    // Don't steal focus from an active fullscreen window.
+                    }
-                    let ws = &mon.workspaces[mon.active_workspace_idx];
+                    AddWindowTarget::Workspace(ws_id) => {
-                    !ws.is_active_fullscreen()
+                        let mon_idx = monitors
-                });
+                            .iter()
+                            .position(|mon| mon.workspaces.iter().any(|ws| ws.id() == ws_id))
+                            .unwrap();
+
+                        (
+                            mon_idx,
+                            MonitorAddWindowTarget::Workspace {
+                                id: ws_id,
+                                column_idx: None,
+                            },
+                        )
+                    }
+                    AddWindowTarget::NextTo(next_to) => {
+                        if let Some(output) = self
+                            .interactive_move
+                            .as_ref()
+                            .and_then(|move_| {
+                                if let InteractiveMoveState::Moving(move_) = move_ {
+                                    Some(move_)
+                                } else {
+                                    None
+                                }
+                            })
+                            .filter(|move_| next_to == move_.tile.window().id())
+                            .map(|move_| move_.output.clone())
+                        {
+                            // The next_to window is being interactively moved.
+                            let mon_idx = monitors
+                                .iter()
+                                .position(|mon| mon.output == output)
+                                .unwrap_or(*active_monitor_idx);
+
+                            (mon_idx, MonitorAddWindowTarget::Auto)
+                        } else {
+                            let mon_idx = monitors
+                                .iter()
+                                .position(|mon| {
+                                    mon.workspaces.iter().any(|ws| ws.has_window(next_to))
+                                })
+                                .unwrap();
+                            (mon_idx, MonitorAddWindowTarget::NextTo(next_to))
+                        }
+                    }
+                };
+                let mon = &mut monitors[mon_idx];
 
                 mon.add_window(
-                    mon.active_workspace_idx,
                     window,
+                    target,
                     activate,
-                    width,
+                    resolved_width,
                     is_full_width,
                     is_floating,
                 );
-                Some(&mon.output)
-            }
-            MonitorSet::NoOutputs { workspaces } => {
-                let ws = if let Some(ws) = workspaces.get_mut(0) {
-                    ws
-                } else {
-                    workspaces.push(Workspace::new_no_outputs(
-                        self.clock.clone(),
-                        self.options.clone(),
-                    ));
-                    &mut workspaces[0]
-                };
-                let activate = activate.map_smart(|| true);
-                ws.add_window(window, activate, width, is_full_width, is_floating);
-                None
-            }
-        }
-    }
 
-    /// Adds a new window to the layout immediately to the right of another window.
+                if activate.map_smart(|| false) {
-    ///
+                    *active_monitor_idx = mon_idx;
-    /// If that another window was active, activates the new window.
-    ///
-    /// Returns an output that the window was added to, if there were any outputs.
-    pub fn add_window_right_of(
-        &mut self,
-        right_of: &W::Id,
-        window: W,
-        width: Option<ColumnWidth>,
-        is_full_width: bool,
-        is_floating: bool,
-    ) -> Option<&Output> {
-        if let Some(InteractiveMoveState::Moving(move_)) = &self.interactive_move {
-            if right_of == move_.tile.window().id() {
-                let output = move_.output.clone();
-                let activate = ActivateWindow::default();
-                if self.monitor_for_output(&output).is_some() {
-                    self.add_window_on_output(
-                        &output,
-                        window,
-                        width,
-                        is_full_width,
-                        is_floating,
-                        activate,
-                    );
-                    return Some(&self.monitor_for_output(&output).unwrap().output);
-                } else {
-                    return self.add_window(window, width, is_full_width, is_floating, activate);
                 }
-            }
-        }
 
-        let width = self.resolve_default_width(&window, width);
-
-        match &mut self.monitor_set {
-            MonitorSet::Normal { monitors, .. } => {
-                let mon = monitors
-                    .iter_mut()
-                    .find(|mon| mon.workspaces.iter().any(|ws| ws.has_window(right_of)))
-                    .unwrap();
-
-                mon.add_window_right_of(right_of, window, width, is_full_width, is_floating);
                 Some(&mon.output)
             }
             MonitorSet::NoOutputs { workspaces } => {
-                let ws = workspaces
+                let (ws_idx, target) = match target {
-                    .iter_mut()
+                    AddWindowTarget::Auto => {
-                    .find(|ws| ws.has_window(right_of))
+                        if workspaces.is_empty() {
-                    .unwrap();
+                            workspaces.push(Workspace::new_no_outputs(
-                ws.add_window_right_of(right_of, window, width, is_full_width, is_floating);
+                                self.clock.clone(),
+                                self.options.clone(),
+                            ));
+                        }
+
+                        (0, WorkspaceAddWindowTarget::Auto)
+                    }
+                    AddWindowTarget::Output(_) => panic!(),
+                    AddWindowTarget::Workspace(ws_id) => {
+                        let ws_idx = workspaces.iter().position(|ws| ws.id() == ws_id).unwrap();
+                        (ws_idx, WorkspaceAddWindowTarget::Auto)
+                    }
+                    AddWindowTarget::NextTo(next_to) => {
+                        if self
+                            .interactive_move
+                            .as_ref()
+                            .and_then(|move_| {
+                                if let InteractiveMoveState::Moving(move_) = move_ {
+                                    Some(move_)
+                                } else {
+                                    None
+                                }
+                            })
+                            .filter(|move_| next_to == move_.tile.window().id())
+                            .is_some()
+                        {
+                            // The next_to window is being interactively moved.
+                            (0, WorkspaceAddWindowTarget::Auto)
+                        } else {
+                            let ws_idx = workspaces
+                                .iter()
+                                .position(|ws| ws.has_window(next_to))
+                                .unwrap();
+                            (ws_idx, WorkspaceAddWindowTarget::NextTo(next_to))
+                        }
+                    }
+                };
+                let ws = &mut workspaces[ws_idx];
+
+                let tile = ws.make_tile(window);
+                ws.add_tile(
+                    tile,
+                    target,
+                    activate,
+                    resolved_width,
+                    is_full_width,
+                    is_floating,
+                );
+
                 None
             }
         }
     }
 
-    /// Adds a new window to the layout on a specific output.
-    pub fn add_window_on_output(
-        &mut self,
-        output: &Output,
-        window: W,
-        width: Option<ColumnWidth>,
-        is_full_width: bool,
-        is_floating: bool,
-        activate: ActivateWindow,
-    ) {
-        let width = self.resolve_default_width(&window, width);
-
-        let MonitorSet::Normal {
-            monitors,
-            active_monitor_idx,
-            ..
-        } = &mut self.monitor_set
-        else {
-            panic!()
-        };
-
-        let (mon_idx, mon) = monitors
-            .iter_mut()
-            .enumerate()
-            .find(|(_, mon)| mon.output == *output)
-            .unwrap();
-
-        if activate == ActivateWindow::Yes {
-            *active_monitor_idx = mon_idx;
-        }
-
-        let activate = activate.map_smart(|| {
-            // Don't steal focus from an active fullscreen window.
-            let ws = &mon.workspaces[mon.active_workspace_idx];
-            mon_idx != *active_monitor_idx || !ws.is_active_fullscreen()
-        });
-
-        mon.add_window(
-            mon.active_workspace_idx,
-            window,
-            activate,
-            width,
-            is_full_width,
-            is_floating,
-        );
-    }
-
     pub fn remove_window(
         &mut self,
         window: &W::Id,
@@ -2801,12 +2746,18 @@ impl<W: LayoutElement> Layout<W> {
             if mon_idx == new_idx && ws_idx == workspace_idx {
                 return;
             }
+            let ws_id = monitors[new_idx].workspaces[workspace_idx].id();
 
             let mon = &mut monitors[mon_idx];
             let activate = window.map_or(true, |win| {
                 mon_idx == *active_monitor_idx
                     && mon.active_window().map(|win| win.id()) == Some(win)
             });
+            let activate = if activate {
+                ActivateWindow::Yes
+            } else {
+                ActivateWindow::No
+            };
 
             let ws = &mut mon.workspaces[ws_idx];
             let transaction = Transaction::new();
@@ -2821,19 +2772,18 @@ impl<W: LayoutElement> Layout<W> {
             removed.tile.stop_move_animations();
 
             let mon = &mut monitors[new_idx];
-            if removed.is_floating {
+            mon.add_tile(
-                mon.add_floating_tile(workspace_idx, removed.tile, activate);
+                removed.tile,
-            } else {
+                MonitorAddWindowTarget::Workspace {
-                mon.add_tile(
+                    id: ws_id,
-                    workspace_idx,
+                    column_idx: None,
-                    None,
+                },
-                    removed.tile,
+                activate,
-                    activate,
+                removed.width,
-                    removed.width,
+                removed.is_full_width,
-                    removed.is_full_width,
+                removed.is_floating,
-                );
+            );
-            }
+            if activate.map_smart(|| false) {
-            if activate {
                 *active_monitor_idx = new_idx;
             }
 
@@ -3443,13 +3393,17 @@ impl<W: LayoutElement> Layout<W> {
 
                 match position {
                     InsertPosition::NewColumn(column_idx) => {
+                        let ws_id = mon.workspaces[ws_idx].id();
                         mon.add_tile(
-                            ws_idx,
-                            Some(column_idx),
                             move_.tile,
-                            true,
+                            MonitorAddWindowTarget::Workspace {
+                                id: ws_id,
+                                column_idx: Some(column_idx),
+                            },
+                            ActivateWindow::Yes,
                             move_.width,
                             move_.is_full_width,
+                            false,
                         );
                     }
                     InsertPosition::InColumn(column_idx, tile_idx) => {
@@ -3474,7 +3428,18 @@ impl<W: LayoutElement> Layout<W> {
                             tile.floating_window_size = Some(size);
                         }
 
-                        mon.add_floating_tile(ws_idx, tile, true);
+                        let ws_id = mon.workspaces[ws_idx].id();
+                        mon.add_tile(
+                            tile,
+                            MonitorAddWindowTarget::Workspace {
+                                id: ws_id,
+                                column_idx: None,
+                            },
+                            ActivateWindow::Yes,
+                            move_.width,
+                            move_.is_full_width,
+                            true,
+                        );
                     }
                 }
 
@@ -3490,18 +3455,23 @@ impl<W: LayoutElement> Layout<W> {
                 tile.animate_move_from(window_render_loc - new_window_render_loc);
             }
             MonitorSet::NoOutputs { workspaces, .. } => {
-                let ws = if let Some(ws) = workspaces.get_mut(0) {
+                if workspaces.is_empty() {
-                    ws
-                } else {
                     workspaces.push(Workspace::new_no_outputs(
                         self.clock.clone(),
                         self.options.clone(),
                     ));
-                    &mut workspaces[0]
+                }
-                };
+                let ws = &mut workspaces[0];
 
                 // No point in trying to use the pointer position without outputs.
-                ws.add_tile(None, move_.tile, true, move_.width, move_.is_full_width);
+                ws.add_tile(
+                    move_.tile,
+                    WorkspaceAddWindowTarget::Auto,
+                    ActivateWindow::Yes,
+                    move_.width,
+                    move_.is_full_width,
+                    move_.is_floating,
+                );
             }
         }
     }
@@ -3894,8 +3864,19 @@ impl<W: LayoutElement> Layout<W> {
         self.windows().any(|(_, win)| win.id() == window)
     }
 
-    fn resolve_default_width(&self, window: &W, width: Option<ColumnWidth>) -> ColumnWidth {
+    fn resolve_default_width(
+        &self,
+        window: &W,
+        width: Option<ColumnWidth>,
+        is_floating: bool,
+    ) -> ColumnWidth {
         let mut width = width.unwrap_or_else(|| ColumnWidth::Fixed(f64::from(window.size().w)));
+        if is_floating {
+            return width;
+        }
+
+        // Add border width to account for the issue that the scrolling layout currently doesn't
+        // take borders into account for fixed sizes.
         if let ColumnWidth::Fixed(w) = &mut width {
             let rules = window.rules();
             let border_config = rules.border.resolve_against(self.options.border);
@@ -3903,6 +3884,7 @@ impl<W: LayoutElement> Layout<W> {
                 *w += border_config.width.0 * 2.;
             }
         }
+
         width
     }
 }
@@ -4242,10 +4224,10 @@ mod tests {
         AddWindow {
             params: TestWindowParams,
         },
-        AddWindowRightOf {
+        AddWindowNextTo {
             params: TestWindowParams,
             #[proptest(strategy = "1..=5usize")]
-            right_of_id: usize,
+            next_to_id: usize,
         },
         AddWindowToNamedWorkspace {
             params: TestWindowParams,
@@ -4548,25 +4530,26 @@ mod tests {
                     let win = TestWindow::new(params);
                     layout.add_window(
                         win,
+                        AddWindowTarget::Auto,
                         None,
                         false,
                         params.is_floating,
                         ActivateWindow::default(),
                     );
                 }
-                Op::AddWindowRightOf {
+                Op::AddWindowNextTo {
                     mut params,
-                    right_of_id,
+                    next_to_id,
                 } => {
-                    let mut found_right_of = false;
+                    let mut found_next_to = false;
 
                     if let Some(InteractiveMoveState::Moving(move_)) = &layout.interactive_move {
                         let win_id = move_.tile.window().0.id;
                         if win_id == params.id {
                             return;
                         }
-                        if win_id == right_of_id {
+                        if win_id == next_to_id {
-                            found_right_of = true;
+                            found_next_to = true;
                         }
                     }
 
@@ -4579,8 +4562,8 @@ mod tests {
                                             return;
                                         }
 
-                                        if win.0.id == right_of_id {
+                                        if win.0.id == next_to_id {
-                                            found_right_of = true;
+                                            found_next_to = true;
                                         }
                                     }
                                 }
@@ -4593,15 +4576,15 @@ mod tests {
                                         return;
                                     }
 
-                                    if win.0.id == right_of_id {
+                                    if win.0.id == next_to_id {
-                                        found_right_of = true;
+                                        found_next_to = true;
                                     }
                                 }
                             }
                         }
                     }
 
-                    if !found_right_of {
+                    if !found_next_to {
                         return;
                     }
 
@@ -4612,14 +4595,21 @@ mod tests {
                     }
 
                     let win = TestWindow::new(params);
-                    layout.add_window_right_of(&right_of_id, win, None, false, params.is_floating);
+                    layout.add_window(
+                        win,
+                        AddWindowTarget::NextTo(&next_to_id),
+                        None,
+                        false,
+                        params.is_floating,
+                        ActivateWindow::default(),
+                    );
                 }
                 Op::AddWindowToNamedWorkspace {
                     mut params,
                     ws_name,
                 } => {
                     let ws_name = format!("ws{ws_name}");
-                    let mut found_workspace = false;
+                    let mut ws_id = None;
 
                     if let Some(InteractiveMoveState::Moving(move_)) = &layout.interactive_move {
                         if move_.tile.window().0.id == params.id {
@@ -4642,7 +4632,7 @@ mod tests {
                                         .as_ref()
                                         .map_or(false, |name| name.eq_ignore_ascii_case(&ws_name))
                                     {
-                                        found_workspace = true;
+                                        ws_id = Some(ws.id());
                                     }
                                 }
                             }
@@ -4660,15 +4650,15 @@ mod tests {
                                     .as_ref()
                                     .map_or(false, |name| name.eq_ignore_ascii_case(&ws_name))
                                 {
-                                    found_workspace = true;
+                                    ws_id = Some(ws.id());
                                 }
                             }
                         }
                     }
 
-                    if !found_workspace {
+                    let Some(ws_id) = ws_id else {
                         return;
-                    }
+                    };
 
                     if let Some(parent_id) = params.parent_id {
                         if parent_id_causes_loop(layout, params.id, parent_id) {
@@ -4677,9 +4667,9 @@ mod tests {
                     }
 
                     let win = TestWindow::new(params);
-                    layout.add_window_to_named_workspace(
+                    layout.add_window(
-                        &ws_name,
                         win,
+                        AddWindowTarget::Workspace(ws_id),
                         None,
                         false,
                         params.is_floating,
@@ -5116,9 +5106,9 @@ mod tests {
             Op::AddWindow {
                 params: TestWindowParams::new(1),
             },
-            Op::AddWindowRightOf {
+            Op::AddWindowNextTo {
                 params: TestWindowParams::new(2),
-                right_of_id: 1,
+                next_to_id: 1,
             },
             Op::AddWindowToNamedWorkspace {
                 params: TestWindowParams::new(3),
@@ -5265,13 +5255,13 @@ mod tests {
             Op::AddWindow {
                 params: TestWindowParams::new(2),
             },
-            Op::AddWindowRightOf {
+            Op::AddWindowNextTo {
                 params: TestWindowParams::new(6),
-                right_of_id: 0,
+                next_to_id: 0,
             },
-            Op::AddWindowRightOf {
+            Op::AddWindowNextTo {
                 params: TestWindowParams::new(7),
-                right_of_id: 1,
+                next_to_id: 1,
             },
             Op::AddWindowToNamedWorkspace {
                 params: TestWindowParams::new(5),
@@ -5663,9 +5653,9 @@ mod tests {
             Op::AddWindow {
                 params: TestWindowParams::new(2),
             },
-            Op::AddWindowRightOf {
+            Op::AddWindowNextTo {
                 params: TestWindowParams::new(3),
-                right_of_id: 1,
+                next_to_id: 1,
             },
         ];
 
@@ -5699,9 +5689,9 @@ mod tests {
             Op::AddWindow {
                 params: TestWindowParams::new(2),
             },
-            Op::AddWindowRightOf {
+            Op::AddWindowNextTo {
                 params: TestWindowParams::new(3),
-                right_of_id: 1,
+                next_to_id: 1,
             },
         ];
 

src/layout/monitor.rs

@@ -11,8 +11,10 @@ use smithay::utils::{Logical, Point, Rectangle};
 
 use super::scrolling::{Column, ColumnWidth};
 use super::tile::Tile;
-use super::workspace::{OutputId, Workspace, WorkspaceId, WorkspaceRenderElement};
+use super::workspace::{
-use super::{LayoutElement, Options};
+    OutputId, Workspace, WorkspaceAddWindowTarget, WorkspaceId, WorkspaceRenderElement,
+};
+use super::{ActivateWindow, LayoutElement, Options};
 use crate::animation::{Animation, Clock};
 use crate::input::swipe_tracker::SwipeTracker;
 use crate::render_helpers::renderer::NiriRenderer;
@@ -66,6 +68,23 @@ pub struct WorkspaceSwitchGesture {
     is_touchpad: bool,
 }
 
+/// Where to put a newly added window.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub enum MonitorAddWindowTarget<'a, W: LayoutElement> {
+    /// No particular preference.
+    #[default]
+    Auto,
+    /// On this workspace.
+    Workspace {
+        /// Id of the target workspace.
+        id: WorkspaceId,
+        /// Override where the window will open as a new column.
+        column_idx: Option<usize>,
+    },
+    /// Next to this existing window.
+    NextTo(&'a W::Id),
+}
+
 pub type MonitorRenderElement<R> =
     RelocateRenderElement<CropRenderElement<WorkspaceRenderElement<R>>>;
 
@@ -220,56 +239,18 @@ impl<W: LayoutElement> Monitor<W> {
 
     pub fn add_window(
         &mut self,
-        mut workspace_idx: usize,
         window: W,
-        activate: bool,
+        target: MonitorAddWindowTarget<W>,
+        activate: ActivateWindow,
         width: ColumnWidth,
         is_full_width: bool,
         is_floating: bool,
     ) {
-        let workspace = &mut self.workspaces[workspace_idx];
+        // Currently, everything a workspace sets on a Tile is the same across all workspaces of a
+        // monitor. So we can use any workspace, not necessarily the exact target workspace.
+        let tile = self.workspaces[0].make_tile(window);
 
-        workspace.add_window(window, activate, width, is_full_width, is_floating);
+        self.add_tile(tile, target, activate, width, is_full_width, is_floating);
-
-        // After adding a new window, workspace becomes this output's own.
-        workspace.original_output = OutputId::new(&self.output);
-
-        if workspace_idx == self.workspaces.len() - 1 {
-            self.add_workspace_bottom();
-        }
-
-        if self.options.empty_workspace_above_first && workspace_idx == 0 {
-            self.add_workspace_top();
-            workspace_idx += 1;
-        }
-
-        if activate {
-            self.activate_workspace(workspace_idx);
-        }
-    }
-
-    pub fn add_window_right_of(
-        &mut self,
-        right_of: &W::Id,
-        window: W,
-        width: ColumnWidth,
-        is_full_width: bool,
-        is_floating: bool,
-    ) {
-        let workspace_idx = self
-            .workspaces
-            .iter_mut()
-            .position(|ws| ws.has_window(right_of))
-            .unwrap();
-        let workspace = &mut self.workspaces[workspace_idx];
-
-        workspace.add_window_right_of(right_of, window, width, is_full_width, is_floating);
-
-        // After adding a new window, workspace becomes this output's own.
-        workspace.original_output = OutputId::new(&self.output);
-
-        // Since we're adding window right of something, the workspace isn't empty, and therefore
-        // cannot be the last one, so we never need to insert a new empty workspace.
     }
 
     pub fn add_column(&mut self, mut workspace_idx: usize, column: Column<W>, activate: bool) {
@@ -295,16 +276,39 @@ impl<W: LayoutElement> Monitor<W> {
 
     pub fn add_tile(
         &mut self,
-        mut workspace_idx: usize,
-        column_idx: Option<usize>,
         tile: Tile<W>,
-        activate: bool,
+        target: MonitorAddWindowTarget<W>,
+        activate: ActivateWindow,
         width: ColumnWidth,
         is_full_width: bool,
+        is_floating: bool,
     ) {
+        let (mut workspace_idx, target) = match target {
+            MonitorAddWindowTarget::Auto => {
+                (self.active_workspace_idx, WorkspaceAddWindowTarget::Auto)
+            }
+            MonitorAddWindowTarget::Workspace { id, column_idx } => {
+                let idx = self.workspaces.iter().position(|ws| ws.id() == id).unwrap();
+                let target = if let Some(column_idx) = column_idx {
+                    WorkspaceAddWindowTarget::NewColumnAt(column_idx)
+                } else {
+                    WorkspaceAddWindowTarget::Auto
+                };
+                (idx, target)
+            }
+            MonitorAddWindowTarget::NextTo(win_id) => {
+                let idx = self
+                    .workspaces
+                    .iter_mut()
+                    .position(|ws| ws.has_window(win_id))
+                    .unwrap();
+                (idx, WorkspaceAddWindowTarget::NextTo(win_id))
+            }
+        };
+
         let workspace = &mut self.workspaces[workspace_idx];
 
-        workspace.add_tile(column_idx, tile, activate, width, is_full_width);
+        workspace.add_tile(tile, target, activate, width, is_full_width, is_floating);
 
         // After adding a new window, workspace becomes this output's own.
         workspace.original_output = OutputId::new(&self.output);
@@ -319,30 +323,7 @@ impl<W: LayoutElement> Monitor<W> {
             workspace_idx += 1;
         }
 
-        if activate {
+        if activate.map_smart(|| false) {
-            self.activate_workspace(workspace_idx);
-        }
-    }
-
-    pub fn add_floating_tile(&mut self, mut workspace_idx: usize, tile: Tile<W>, activate: bool) {
-        let workspace = &mut self.workspaces[workspace_idx];
-
-        workspace.add_floating_tile(tile, activate);
-
-        // After adding a new window, workspace becomes this output's own.
-        workspace.original_output = OutputId::new(&self.output);
-
-        if workspace_idx == self.workspaces.len() - 1 {
-            // Insert a new empty workspace.
-            self.add_workspace_bottom();
-        }
-
-        if self.options.empty_workspace_above_first && workspace_idx == 0 {
-            self.add_workspace_top();
-            workspace_idx += 1;
-        }
-
-        if activate {
             self.activate_workspace(workspace_idx);
         }
     }
@@ -518,24 +499,24 @@ impl<W: LayoutElement> Monitor<W> {
         if new_idx == source_workspace_idx {
             return;
         }
+        let new_id = self.workspaces[new_idx].id();
 
         let workspace = &mut self.workspaces[source_workspace_idx];
         let Some(removed) = workspace.remove_active_tile(Transaction::new()) else {
             return;
         };
 
-        if removed.is_floating {
+        self.add_tile(
-            self.add_floating_tile(new_idx, removed.tile, true);
+            removed.tile,
-        } else {
+            MonitorAddWindowTarget::Workspace {
-            self.add_tile(
+                id: new_id,
-                new_idx,
+                column_idx: None,
-                None,
+            },
-                removed.tile,
+            ActivateWindow::Yes,
-                true,
+            removed.width,
-                removed.width,
+            removed.is_full_width,
-                removed.is_full_width,
+            removed.is_floating,
-            );
+        );
-        }
     }
 
     pub fn move_to_workspace_down(&mut self) {
@@ -545,24 +526,24 @@ impl<W: LayoutElement> Monitor<W> {
         if new_idx == source_workspace_idx {
             return;
         }
+        let new_id = self.workspaces[new_idx].id();
 
         let workspace = &mut self.workspaces[source_workspace_idx];
         let Some(removed) = workspace.remove_active_tile(Transaction::new()) else {
             return;
         };
 
-        if removed.is_floating {
+        self.add_tile(
-            self.add_floating_tile(new_idx, removed.tile, true);
+            removed.tile,
-        } else {
+            MonitorAddWindowTarget::Workspace {
-            self.add_tile(
+                id: new_id,
-                new_idx,
+                column_idx: None,
-                None,
+            },
-                removed.tile,
+            ActivateWindow::Yes,
-                true,
+            removed.width,
-                removed.width,
+            removed.is_full_width,
-                removed.is_full_width,
+            removed.is_floating,
-            );
+        );
-        }
     }
 
     pub fn move_to_workspace(&mut self, window: Option<&W::Id>, idx: usize) {
@@ -579,10 +560,16 @@ impl<W: LayoutElement> Monitor<W> {
         if new_idx == source_workspace_idx {
             return;
         }
+        let new_id = self.workspaces[new_idx].id();
 
         let activate = window.map_or(true, |win| {
             self.active_window().map(|win| win.id()) == Some(win)
         });
+        let activate = if activate {
+            ActivateWindow::Yes
+        } else {
+            ActivateWindow::No
+        };
 
         let workspace = &mut self.workspaces[source_workspace_idx];
         let transaction = Transaction::new();
@@ -594,18 +581,17 @@ impl<W: LayoutElement> Monitor<W> {
             return;
         };
 
-        if removed.is_floating {
+        self.add_tile(
-            self.add_floating_tile(new_idx, removed.tile, activate);
+            removed.tile,
-        } else {
+            MonitorAddWindowTarget::Workspace {
-            self.add_tile(
+                id: new_id,
-                new_idx,
+                column_idx: None,
-                None,
+            },
-                removed.tile,
+            activate,
-                activate,
+            removed.width,
-                removed.width,
+            removed.is_full_width,
-                removed.is_full_width,
+            removed.is_floating,
-            );
+        );
-        }
 
         if self.workspace_switch.is_none() {
             self.clean_up_workspaces();

src/layout/scrolling.rs

@@ -783,6 +783,7 @@ impl<W: LayoutElement> ScrollingSpace<W> {
         &mut self,
         right_of: &W::Id,
         tile: Tile<W>,
+        activate: bool,
         width: ColumnWidth,
         is_full_width: bool,
     ) {
@@ -793,9 +794,6 @@ impl<W: LayoutElement> ScrollingSpace<W> {
             .unwrap();
         let col_idx = right_of_idx + 1;
 
-        // Activate the new window if right_of was active.
-        let activate = self.active_column_idx == right_of_idx;
-
         self.add_tile(Some(col_idx), tile, activate, width, is_full_width, None);
     }
 

src/layout/workspace.rs

@@ -18,7 +18,7 @@ use super::scrolling::{
     Column, ColumnWidth, InsertHint, InsertPosition, ScrollingSpace, ScrollingSpaceRenderElement,
 };
 use super::tile::{Tile, TileRenderSnapshot};
-use super::{InteractiveResizeData, LayoutElement, Options, RemovedTile, SizeFrac};
+use super::{ActivateWindow, InteractiveResizeData, LayoutElement, Options, RemovedTile, SizeFrac};
 use crate::animation::Clock;
 use crate::niri_render_elements;
 use crate::render_helpers::renderer::NiriRenderer;
@@ -157,6 +157,18 @@ enum FloatingActive {
     Yes,
 }
 
+/// Where to put a newly added window.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub enum WorkspaceAddWindowTarget<'a, W: LayoutElement> {
+    /// No particular preference.
+    #[default]
+    Auto,
+    /// As a new column at this index.
+    NewColumnAt(usize),
+    /// Next to this existing window.
+    NextTo(&'a W::Id),
+}
+
 impl OutputId {
     pub fn new(output: &Output) -> Self {
         let output_name = output.user_data().get::<OutputName>().unwrap();
@@ -488,55 +500,138 @@ impl<W: LayoutElement> Workspace<W> {
         self.view_size
     }
 
-    pub fn add_window(
+    pub fn make_tile(&self, window: W) -> Tile<W> {
-        &mut self,
+        Tile::new(
-        window: W,
-        activate: bool,
-        width: ColumnWidth,
-        is_full_width: bool,
-        is_floating: bool,
-    ) {
-        let mut tile = Tile::new(
             window,
             self.view_size,
             self.scale.fractional_scale(),
             self.clock.clone(),
             self.options.clone(),
-        );
+        )
-        tile.unfullscreen_to_floating = is_floating;
-
-        // If the tile is pending fullscreen, open it in the scrolling layout where it can go
-        // fullscreen.
-        if is_floating && !tile.window().is_pending_fullscreen() {
-            self.add_floating_tile(tile, activate);
-        } else {
-            self.add_tile(None, tile, activate, width, is_full_width);
-        }
     }
 
     pub fn add_tile(
         &mut self,
-        col_idx: Option<usize>,
+        mut tile: Tile<W>,
-        tile: Tile<W>,
+        target: WorkspaceAddWindowTarget<W>,
-        activate: bool,
+        activate: ActivateWindow,
         width: ColumnWidth,
         is_full_width: bool,
+        is_floating: bool,
     ) {
         self.enter_output_for_window(tile.window());
-        self.scrolling
+        tile.unfullscreen_to_floating = is_floating;
-            .add_tile(col_idx, tile, activate, width, is_full_width, None);
 
-        if activate {
+        match target {
-            self.floating_is_active = FloatingActive::No;
+            WorkspaceAddWindowTarget::Auto => {
-        }
+                // Don't steal focus from an active fullscreen window.
-    }
+                let activate = activate.map_smart(|| !self.is_active_fullscreen());
 
-    pub fn add_floating_tile(&mut self, tile: Tile<W>, activate: bool) {
+                // If the tile is pending fullscreen, open it in the scrolling layout where it can
-        self.enter_output_for_window(tile.window());
+                // go fullscreen.
-        self.floating.add_tile(tile, activate);
+                if is_floating && !tile.window().is_pending_fullscreen() {
+                    self.floating.add_tile(tile, activate);
 
-        if activate || self.scrolling.is_empty() {
+                    if activate || self.scrolling.is_empty() {
-            self.floating_is_active = FloatingActive::Yes;
+                        self.floating_is_active = FloatingActive::Yes;
+                    }
+                } else {
+                    self.scrolling
+                        .add_tile(None, tile, activate, width, is_full_width, None);
+
+                    if activate {
+                        self.floating_is_active = FloatingActive::No;
+                    }
+                }
+            }
+            WorkspaceAddWindowTarget::NewColumnAt(col_idx) => {
+                let activate = activate.map_smart(|| false);
+                self.scrolling
+                    .add_tile(Some(col_idx), tile, activate, width, is_full_width, None);
+
+                if activate {
+                    self.floating_is_active = FloatingActive::No;
+                }
+            }
+            WorkspaceAddWindowTarget::NextTo(next_to) => {
+                let activate = activate.map_smart(|| self.active_window().unwrap().id() == next_to);
+
+                let floating_has_window = self.floating.has_window(next_to);
+                if is_floating || floating_has_window {
+                    if floating_has_window {
+                        self.floating.add_tile_above(next_to, tile, activate);
+                    } else {
+                        // FIXME: use static pos
+                        let (next_to_tile, render_pos) = self
+                            .scrolling
+                            .tiles_with_render_positions()
+                            .find(|(tile, _)| tile.window().id() == next_to)
+                            .unwrap();
+
+                        // Position the new tile in the center above the next_to tile. Think a
+                        // dialog opening on top of a window.
+                        let tile_size = tile.tile_size();
+                        let pos = render_pos
+                            + (next_to_tile.tile_size().to_point() - tile_size.to_point())
+                                .downscale(2.);
+                        let pos = self.floating.clamp_within_working_area(pos, tile_size);
+                        let pos = self.floating.logical_to_size_frac(pos);
+                        tile.floating_pos = Some(pos);
+
+                        self.floating.add_tile(tile, activate);
+
+                        if activate {
+                            self.floating_is_active = FloatingActive::Yes;
+                        }
+                    }
+                } else {
+                    self.scrolling
+                        .add_tile_right_of(next_to, tile, activate, width, is_full_width);
+                }
+
+                // if is_floating && !tile.window().is_pending_fullscreen() {
+                //     if floating_has_window {
+                //         self.floating.add_tile_above(next_to, tile, activate);
+                //     } else {
+                //         // FIXME: use static pos
+                //         let (next_to_tile, render_pos) = self
+                //             .scrolling
+                //             .tiles_with_render_positions()
+                //             .find(|(tile, _)| tile.window().id() == next_to)
+                //             .unwrap();
+                //
+                //         // Position the new tile in the center above the next_to tile. Think a
+                //         // dialog opening on top of a window.
+                //         let tile_size = tile.tile_size();
+                //         let pos = render_pos
+                //             + (next_to_tile.tile_size().to_point() - tile_size.to_point())
+                //                 .downscale(2.);
+                //         let pos = self.floating.clamp_within_working_area(pos, tile_size);
+                //         let pos = self.floating.logical_to_size_frac(pos);
+                //         tile.floating_pos = Some(pos);
+                //
+                //         self.floating.add_tile(tile, activate);
+                //     }
+                //
+                //     if activate || self.scrolling.is_empty() {
+                //         self.floating_is_active = FloatingActive::Yes;
+                //     }
+                // } else if floating_has_window {
+                //     self.scrolling
+                //         .add_tile(None, tile, activate, width, is_full_width, None);
+                //
+                //     if activate {
+                //         self.floating_is_active = FloatingActive::No;
+                //     }
+                // } else {
+                //     self.scrolling
+                //         .add_tile_right_of(next_to, tile, activate, width, is_full_width);
+                //
+                //     if activate {
+                //         self.floating_is_active = FloatingActive::No;
+                //     }
+                // }
+            }
         }
     }
 
@@ -556,70 +651,6 @@ impl<W: LayoutElement> Workspace<W> {
         }
     }
 
-    pub fn add_window_right_of(
-        &mut self,
-        right_of: &W::Id,
-        window: W,
-        width: ColumnWidth,
-        is_full_width: bool,
-        // TODO: smarter enum, so you can override is_floating = false for floating right_of.
-        is_floating: bool,
-    ) {
-        let mut tile = Tile::new(
-            window,
-            self.view_size,
-            self.scale.fractional_scale(),
-            self.clock.clone(),
-            self.options.clone(),
-        );
-        tile.unfullscreen_to_floating = is_floating;
-        self.add_tile_right_of(right_of, tile, width, is_full_width, is_floating);
-    }
-
-    pub fn add_tile_right_of(
-        &mut self,
-        right_of: &W::Id,
-        mut tile: Tile<W>,
-        width: ColumnWidth,
-        is_full_width: bool,
-        is_floating: bool,
-    ) {
-        self.enter_output_for_window(tile.window());
-
-        // TODO: open-fullscreen into scrolling.
-        let floating_has_window = self.floating.has_window(right_of);
-        if is_floating || floating_has_window {
-            if floating_has_window {
-                self.floating.add_tile_above(right_of, tile);
-            } else {
-                let activate = self.scrolling.active_window().unwrap().id() == right_of;
-                // FIXME: use static pos
-                let (right_of_tile, render_pos) = self
-                    .scrolling
-                    .tiles_with_render_positions()
-                    .find(|(tile, _)| tile.window().id() == right_of)
-                    .unwrap();
-
-                // Position the new tile in the center above the right_of tile. Think a dialog
-                // opening on top of a window.
-                let tile_size = tile.tile_size();
-                let pos = render_pos
-                    + (right_of_tile.tile_size().to_point() - tile_size.to_point()).downscale(2.);
-                let pos = self.floating.clamp_within_working_area(pos, tile_size);
-                let pos = self.floating.logical_to_size_frac(pos);
-                tile.floating_pos = Some(pos);
-
-                self.floating.add_tile(tile, activate);
-                if activate {
-                    self.floating_is_active = FloatingActive::Yes;
-                }
-            }
-        } else {
-            self.scrolling
-                .add_tile_right_of(right_of, tile, width, is_full_width);
-        }
-    }
-
     pub fn add_column(&mut self, column: Column<W>, activate: bool) {
         for (tile, _) in column.tiles() {
             self.enter_output_for_window(tile.window());
@@ -702,10 +733,12 @@ impl<W: LayoutElement> Workspace<W> {
     pub fn resolve_default_width(
         &self,
         default_width: Option<Option<ColumnWidth>>,
+        is_floating: bool,
     ) -> Option<ColumnWidth> {
         match default_width {
             Some(Some(width)) => Some(width),
             Some(None) => None,
+            None if is_floating => None,
             None => self.options.default_column_width,
         }
     }