diff --git a/rift.default.toml b/rift.default.toml index 1501ceadd..75faf751d 100644 --- a/rift.default.toml +++ b/rift.default.toml @@ -26,6 +26,14 @@ animation_fps = 100.0 focus_follows_mouse = true mouse_follows_focus = true mouse_hides_on_focus = true +# Which display a newly created window opens on: +# - "default": wherever macOS and the app put it +# - "focused": the display holding keyboard focus when the window appears +# - "cursor": the display under the mouse cursor when the window appears. Useful +# when a launcher activates an app whose other windows live on another display +# before asking it for a new window. +# A window whose app rule names a workspace always goes where the rule says. +new_window_display = "default" # Treat displays stacked in macOS as a horizontal mouse crossing chain when # they are physically side-by-side. At a left/right edge, the cursor enters # the logical neighbor at the same vertical offset, if that point exists. @@ -301,11 +309,26 @@ prevent_wrapping = false reapply_app_rules_on_title_change = false # Workspace-specific rules -# - workspace: target workspace by index (integer) or name (string) +# - workspace: target workspace by index (integer) or name (string); unnamed +# workspaces are named "Workspace N" # - layout: layout mode to use ("traditional", "bsp", "stack", "master_stack", "scrolling", "floating") +# - display: the display the workspace lives on, as a display UUID (recommended: +# stable across reconnects, see `rift-cli query displays`) or a 0-based index in +# physical order (left to right, then top to bottom) +# +# A workspace bound to a display lives there while the display is connected: +# - switch_to_workspace focuses that display and switches the workspace there +# - move_window_to_workspace sends the window to that display +# - app_rules targeting the workspace place new windows on that display +# - when the display is disconnected the workspace falls back to the current +# display; once it reconnects, windows parked there are moved back +# Where several rules match a workspace, the last one giving a setting wins. +# # workspace_rules = [ # { workspace = 1, layout = "bsp" }, -# { workspace = "second", layout = "scrolling" } +# { workspace = "second", layout = "scrolling" }, +# { workspace = "web", display = "37D8832A-2D66-02CA-B9F7-8F30A301B230" }, +# { workspace = 4, display = 1 }, # ] workspace_rules = [] diff --git a/src/actor/app.rs b/src/actor/app.rs index 643517c70..8ffef108d 100644 --- a/src/actor/app.rs +++ b/src/actor/app.rs @@ -982,7 +982,10 @@ impl State { Duration::from_secs(60) } - _ => Duration::ZERO, + // A brief grace period for everyone else: a busy app can reject the + // first accessibility requests, and giving up at once would leave it + // unmanaged until it relaunches. + _ => Duration::from_secs(2), }; let mut sleep_dur = Duration::from_millis(20); let mut sleep = || { diff --git a/src/actor/config.rs b/src/actor/config.rs index 73949d373..ed37886b0 100644 --- a/src/actor/config.rs +++ b/src/actor/config.rs @@ -409,7 +409,8 @@ mod tests { s.virtual_workspaces.workspace_names[1] = "Development".into(); s.virtual_workspaces.workspace_rules.push(WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(1), - layout: LayoutMode::Scrolling, + layout: Some(LayoutMode::Scrolling), + display: None, }); s.virtual_workspaces.app_rules.push(AppWorkspaceRule { app_id: Some("com.apple.Safari".into()), diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index 8d2181f05..7d6dca183 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -15,6 +15,7 @@ mod query; mod replay; pub mod transaction_manager; mod utils; +mod workspace_bindings; #[cfg(test)] mod testing; @@ -434,6 +435,12 @@ pub struct Reactor { pub animation_tx: Option, viewport_gesture: Option, presentations: HashMap, + /// Cross-display moves rift started that macOS may not have caught up with: + /// window -> (target space, end of the grace period). + in_flight_display_moves: HashMap, + /// Workspace display bindings need re-applying once the current event's + /// outcome has settled window membership. + bindings_need_check: bool, #[cfg(test)] event_outcome_phase_trace: Vec<&'static str>, #[cfg(test)] @@ -445,6 +452,11 @@ pub struct Reactor { } impl Reactor { + /// How long a window rift moved to another display is held there against reports + /// of its old display. Those reports lag the move while macOS and the app catch up, + /// and following them makes the window flip back and forth between displays. + const DISPLAY_MOVE_GRACE: Duration = Duration::from_secs(2); + pub fn spawn( config: Config, layout_engine: LayoutEngine, @@ -571,6 +583,8 @@ impl Reactor { animation_tx: None, viewport_gesture: None, presentations: HashMap::default(), + in_flight_display_moves: HashMap::default(), + bindings_need_check: false, #[cfg(test)] event_outcome_phase_trace: Vec::new(), #[cfg(test)] @@ -1197,6 +1211,7 @@ impl Reactor { outcome = outcome.with_focused_window_broadcast(focused_window); } self.apply_event_outcome(outcome); + self.apply_pending_display_bindings(); if may_make_ready && self.startup_ready.is_some() && let Some(space) = self.default_query_space() @@ -1893,6 +1908,9 @@ impl Reactor { let Some(index) = workspaces.iter().position(|(id, _)| *id == workspace) else { return Ok(EventOutcome::no_change()); }; + if let Some(routed) = self.route_overview_workspace_selection(space, index) { + return routed; + } // Change display context without first focusing its old workspace's window. if let Some(screen) = self.space_state.screen_by_space(space) { if crate::sys::screen::set_active_menu_bar_display_uuid(&screen.display_uuid) { @@ -1946,6 +1964,9 @@ impl Reactor { if !changed { return Ok(EventOutcome::no_change()); } + // A drop onto a display's copy of a workspace bound elsewhere sends + // the window on to that workspace's display. + self.check_display_bindings_later(); let source_space = source.unwrap().space; let destination = destination.unwrap(); let mut outcome = EventOutcome::layout_changed(false); @@ -1959,6 +1980,7 @@ impl Reactor { { self.state.windows.set_window_server_space(server_id, Some(destination)); } + self.note_display_move_in_flight(intent.window, destination); let frame = intent.frame.unwrap_or_else(|| { let destination = self .space_state @@ -2118,13 +2140,18 @@ impl Reactor { return Ok(system_workflow::handle_raise_timeout(sequence_id)?); } Event::ConfigUpdated(new_cfg) => { - return command_workflow::handle_config_updated( + let outcome = command_workflow::handle_config_updated( &mut self.config, &mut self.layout_manager, &self.state, &mut self.drag_manager, new_cfg, - ); + )?; + // Bindings may have changed. + let screens = self.space_state.screens.clone(); + self.refresh_display_bindings(&screens); + self.check_display_bindings_later(); + return Ok(outcome); } Event::Command(Command::Metrics(cmd)) => { return command_workflow::handle_command_metrics(cmd); @@ -2280,51 +2307,27 @@ impl Reactor { ); } Event::Command(Command::Reactor(ReactorCommand::FocusDisplay(selector))) => { - let screen = self.screen_for_selector(&selector, None).cloned(); - let focus_window = screen.as_ref().and_then(|screen| { - let space = screen.space?; - self.last_focused_window_in_space(space).or_else(|| { - self.layout_manager - .layout_engine - .workspaces() - .windows_in_active_workspace(&self.state.windows, space) - .into_iter() - .next() - }) - }); - let target_is_active = screen - .as_ref() - .and_then(|screen| screen.space) - .is_none_or(|space| self.is_space_active(space)); - if target_is_active - && let Some(screen) = screen.as_ref().filter(|screen| screen.space.is_some()) - { - if crate::sys::screen::set_active_menu_bar_display_uuid(&screen.display_uuid) { - self.space_state.menu_bar_space = screen.space; - } - // Honor explicit display selection before the native notification arrives, - // even on activation failure. Later spaces-actor updates remain authoritative. - self.space_state.command_space = screen.space; - } - let focus_window_center = focus_window - .and_then(|wid| self.state.windows.window(wid)) - .map(|window| window.frame_monotonic.mid()); - return command_workflow::handle_focus_display( - &self.app_manager, - command_workflow::DisplayFocusPayload { - screen, - target_is_active, - focus_window, - focus_window_center, - }, - ); + return self.focus_display_by_selector(&selector); } Event::Command(Command::Layout(command)) => { + if let Some(routed) = self.route_bound_workspace_command(&command) { + return routed; + } let post_arrange_mouse_warp = self.config.settings.mouse_follows_focus.then(|| self.main_window()).flatten(); let command_space = self.command_context_space(); + let is_move_node = matches!(command, layout::LayoutCommand::MoveNode(_)); + // A move-node can carry the windows of the command display elsewhere. + let moved_candidates: Vec = match command_space { + Some(space) if is_move_node => self + .layout_manager + .layout_engine + .workspaces() + .windows_in_active_workspace(&self.state.windows, space), + _ => Vec::new(), + }; let (visible_spaces, visible_space_frames) = self.visible_spaces_for_layout(false); - return command_workflow::handle_command_layout( + let outcome = command_workflow::handle_command_layout( &mut self.state, &mut self.layout_manager, &mut self.workspace_switch_manager, @@ -2335,7 +2338,18 @@ impl Reactor { visible_space_frames, post_arrange_mouse_warp, }, - ); + )?; + if is_move_node { + for window in moved_candidates { + if let Some(space) = self.assigned_space_for_window_id(window) + && Some(space) != command_space + { + self.note_display_move_in_flight(window, space); + } + } + self.follow_focused_window_to_its_display(command_space); + } + return Ok(outcome); } Event::Command(Command::Reactor(ReactorCommand::MoveWindowToDisplay { selector, @@ -2419,7 +2433,7 @@ impl Reactor { // presentation work before the transfer installs its destination frame. self.cancel_window_presentations(vec![window]); let target_frame = Self::center_frame_on_screen(window_frame, target_screen.frame); - return command_workflow::handle_command_reactor_move_window_to_display( + let outcome = command_workflow::handle_command_reactor_move_window_to_display( &mut self.state, &mut self.layout_manager, command_workflow::MoveWindowToDisplayPayload { @@ -2429,8 +2443,12 @@ impl Reactor { target_space, target_screen: target_screen.frame, target_frame, + target_workspace: None, + follow: false, }, - ); + )?; + self.note_display_move_in_flight(window, target_space); + return Ok(outcome); } Event::Command(Command::Reactor(ReactorCommand::MoveWorkspaceToDisplay { selector, @@ -2473,6 +2491,13 @@ impl Reactor { if source_space == target_space { return Ok(EventOutcome::no_change()); } + if self.bound_workspace_blocks_display_move(source_space, target_space) { + warn!( + ?selector, + "Move workspace to display ignored: the workspace is bound to its display" + ); + return Ok(EventOutcome::no_change()); + } let windows = self .layout_manager @@ -2501,7 +2526,9 @@ impl Reactor { return Ok(EventOutcome::no_change()); } - return command_workflow::handle_command_reactor_move_workspace_to_display( + let moved: Vec = + moves.iter().map(|window_move| window_move.window).collect(); + let outcome = command_workflow::handle_command_reactor_move_workspace_to_display( &mut self.state, &mut self.layout_manager, &mut self.workspace_switch_manager, @@ -2511,7 +2538,11 @@ impl Reactor { target_space, target_screen: target_screen.frame, }, - ); + )?; + for window in moved { + self.note_display_move_in_flight(window, target_space); + } + return Ok(outcome); } _ => (), } @@ -2580,6 +2611,7 @@ impl Reactor { } if self.state.windows.window(window).is_some_and(WindowState::is_admitted) { self.send_layout_event(LayoutEvent::WindowAdded(space, window)); + self.place_new_window_on_configured_display(window, space); } } } @@ -3244,6 +3276,9 @@ impl Reactor { active_window_spaces, .. } = space_state; + // Before any new native space gets its workspaces: start each display on a + // workspace it owns. + self.refresh_display_bindings(&screens); self.space_state.active_window_spaces = active_window_spaces; self.space_state.membership_complete = membership_complete; let activation_config = self.activation_cfg(); @@ -3351,6 +3386,10 @@ impl Reactor { if should_force_refresh_layout { outcome = outcome.with_arrange_passes(1); } + if display_set_changed || should_force_refresh_layout { + // A display joined, left or moved. + self.check_display_bindings_later(); + } Ok(outcome) } @@ -3690,7 +3729,36 @@ impl Reactor { self.state.windows.workspace_info_for_window(wid).map(|info| info.space) } + /// Record that rift just moved `window` onto `target`'s display. + fn note_display_move_in_flight(&mut self, window: WindowId, target: SpaceId) { + if self.assigned_space_for_window_id(window) != Some(target) { + return; + } + let Some(wsid) = self.state.windows.window(window).and_then(|state| state.info.sys_id) + else { + return; + }; + let now = Instant::now(); + self.in_flight_display_moves.retain(|_, (_, deadline)| *deadline > now); + self.in_flight_display_moves + .insert(wsid, (target, now + Self::DISPLAY_MOVE_GRACE)); + } + + /// Target of a cross-display move rift started for `wsid`, while it is still in + /// its grace period and the window is still assigned there. + fn in_flight_display_move_target(&self, wsid: WindowServerId) -> Option { + let (target, deadline) = *self.in_flight_display_moves.get(&wsid)?; + if Instant::now() >= deadline { + return None; + } + let wid = self.state.windows.tracked_window_id(wsid)?; + (self.assigned_space_for_window_id(wid) == Some(target)).then_some(target) + } + fn pending_target_space_for_window_server_id(&self, wsid: WindowServerId) -> Option { + if let Some(target) = self.in_flight_display_move_target(wsid) { + return Some(target); + } let wid = self.state.windows.tracked_window_id(wsid)?; let target_frame = self.transaction_manager.get_target_frame(wsid)?; let assigned_space = self.assigned_space_for_window_id(wid)?; @@ -3823,9 +3891,12 @@ impl Reactor { }) .collect(); for (wid, authoritative_space) in windows { + // A window keeps its workspace number when its display went away, or when + // it is returning to the display its workspace is bound to. let preserve_ordinal = self .assigned_space_for_window_id(wid) - .is_some_and(|space| invalidated_spaces.contains(&space)); + .is_some_and(|space| invalidated_spaces.contains(&space)) + || self.window_returns_to_bound_display(wid, authoritative_space); self.reassign_window_to_authoritative_space(wid, authoritative_space, preserve_ordinal); } } @@ -3862,6 +3933,12 @@ impl Reactor { wsid: WindowServerId, observation: Option, ) -> Option { + if let Some(target) = self.in_flight_display_move_target(wsid) { + // Rift just moved this window to another display. Reports of its old + // display, even from a live query, are lag rather than the user moving it. + trace!(?wsid, ?observation, ?target, "Holding window on its new display"); + return Some(target); + } let pending = self.pending_target_space_for_window_server_id(wsid); let live = if observation.is_none() || pending.is_some_and(|target| observation != Some(target)) { @@ -4086,6 +4163,15 @@ impl Reactor { if focus_desktop && let Some(space) = self.workspace_command_space() { self.focus_desktop_if_active_workspace_empty(space); } + if matches!( + event_clone, + LayoutEvent::WindowAdded(..) + | LayoutEvent::WindowObserved(..) + | LayoutEvent::WindowDiscoveryCompleted(..) + ) { + // A new or rediscovered window may sit in a workspace bound elsewhere. + self.check_display_bindings_later(); + } for space in self.space_state.iter_known_spaces() { self.layout_manager.layout_engine.debug_tree_desc(space, "after event", false); } @@ -5292,17 +5378,143 @@ impl Reactor { CGRect::new(origin, frame.size) } - fn screens_in_physical_order(&self) -> Vec<&ScreenInfo> { - let mut screens: Vec<&ScreenInfo> = self.space_state.screens.iter().collect(); - screens.sort_by(|a, b| { - let x_order = a.frame.origin.x.total_cmp(&b.frame.origin.x); - if x_order == std::cmp::Ordering::Equal { - a.frame.origin.y.total_cmp(&b.frame.origin.y) - } else { - x_order + /// Move a newly created window to the display `settings.new_window_display` + /// names when macOS put it on another one. A window whose app rule names a + /// workspace stays where the rule put it. + fn place_new_window_on_configured_display(&mut self, window: WindowId, space: SpaceId) { + use crate::common::config::NewWindowDisplay; + let target_space = match self.config.settings.new_window_display { + NewWindowDisplay::Default => return, + NewWindowDisplay::Focused => self.workspace_command_space(), + NewWindowDisplay::Cursor => window_server::current_cursor_location() + .ok() + .and_then(|point| self.screen_for_point(point)) + .and_then(|screen| screen.space), + }; + let Some(target_space) = + target_space.filter(|target| *target != space && self.is_space_active(*target)) + else { + return; + }; + let Some(state) = self.state.windows.window(window) else { + return; + }; + if !state.is_admitted() || !state.info.is_standard { + return; + } + let app_info = self.app_manager.apps.get(&window.pid).map(|app| app.info.clone()); + let names_workspace = self.layout_manager.layout_engine.app_rule_names_workspace( + crate::model::WindowRuleContext { + app_bundle_id: app_info.as_ref().and_then(|info| info.bundle_id.as_deref()), + app_name: app_info.as_ref().and_then(|info| info.localized_name.as_deref()), + window_title: Some(state.info.title.as_str()), + ax_role: state.info.ax_role.as_deref(), + ax_subrole: state.info.ax_subrole.as_deref(), + }, + ); + if names_workspace { + return; + } + let Some(target_screen) = self.space_state.screen_by_space(target_space).cloned() else { + return; + }; + let window_server_id = state.info.sys_id; + let target_frame = Self::center_frame_on_screen(state.frame_monotonic, target_screen.frame); + match command_workflow::handle_command_reactor_move_window_to_display( + &mut self.state, + &mut self.layout_manager, + command_workflow::MoveWindowToDisplayPayload { + window, + window_server_id, + source_space: space, + target_space, + target_screen: target_screen.frame, + target_frame, + target_workspace: None, + follow: false, + }, + ) { + Ok(outcome) => { + self.note_display_move_in_flight(window, target_space); + self.apply_event_outcome(outcome); + } + Err(error) => { + warn!(?window, %error, "Could not open new window on the configured display") } + } + } + + /// After a layout command carried the focused window onto another display, make + /// that display the command context, as an explicit `focus_display` would. macOS + /// moves its active display along with the key window only later, and until then + /// the next command would still act on the display the window just left. + fn follow_focused_window_to_its_display(&mut self, previous_space: Option) { + let Some(window) = self.layout_manager.layout_engine.focused_window() else { + return; + }; + let Some(space) = self.assigned_space_for_window_id(window) else { + return; + }; + if Some(space) == previous_space || !self.is_space_active(space) { + return; + } + let Some(display_uuid) = self.display_uuid_for_space(space) else { + return; + }; + if crate::sys::screen::set_active_menu_bar_display_uuid(&display_uuid) { + self.space_state.menu_bar_space = Some(space); + } + self.space_state.command_space = Some(space); + } + + /// Focus the display `selector` names: make it the command and menu-bar + /// context and focus its last focused window. + fn focus_display_by_selector( + &mut self, + selector: &DisplaySelector, + ) -> anyhow::Result { + let screen = self.screen_for_selector(selector, None).cloned(); + let focus_window = screen.as_ref().and_then(|screen| { + let space = screen.space?; + self.last_focused_window_in_space(space).or_else(|| { + self.layout_manager + .layout_engine + .workspaces() + .windows_in_active_workspace(&self.state.windows, space) + .into_iter() + .next() + }) }); - screens + let target_is_active = screen + .as_ref() + .and_then(|screen| screen.space) + .is_none_or(|space| self.is_space_active(space)); + if target_is_active + && let Some(screen) = screen.as_ref().filter(|screen| screen.space.is_some()) + { + if crate::sys::screen::set_active_menu_bar_display_uuid(&screen.display_uuid) { + self.space_state.menu_bar_space = screen.space; + } + // Honor explicit display selection before the native notification arrives, + // even on activation failure. Later spaces-actor updates remain authoritative. + self.space_state.command_space = screen.space; + } + let focus_window_center = focus_window + .and_then(|wid| self.state.windows.window(wid)) + .map(|window| window.frame_monotonic.mid()); + command_workflow::handle_focus_display( + &self.app_manager, + command_workflow::DisplayFocusPayload { + screen, + target_is_active, + focus_window, + focus_window_center, + }, + ) + } + + fn screens_in_physical_order(&self) -> Vec<&ScreenInfo> { + workspace_bindings::physical_order(&self.space_state.screens) } fn store_current_floating_positions(&mut self, space: SpaceId) { diff --git a/src/actor/reactor/events/command.rs b/src/actor/reactor/events/command.rs index 09075d655..2019e14d1 100644 --- a/src/actor/reactor/events/command.rs +++ b/src/actor/reactor/events/command.rs @@ -448,6 +448,10 @@ pub struct MoveWindowToDisplayPayload { pub target_space: SpaceId, pub target_screen: objc2_core_foundation::CGRect, pub target_frame: objc2_core_foundation::CGRect, + /// Workspace on the target display to move into; its active one when `None`. + pub target_workspace: Option, + /// Activate the target workspace and focus the window there. + pub follow: bool, } pub fn handle_command_reactor_move_window_to_display( @@ -462,13 +466,24 @@ pub fn handle_command_reactor_move_window_to_display( return Ok(EventOutcome::no_change()); } - let response = layout.layout_engine.move_window_to_space( - &mut state.windows, - payload.source_space, - payload.target_space, - payload.target_screen.size, - payload.window, - ); + let response = match payload.target_workspace { + None => layout.layout_engine.move_window_to_space( + &mut state.windows, + payload.source_space, + payload.target_space, + payload.target_screen.size, + payload.window, + ), + Some(workspace) => layout.layout_engine.move_window_to_workspace_on_space( + &mut state.windows, + payload.source_space, + payload.target_space, + payload.target_screen.size, + payload.window, + workspace, + payload.follow, + ), + }; if state .windows @@ -480,7 +495,7 @@ pub fn handle_command_reactor_move_window_to_display( } Ok(EventOutcome::layout_changed(false) - .with_layout_response(response, None) + .with_layout_response(response, payload.follow.then_some(payload.target_space)) .with_pre_layout_window_frame_write(payload.window, payload.target_frame, true)) } diff --git a/src/actor/reactor/testing.rs b/src/actor/reactor/testing.rs index 10b29ca3b..b7097c8c9 100644 --- a/src/actor/reactor/testing.rs +++ b/src/actor/reactor/testing.rs @@ -66,6 +66,14 @@ impl Reactor { .expect("test window should have a WindowServer identity") } + /// End the grace period of every cross-display move rift started. + pub fn expire_display_moves_for_test(&mut self) { + let expired = std::time::Instant::now() - std::time::Duration::from_millis(1); + for (_, deadline) in self.in_flight_display_moves.values_mut() { + *deadline = expired; + } + } + pub fn test_active_workspace_windows(&self, space: SpaceId) -> Vec { self.layout_manager .layout_engine @@ -685,3 +693,16 @@ pub fn next_test_topology_revision() -> u64 { next }) } + +/// A 1000x1000 display at the origin: the left one of two side by side. +pub fn left_screen() -> CGRect { CGRect::new(CGPoint::new(0., 0.), CGSize::new(1000., 1000.)) } + +/// A 1000x1000 display just right of [`left_screen`]. +pub fn right_screen() -> CGRect { CGRect::new(CGPoint::new(1000., 0.), CGSize::new(1000., 1000.)) } + +/// Deliver the snapshot that follows connecting exactly these displays. +pub fn connect_displays(reactor: &mut Reactor, frames: Vec, spaces: Vec>) { + reactor.handle_event(space_state_event_with(frames, spaces, |state| { + state.display_set_changed = true + })); +} diff --git a/src/actor/reactor/tests.rs b/src/actor/reactor/tests.rs index 691bd99c7..c6a01ed17 100644 --- a/src/actor/reactor/tests.rs +++ b/src/actor/reactor/tests.rs @@ -863,6 +863,37 @@ fn focus_display_invalid_or_inactive_target_preserves_context() { crate::sys::screen::TEST_ACTIVE_DISPLAY.with(|display| assert!(display.borrow().is_none())); } +#[test] +fn commands_follow_a_window_move_node_carried_to_another_display() { + let mut reactor = test_reactor(); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let mut apps = Apps::new(); + let window = WindowId::new(1, 1); + make_active_app(&mut apps, &mut reactor, 1, make_windows(1), Some(window)); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(left_space)); + + reactor.handle_test_layout_command(LayoutCommand::MoveNode(Direction::Right)); + apps.simulate_until_quiet(&mut reactor); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + assert_eq!( + reactor.space_state.command_space, + Some(right_space), + "commands follow the window to the display it moved to" + ); + + reactor.handle_test_layout_command(LayoutCommand::MoveNode(Direction::Left)); + apps.simulate_until_quiet(&mut reactor); + assert_eq!( + reactor.assigned_space_for_window_id(window), + Some(left_space), + "so the next move-node brings it straight back" + ); +} + #[test] fn passive_command_space_change_does_not_override_clicked_window_focus() { let (mut apps, mut reactor) = test_context(); @@ -2233,6 +2264,59 @@ fn recent_cross_display_move_ignores_conflicting_geometry_space_change() { assert_eq!(reactor.state.windows.window_server_space(wsid), Some(space2)); } +/// The window server still reports the window on `space`, and a reconcile plus a +/// stale frame report from the app arrive before macOS catches up with the move. +fn deliver_lagging_reports(reactor: &mut Reactor, window: WindowId, space: SpaceId, frame: CGRect) { + let wsid = reactor.test_window_server_id(window); + crate::sys::window_server::set_window_spaces_override(wsid, Some(vec![space.get()])); + reactor.reconcile_authoritative_active_window_snapshot(vec![(wsid, Some(space))], false, &[]); + reactor.handle_event(Event::WindowFrameChanged( + window, + frame, + None, + Requested(false), + Some(MouseState::Up), + )); +} + +#[test] +fn a_window_moved_to_another_display_holds_there_until_macos_catches_up() { + let mut reactor = test_reactor(); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let mut apps = Apps::new(); + let window = WindowId::new(1, 1); + make_active_app(&mut apps, &mut reactor, 1, make_windows(1), Some(window)); + let original_frame = reactor.state.windows.window(window).unwrap().frame_monotonic; + + reactor.handle_event(Event::Command(Command::Reactor( + ReactorCommand::MoveWindowToDisplay { + selector: DisplaySelector::Index(1), + window_id: None, + }, + ))); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + + deliver_lagging_reports(&mut reactor, window, left_space, original_frame); + assert_eq!( + reactor.assigned_space_for_window_id(window), + Some(right_space), + "a lagging report of the old display is not the user moving the window" + ); + + // Once the grace period is over, a real move to the other display is followed. + reactor.expire_display_moves_for_test(); + deliver_lagging_reports(&mut reactor, window, left_space, original_frame); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(left_space)); + crate::sys::window_server::set_window_spaces_override( + reactor.test_window_server_id(window), + None, + ); +} + #[test] fn central_space_resolution_prefers_recent_move_target_over_stale_server_space() { let (mut reactor, wid, wsid, space1, space2, moved_frame) = @@ -3417,6 +3501,151 @@ fn native_focus_race_waits_for_new_window_activation() { assert!(raise_rx.try_recv().is_err()); } +fn reactor_for_new_window_placement( + policy: crate::common::config::NewWindowDisplay, + settings: crate::common::config::VirtualWorkspaceSettings, +) -> (Apps, Reactor, SpaceId, SpaceId) { + let mut reactor = test_reactor_with_workspace_settings(&settings); + reactor.config.virtual_workspaces = settings; + reactor.config.settings.new_window_display = policy; + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let mut apps = Apps::new(); + make_active_app( + &mut apps, + &mut reactor, + 1, + make_windows(1), + Some(WindowId::new(1, 1)), + ); + (apps, reactor, left_space, right_space) +} + +/// The app opens a new window on the left display, as macOS decided. +fn open_new_window_on_left(reactor: &mut Reactor, apps: &mut Apps) -> WindowId { + let window = WindowId::new(1, 1002); + let wsid = WindowServerId::new(10002); + let frame = CGRect::new(CGPoint::new(200., 200.), CGSize::new(400., 400.)); + reactor.handle_event(Event::WindowCreated( + window, + make_window_info(frame, Some(wsid), "New Window", None), + Some(crate::sys::window_server::WindowServerInfo { + id: wsid, + pid: 1, + layer: 0, + frame, + min_frame: frame.size, + max_frame: frame.size, + }), + None, + )); + apps.simulate_until_quiet(reactor); + window +} + +#[test] +fn new_windows_open_on_the_display_the_setting_names() { + use crate::common::config::{NewWindowDisplay, VirtualWorkspaceSettings}; + let on_right = CGPoint::new(1500., 500.); + + let (mut apps, mut reactor, _left, right_space) = reactor_for_new_window_placement( + NewWindowDisplay::Cursor, + VirtualWorkspaceSettings::default(), + ); + crate::sys::window_server::set_cursor_location_override(Some(on_right)); + let window = open_new_window_on_left(&mut reactor, &mut apps); + crate::sys::window_server::set_cursor_location_override(None); + let right_active = + reactor.layout_manager.layout_engine.workspaces().active_workspace(right_space); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + assert_eq!( + reactor.test_workspace_for_window(right_space, window), + right_active + ); + + let (mut apps, mut reactor, _left, right_space) = reactor_for_new_window_placement( + NewWindowDisplay::Focused, + VirtualWorkspaceSettings::default(), + ); + reactor.space_state.command_space = Some(right_space); + let window = open_new_window_on_left(&mut reactor, &mut apps); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + + let (mut apps, mut reactor, left_space, _right) = reactor_for_new_window_placement( + NewWindowDisplay::Default, + VirtualWorkspaceSettings::default(), + ); + crate::sys::window_server::set_cursor_location_override(Some(on_right)); + let window = open_new_window_on_left(&mut reactor, &mut apps); + crate::sys::window_server::set_cursor_location_override(None); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(left_space)); +} + +#[test] +fn an_app_rule_naming_a_workspace_wins_over_new_window_display() { + use crate::common::config::NewWindowDisplay; + let settings = crate::common::config::VirtualWorkspaceSettings { + app_rules: vec![crate::common::config::AppWorkspaceRule { + app_id: Some("com.testapp1".into()), + workspace: Some(WorkspaceSelector::Index(1)), + ..Default::default() + }], + ..Default::default() + }; + let (mut apps, mut reactor, left_space, _right) = + reactor_for_new_window_placement(NewWindowDisplay::Cursor, settings); + crate::sys::window_server::set_cursor_location_override(Some(CGPoint::new(1500., 500.))); + let window = open_new_window_on_left(&mut reactor, &mut apps); + crate::sys::window_server::set_cursor_location_override(None); + let left_workspaces = reactor.test_workspace_ids(left_space); + assert_eq!( + reactor.test_workspace_for_window(left_space, window), + Some(left_workspaces[1]) + ); +} + +#[test] +fn a_new_window_placed_on_the_cursor_display_is_not_pulled_back_by_lagging_reports() { + use crate::common::config::{NewWindowDisplay, VirtualWorkspaceSettings}; + let (mut apps, mut reactor, left_space, right_space) = reactor_for_new_window_placement( + NewWindowDisplay::Cursor, + VirtualWorkspaceSettings::default(), + ); + crate::sys::window_server::set_cursor_location_override(Some(CGPoint::new(500., 500.))); + let window = WindowId::new(1, 1002); + let wsid = WindowServerId::new(10002); + let frame = CGRect::new(CGPoint::new(1200., 200.), CGSize::new(400., 400.)); + crate::sys::window_server::set_window_spaces_override(wsid, Some(vec![right_space.get()])); + reactor.handle_event(Event::WindowCreated( + window, + make_window_info(frame, Some(wsid), "New Window", None), + Some(crate::sys::window_server::WindowServerInfo { + id: wsid, + pid: 1, + layer: 0, + frame, + min_frame: frame.size, + max_frame: frame.size, + }), + None, + )); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(left_space)); + + deliver_lagging_reports(&mut reactor, window, right_space, frame); + apps.simulate_until_quiet(&mut reactor); + + assert_eq!( + reactor.assigned_space_for_window_id(window), + Some(left_space), + "reports of the old display while macOS catches up must not move the window back" + ); + crate::sys::window_server::set_window_spaces_override(wsid, None); + crate::sys::window_server::set_cursor_location_override(None); +} + fn pending_activation_context() -> (Apps, Reactor, SpaceId, WindowId, WindowServerInfo) { let (mut apps, mut reactor) = test_context(); let space = SpaceId::new(1); @@ -8042,3 +8271,543 @@ fn topology_snapshot_preserves_workspace_placement_with_incomplete_delta() { } } } + +/// Settings with one workspace per entry, named `ws0`, `ws1`, …, each bound as +/// given by a workspace rule. +fn bound_workspace_settings( + bindings: Vec>, +) -> crate::common::config::VirtualWorkspaceSettings { + use crate::common::config::{VirtualWorkspaceSettings, WorkspaceLayoutRule, WorkspaceSelector}; + VirtualWorkspaceSettings { + default_workspace_count: bindings.len(), + workspace_names: (0..bindings.len()).map(|index| format!("ws{index}")).collect(), + workspace_rules: bindings + .into_iter() + .enumerate() + .filter_map(|(index, display)| { + Some(WorkspaceLayoutRule { + workspace: WorkspaceSelector::Index(index), + layout: None, + display: Some(display?), + }) + }) + .collect(), + ..Default::default() + } +} + +/// Workspaces 0 and 1 on the left display, 2 and 3 on the right one. +fn left_right_bindings() -> Vec> { + let left = || Some(DisplaySelector::Uuid("test-display-0".into())); + let right = || Some(DisplaySelector::Uuid("test-display-1".into())); + vec![left(), left(), right(), right()] +} + +fn bound_reactor(settings: crate::common::config::VirtualWorkspaceSettings) -> Reactor { + let mut reactor = test_reactor_with_workspace_settings(&settings); + reactor.config.virtual_workspaces = settings; + reactor +} + +fn active_workspace_of( + reactor: &Reactor, + space: SpaceId, +) -> Option { + reactor.layout_manager.layout_engine.workspaces().active_workspace(space) +} + +#[test] +fn displays_start_on_a_workspace_bound_to_them_and_keep_windows_found_there() { + let mut reactor = bound_reactor(bound_workspace_settings(left_right_bindings())); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + // No display-set change flag: the starting workspace must not depend on the + // later display-change pass, which a startup snapshot does not always get. + reactor.handle_event(space_state_event(vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ])); + let mut apps = Apps::new(); + let mut on_right = make_window(1); + on_right.frame = CGRect::new(CGPoint::new(1200., 100.), CGSize::new(400., 400.)); + apps.make_app_and_settle(&mut reactor, 1, vec![on_right, make_window(2)]); + + let left_workspaces = reactor.test_workspace_ids(left_space); + let right_workspaces = reactor.test_workspace_ids(right_space); + assert_eq!( + active_workspace_of(&reactor, left_space), + Some(left_workspaces[0]) + ); + assert_eq!( + active_workspace_of(&reactor, right_space), + Some(right_workspaces[2]), + "the right display starts on its first bound workspace, not the left's default" + ); + assert_eq!( + reactor.test_workspace_for_window(right_space, WindowId::new(1, 1)), + Some(right_workspaces[2]), + "a window found on the right display stays there" + ); + assert_eq!( + reactor.test_workspace_for_window(left_space, WindowId::new(1, 2)), + Some(left_workspaces[0]) + ); +} + +#[test] +fn cycling_and_back_and_forth_skip_workspaces_bound_to_other_displays() { + let mut settings = bound_workspace_settings(left_right_bindings()); + settings.workspace_auto_back_and_forth = true; + let mut reactor = bound_reactor(settings); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let left_workspaces = reactor.test_workspace_ids(left_space); + let active = |reactor: &Reactor| active_workspace_of(reactor, left_space); + + reactor.handle_test_layout_command(LayoutCommand::NextWorkspace(None)); + assert_eq!(active(&reactor), Some(left_workspaces[1])); + reactor.handle_test_layout_command(LayoutCommand::NextWorkspace(None)); + assert_eq!( + active(&reactor), + Some(left_workspaces[0]), + "cycling wraps past the right display's workspaces" + ); + reactor.handle_test_layout_command(LayoutCommand::PrevWorkspace(None)); + assert_eq!(active(&reactor), Some(left_workspaces[1])); + + // A back-and-forth target on the left display that belongs to the right one + // is never used; a local one still is. + assert!(reactor.set_test_active_workspace(left_space, left_workspaces[2])); + assert!(reactor.set_test_active_workspace(left_space, left_workspaces[0])); + reactor.handle_test_layout_command(LayoutCommand::SwitchToWorkspace(0)); + reactor.handle_test_layout_command(LayoutCommand::SwitchToLastWorkspace); + assert_eq!(active(&reactor), Some(left_workspaces[0])); + reactor.handle_test_layout_command(LayoutCommand::SwitchToWorkspace(1)); + reactor.handle_test_layout_command(LayoutCommand::SwitchToWorkspace(1)); + assert_eq!(active(&reactor), Some(left_workspaces[0])); +} + +/// Workspace 1 bound to the right one of two displays, 0 and 2 unbound, with two +/// windows on the left display. +fn two_display_reactor_with_bound_workspace() -> (Apps, Reactor, SpaceId, SpaceId) { + let mut reactor = bound_reactor(bound_workspace_settings(vec![ + None, + Some(DisplaySelector::Uuid("test-display-1".into())), + None, + ])); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let mut apps = Apps::new(); + apps.make_app_and_settle(&mut reactor, 1, make_windows(2)); + (apps, reactor, left_space, right_space) +} + +#[test] +fn switching_to_a_bound_workspace_routes_to_the_owning_display() { + let (mut apps, mut reactor, left_space, right_space) = + two_display_reactor_with_bound_workspace(); + assert_eq!(reactor.space_state.command_space, Some(left_space)); + let left_workspaces = reactor.test_workspace_ids(left_space); + let right_workspaces = reactor.test_workspace_ids(right_space); + + reactor.handle_test_layout_command(LayoutCommand::SwitchToWorkspace(1)); + apps.simulate_until_quiet(&mut reactor); + + let workspaces = reactor.layout_manager.layout_engine.workspaces(); + assert_eq!( + workspaces.active_workspace(right_space), + Some(right_workspaces[1]), + "the bound workspace should activate on its display" + ); + assert_eq!( + workspaces.active_workspace(left_space), + Some(left_workspaces[0]), + "the display the command came from should keep its workspace" + ); + assert_eq!( + reactor.space_state.command_space, + Some(right_space), + "command context should follow the bound display" + ); + + // Unbound workspaces still switch on the current display. + reactor.space_state.command_space = Some(left_space); + reactor.handle_test_layout_command(LayoutCommand::SwitchToWorkspace(2)); + apps.simulate_until_quiet(&mut reactor); + let workspaces = reactor.layout_manager.layout_engine.workspaces(); + assert_eq!(workspaces.active_workspace(left_space), Some(left_workspaces[2])); + assert_eq!( + workspaces.active_workspace(right_space), + Some(right_workspaces[1]) + ); +} + +#[test] +fn moving_a_window_to_a_bound_workspace_relocates_it_to_the_owning_display() { + let (mut apps, mut reactor, left_space, right_space) = + two_display_reactor_with_bound_workspace(); + let right_workspaces = reactor.test_workspace_ids(right_space); + let moved = WindowId::new(1, 2); + let stays = WindowId::new(1, 1); + + reactor.handle_test_layout_command(LayoutCommand::MoveWindowToWorkspace { + workspace: WorkspaceSelector::Name("ws1".into()), + follow: false, + window_id: Some(2), + }); + apps.simulate_until_quiet(&mut reactor); + + assert_eq!(reactor.assigned_space_for_window_id(moved), Some(right_space)); + assert_eq!( + reactor.test_workspace_for_window(right_space, moved), + Some(right_workspaces[1]) + ); + assert_eq!(reactor.assigned_space_for_window_id(stays), Some(left_space)); + assert_eq!( + reactor.layout_manager.layout_engine.workspaces().active_workspace(right_space), + Some(right_workspaces[0]), + "without follow the owning display keeps its active workspace" + ); + assert_eq!(reactor.space_state.command_space, Some(left_space)); + let frame = reactor.state.windows.window(moved).unwrap().frame_monotonic; + assert!( + frame.origin.x >= 1000., + "window frame should be placed on the right display: {frame:?}" + ); + + reactor.handle_test_layout_command(LayoutCommand::MoveWindowToWorkspace { + workspace: WorkspaceSelector::Index(1), + follow: true, + window_id: Some(1), + }); + apps.simulate_until_quiet(&mut reactor); + + assert_eq!(reactor.assigned_space_for_window_id(stays), Some(right_space)); + assert_eq!( + reactor.layout_manager.layout_engine.workspaces().active_workspace(right_space), + Some(right_workspaces[1]), + "follow should activate the bound workspace on its display" + ); + assert_eq!(reactor.space_state.command_space, Some(right_space)); +} + +#[test] +fn selecting_a_foreign_copy_in_the_overview_switches_on_the_owning_display() { + let mut reactor = bound_reactor(bound_workspace_settings(left_right_bindings())); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let left_workspaces = reactor.test_workspace_ids(left_space); + let right_workspaces = reactor.test_workspace_ids(right_space); + + reactor.handle_event(Event::OverviewSelectWorkspace { + display: "test-display-1".into(), + workspace: right_workspaces[1], + }); + + assert_eq!( + active_workspace_of(&reactor, right_space), + Some(right_workspaces[2]), + "the right display keeps its own workspace" + ); + assert_eq!( + active_workspace_of(&reactor, left_space), + Some(left_workspaces[1]) + ); + assert_eq!(reactor.space_state.command_space, Some(left_space)); +} + +#[test] +fn moving_a_bound_workspace_to_another_display_is_refused() { + let mut reactor = bound_reactor(bound_workspace_settings(left_right_bindings())); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let mut apps = Apps::new(); + apps.make_app_and_settle(&mut reactor, 1, make_windows(2)); + + reactor.handle_event(Event::Command(Command::Reactor( + ReactorCommand::MoveWorkspaceToDisplay { + selector: DisplaySelector::Index(1), + wrap_around: false, + }, + ))); + + for index in 1..=2 { + assert_eq!( + reactor.assigned_space_for_window_id(WindowId::new(1, index)), + Some(left_space) + ); + } +} + +/// Nine workspaces: 1-5 bound to the left display, 6-9 to the right one. +fn nine_bound_workspace_settings() -> crate::common::config::VirtualWorkspaceSettings { + let left = || Some(DisplaySelector::Uuid("test-display-0".into())); + let right = || Some(DisplaySelector::Uuid("test-display-1".into())); + bound_workspace_settings(vec![ + left(), + left(), + left(), + left(), + left(), + right(), + right(), + right(), + right(), + ]) +} + +fn window_in_workspace_nine_on_the_right( + settings: crate::common::config::VirtualWorkspaceSettings, + right_shows_it: bool, +) -> (Apps, Reactor, WindowId, SpaceId, SpaceId) { + let mut reactor = test_reactor_with_workspace_settings(&settings); + reactor.config.virtual_workspaces = settings; + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + let mut apps = Apps::new(); + let mut on_right = make_window(1); + on_right.frame = CGRect::new(CGPoint::new(1200., 100.), CGSize::new(400., 400.)); + apps.make_app_and_settle(&mut reactor, 1, vec![on_right, make_window(2)]); + let window = WindowId::new(1, 1); + let right_workspaces = reactor.test_workspace_ids(right_space); + assert!(reactor.assign_test_window_to_workspace(right_space, window, right_workspaces[8])); + let shown = if right_shows_it { + right_workspaces[8] + } else { + right_workspaces[5] + }; + assert!(reactor.set_test_active_workspace(right_space, shown)); + apps.simulate_until_quiet(&mut reactor); + (apps, reactor, window, left_space, right_space) +} + +fn report_windows_on(reactor: &Reactor, space: SpaceId, windows: &[WindowId]) { + let ids: Vec = + windows.iter().map(|wid| reactor.test_window_server_id(*wid).as_u32()).collect(); + for wid in windows { + crate::sys::window_server::set_window_spaces_override( + reactor.test_window_server_id(*wid), + Some(vec![space.get()]), + ); + } + crate::sys::window_server::set_space_window_list_for_space_override(space.get(), Some(ids)); +} + +fn clear_window_reports(reactor: &Reactor, spaces: &[SpaceId], windows: &[WindowId]) { + for wid in windows { + crate::sys::window_server::set_window_spaces_override( + reactor.test_window_server_id(*wid), + None, + ); + } + for space in spaces { + crate::sys::window_server::set_space_window_list_for_space_override(space.get(), None); + } +} + +fn unplug_right_display( + reactor: &mut Reactor, + left_space: SpaceId, + right_space: SpaceId, + delta: bool, +) { + let windows = [WindowId::new(1, 1), WindowId::new(1, 2)]; + report_windows_on(reactor, left_space, &windows); + let moved = reactor.test_window_server_id(windows[0]); + let others: Vec<_> = windows.iter().map(|wid| reactor.test_window_server_id(*wid)).collect(); + reactor.handle_event(space_state_event_with( + vec![left_screen()], + vec![Some(left_space)], + |state| { + state.display_set_changed = true; + state.should_force_refresh_layout = true; + state.membership_complete = true; + for wsid in &others { + state.active_window_spaces.insert(*wsid, left_space); + } + if delta { + state.topology_window_delta = Some(crate::actor::spaces::TopologyWindowDelta { + appeared: vec![(moved, left_space)], + disappeared: vec![(moved, right_space)], + ..Default::default() + }); + } + }, + )); +} + +#[test] +fn bound_window_returns_to_its_display_after_unplug_and_replug() { + // On replug either rift moves the window home, or macOS puts it back on the + // returning display first and rift only learns where it went. That display + // shows workspace 6, so joining the workspace it shows would be wrong. + for macos_moves_it_back in [false, true] { + let (mut apps, mut reactor, window, left_space, right_space) = + window_in_workspace_nine_on_the_right(nine_bound_workspace_settings(), false); + unplug_right_display(&mut reactor, left_space, right_space, false); + apps.simulate_until_quiet(&mut reactor); + let left_workspaces = reactor.test_workspace_ids(left_space); + assert_eq!( + reactor.test_workspace_for_window(left_space, window), + Some(left_workspaces[8]) + ); + + let other = WindowId::new(1, 2); + clear_window_reports(&reactor, &[left_space], &[window, other]); + let wsid = reactor.test_window_server_id(window); + if macos_moves_it_back { + report_windows_on(&reactor, right_space, &[window]); + report_windows_on(&reactor, left_space, &[other]); + } + reactor.handle_event(space_state_event_with( + vec![left_screen(), right_screen()], + vec![Some(left_space), Some(right_space)], + |state| { + state.display_set_changed = true; + state.should_force_refresh_layout = true; + if macos_moves_it_back { + state.active_window_spaces.insert(wsid, right_space); + } + }, + )); + apps.simulate_until_quiet(&mut reactor); + + let right_workspaces = reactor.test_workspace_ids(right_space); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + assert_eq!( + reactor.test_workspace_for_window(right_space, window), + Some(right_workspaces[8]), + "macos_moves_it_back={macos_moves_it_back}: the window is back in workspace 9" + ); + clear_window_reports(&reactor, &[left_space, right_space], &[window, other]); + } +} + +#[test] +fn reconnecting_a_display_takes_back_its_workspace_and_windows() { + let mut reactor = bound_reactor(bound_workspace_settings(left_right_bindings())); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + connect_displays(&mut reactor, vec![left_screen()], vec![Some(left_space)]); + // With the right display absent its workspaces fall back to the left one. + reactor.handle_test_layout_command(LayoutCommand::SwitchToWorkspace(3)); + let mut apps = Apps::new(); + apps.make_app_and_settle(&mut reactor, 1, make_windows(2)); + let left_workspaces = reactor.test_workspace_ids(left_space); + assert_eq!( + active_workspace_of(&reactor, left_space), + Some(left_workspaces[3]) + ); + assert_eq!( + reactor.test_workspace_for_window(left_space, WindowId::new(1, 1)), + Some(left_workspaces[3]) + ); + + connect_displays(&mut reactor, vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ]); + // Applied as part of the reconnect snapshot, before any window is rediscovered. + + let right_workspaces = reactor.test_workspace_ids(right_space); + assert_eq!( + active_workspace_of(&reactor, left_space), + Some(left_workspaces[0]), + "the left display goes back to its own workspace" + ); + assert_eq!( + active_workspace_of(&reactor, right_space), + Some(right_workspaces[3]), + "the workspace the user was on moves to its display" + ); + for index in 1..=2 { + let window = WindowId::new(1, index); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + assert_eq!( + reactor.test_workspace_for_window(right_space, window), + Some(right_workspaces[3]) + ); + } +} + +#[test] +fn app_rule_targeting_a_bound_workspace_places_new_windows_on_the_owning_display() { + let mut settings = + bound_workspace_settings(vec![None, Some(DisplaySelector::Uuid("test-display-1".into()))]); + settings.app_rules = vec![crate::common::config::AppWorkspaceRule { + app_id: Some("com.testapp1".into()), + workspace: Some(WorkspaceSelector::Name("ws1".into())), + ..Default::default() + }]; + let mut reactor = bound_reactor(settings); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + reactor.handle_event(space_state_event(vec![left_screen(), right_screen()], vec![ + Some(left_space), + Some(right_space), + ])); + let mut apps = Apps::new(); + let window = WindowId::new(1, 1); + + make_active_app(&mut apps, &mut reactor, 1, make_windows(1), Some(window)); + + let right_workspaces = reactor.test_workspace_ids(right_space); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(right_space)); + assert_eq!( + reactor.test_workspace_for_window(right_space, window), + Some(right_workspaces[1]) + ); + let frame = reactor.state.windows.window(window).unwrap().frame_monotonic; + assert!( + frame.origin.x >= 1000., + "window should be placed on the right display: {frame:?}" + ); +} + +#[test] +fn binding_work_waits_while_the_native_topology_is_invalidated() { + let mut reactor = bound_reactor(bound_workspace_settings(left_right_bindings())); + let (left_space, right_space) = (SpaceId::new(1), SpaceId::new(2)); + let both = || vec![left_screen(), right_screen()]; + connect_displays(&mut reactor, both(), vec![Some(left_space), Some(right_space)]); + let mut apps = Apps::new(); + apps.make_app_and_settle(&mut reactor, 1, make_windows(1)); + let window = WindowId::new(1, 1); + assert_eq!(reactor.assigned_space_for_window_id(window), Some(left_space)); + + // Sleep or display churn invalidates the native topology, and meanwhile the + // window turns up in the left display's copy of a workspace bound right. + reactor.handle_event(Event::TopologyInvalidated(next_test_topology_revision())); + let bound_right = reactor.test_workspace(left_space, 3); + assert!(reactor.assign_test_window_to_workspace(left_space, window, bound_right)); + reactor.check_display_bindings_later(); + reactor.apply_pending_display_bindings(); + assert_eq!( + reactor.assigned_space_for_window_id(window), + Some(left_space), + "nothing moves on window and space data from an unsettled system" + ); + + reactor.handle_event(space_state_event(both(), vec![ + Some(left_space), + Some(right_space), + ])); + apps.simulate_until_quiet(&mut reactor); + assert_eq!( + reactor.assigned_space_for_window_id(window), + Some(right_space), + "the authoritative snapshot that ends the instability runs the queued work" + ); +} diff --git a/src/actor/reactor/workspace_bindings.rs b/src/actor/reactor/workspace_bindings.rs new file mode 100644 index 000000000..521d64a8c --- /dev/null +++ b/src/actor/reactor/workspace_bindings.rs @@ -0,0 +1,437 @@ +//! Workspace display bindings. +//! +//! `workspace_rules` may bind a workspace to a display. Every native space still +//! owns the full workspace list, so a binding is enforced by routing rather than +//! by a different topology. While the owning display is connected, a bound +//! workspace is only ever shown there: +//! +//! - Each display starts on a workspace bound to it, or an unbound one. +//! - Cycling, relative window moves and back-and-forth on a display skip the +//! workspaces bound to other displays. +//! - `switch_to_workspace`, or picking the workspace in the overview, focuses +//! the owning display and switches there; `move_window_to_workspace` sends +//! the window to the owning display's copy of the workspace, and +//! `move_workspace_to_display` will not take a bound workspace off its display. +//! - Windows that end up in a bound workspace on another display (an app rule, +//! an overview drop, or macOS parking them while the owner was unplugged) move +//! to the owner, and after a display change a display showing a workspace +//! bound elsewhere switches back to one of its own. +//! +//! When the owning display is not connected the workspace behaves like an +//! unbound one on whatever display the user is on. + +use tracing::warn; + +use super::{DisplaySelector, EventOutcome, Reactor, ScreenInfo}; +use crate::actor::app::WindowId; +use crate::actor::reactor::events::command as command_workflow; +use crate::common::collections::HashSet; +use crate::layout_engine::LayoutCommand; +use crate::model::VirtualWorkspaceId; +use crate::sys::screen::SpaceId; + +/// Order screens by physical arrangement: left to right, then top to bottom. +/// +/// `DisplaySelector::Index` counts displays in this order. +pub(crate) fn physical_order(screens: &[ScreenInfo]) -> Vec<&ScreenInfo> { + let mut ordered: Vec<&ScreenInfo> = screens.iter().collect(); + ordered.sort_by(|a, b| { + a.frame + .origin + .x + .total_cmp(&b.frame.origin.x) + .then_with(|| a.frame.origin.y.total_cmp(&b.frame.origin.y)) + }); + ordered +} + +/// The screen among `screens` that a workspace binding names. +pub(crate) fn bound_screen<'a>( + screens: &'a [ScreenInfo], + binding: &DisplaySelector, +) -> Option<&'a ScreenInfo> { + match binding { + DisplaySelector::Uuid(uuid) => screens.iter().find(|screen| screen.display_uuid == *uuid), + DisplaySelector::Index(index) => physical_order(screens).get(*index).copied(), + DisplaySelector::Direction(_) => None, + } +} + +fn same_screen(a: &ScreenInfo, b: &ScreenInfo) -> bool { + a.id == b.id && a.display_uuid == b.display_uuid +} + +impl Reactor { + pub(crate) fn has_workspace_display_bindings(&self) -> bool { + self.config.virtual_workspaces.has_display_bindings() + } + + /// Tell the workspace manager, for every display in `screens`, which + /// workspace it starts on and which workspaces are bound to another one. + /// + /// Runs on each forwarded snapshot before any new native space gets its + /// workspaces, so windows found on a display land in one of its own, and on + /// config reload. + pub(crate) fn refresh_display_bindings(&mut self, screens: &[ScreenInfo]) { + if !self.has_workspace_display_bindings() { + self.layout_manager.layout_engine.workspaces_mut().set_display_bindings([]); + return; + } + let settings = &self.config.virtual_workspaces; + let count = settings.default_workspace_count.max(1); + let default = settings.default_workspace; + // Per workspace: `None` when unbound, else its owner if connected. + let owners: Vec>> = (0..count) + .map(|index| { + settings + .display_binding_for_workspace(index) + .map(|binding| bound_screen(screens, binding)) + }) + .collect(); + let bindings: Vec<_> = screens + .iter() + .filter_map(|screen| { + let owned_here = + |owner: &Option>| matches!(owner, Some(Some(owner)) if same_screen(owner, screen)); + let foreign: HashSet = (0..count) + .filter(|index| matches!(owners[*index], Some(Some(_))) && !owned_here(&owners[*index])) + .collect(); + // default_workspace if it may live here, else the first workspace + // bound here, else the first unbound one, else one not foreign. + let start = (default < count && !foreign.contains(&default)) + .then_some(default) + .or_else(|| owners.iter().position(owned_here)) + .or_else(|| owners.iter().position(Option::is_none)) + .or_else(|| (0..count).find(|index| !foreign.contains(index))); + Some((screen.space?, start, foreign)) + }) + .collect(); + self.layout_manager + .layout_engine + .workspaces_mut() + .set_display_bindings(bindings); + } + + /// The active native space of the display workspace `index` is bound to, if + /// that display is connected. + fn bound_space_for_workspace_index(&self, index: usize) -> Option { + let binding = self.config.virtual_workspaces.display_binding_for_workspace(index)?; + let screen = bound_screen(&self.space_state.screens, binding)?; + screen.space.filter(|space| self.is_space_active(*space)) + } + + fn workspace_ordinal(&self, space: SpaceId, workspace: VirtualWorkspaceId) -> Option { + let workspaces = self.layout_manager.layout_engine.workspaces(); + workspaces.workspace_ids(space).iter().position(|id| *id == workspace) + } + + /// Redirect commands that show, or move a window to, a workspace bound to + /// another display. `None` means the command takes the regular path. + pub(crate) fn route_bound_workspace_command( + &mut self, + command: &LayoutCommand, + ) -> Option> { + if !self.has_workspace_display_bindings() { + return None; + } + match command { + LayoutCommand::SwitchToWorkspace(index) => { + let owner = self.bound_space_for_workspace_index(*index)?; + (self.command_context_space() != Some(owner)) + .then(|| self.switch_to_bound_workspace(owner, *index)) + } + LayoutCommand::MoveWindowToWorkspace { workspace, follow, window_id } => { + let window = self.resolve_command_window(*window_id)?; + let source = self + .assigned_space_for_window_id(window) + .filter(|space| self.is_space_active(*space))?; + let workspaces = self.layout_manager.layout_engine.workspaces(); + let target = workspaces.resolve_workspace(source, workspace)?; + let index = self.workspace_ordinal(source, target)?; + let owner = + self.bound_space_for_workspace_index(index).filter(|owner| *owner != source)?; + Some(self.move_window_to_bound_workspace(window, source, owner, index, *follow)) + } + _ => None, + } + } + + /// Picking a display's copy of a workspace bound elsewhere in the overview + /// switches to it on its owner instead. + pub(crate) fn route_overview_workspace_selection( + &mut self, + space: SpaceId, + index: usize, + ) -> Option> { + let owner = self.bound_space_for_workspace_index(index).filter(|owner| *owner != space)?; + Some(self.switch_to_bound_workspace(owner, index)) + } + + /// Whether moving the workspace `source` shows to `target` would take a bound + /// workspace off its display. + pub(crate) fn bound_workspace_blocks_display_move( + &self, + source: SpaceId, + target: SpaceId, + ) -> bool { + let active = self.layout_manager.layout_engine.workspaces().active_workspace(source); + active + .and_then(|active| self.workspace_ordinal(source, active)) + .and_then(|index| self.bound_space_for_workspace_index(index)) + .is_some_and(|owner| owner != target) + } + + /// Resolve the window a workspace command acts on. A window named by id may + /// live on any display, not just the one holding the command context. + fn resolve_command_window(&self, window_id: Option) -> Option { + let Some(idx) = window_id else { + return self.layout_manager.layout_engine.focused_window(); + }; + let workspaces = self.layout_manager.layout_engine.workspaces(); + self.command_context_space() + .into_iter() + .chain(self.iter_active_spaces()) + .find_map(|space| workspaces.find_window_by_idx(&self.state.windows, space, idx)) + } + + /// Run a layout command through the regular command path on `space`. + fn dispatch_layout_command_on( + &mut self, + command: LayoutCommand, + space: SpaceId, + ) -> anyhow::Result { + let post_arrange_mouse_warp = + self.config.settings.mouse_follows_focus.then(|| self.main_window()).flatten(); + let (visible_spaces, visible_space_frames) = self.visible_spaces_for_layout(false); + command_workflow::handle_command_layout( + &mut self.state, + &mut self.layout_manager, + &mut self.workspace_switch_manager, + command_workflow::LayoutCommandPayload { + command, + command_space: Some(space), + visible_spaces, + visible_space_frames, + post_arrange_mouse_warp, + }, + ) + } + + /// Make `screen` the command and menu-bar context, as `focus_display` does. + fn adopt_display_context(&mut self, screen: &ScreenInfo) { + if crate::sys::screen::set_active_menu_bar_display_uuid(&screen.display_uuid) { + self.space_state.menu_bar_space = screen.space; + } + self.space_state.command_space = screen.space; + } + + fn switch_to_bound_workspace( + &mut self, + owner: SpaceId, + index: usize, + ) -> anyhow::Result { + let Some(screen) = self.space_state.screen_by_space(owner).cloned() else { + return Ok(EventOutcome::no_change()); + }; + let workspaces = self.layout_manager.layout_engine.workspaces_mut(); + if workspaces.workspace_id_at(owner, Some(index)) == workspaces.active_workspace(owner) { + // Already showing there: go there, as focus_display does. + return self.focus_display_by_selector(&DisplaySelector::Uuid(screen.display_uuid)); + } + self.adopt_display_context(&screen); + let outcome = + self.dispatch_layout_command_on(LayoutCommand::SwitchToWorkspace(index), owner)?; + // An empty workspace has nothing to focus; warp the cursor so the user + // still lands on the display they asked for. + let focuses = outcome + .layout_responses + .iter() + .any(|(response, _)| response.focus_window.is_some()); + Ok(if focuses { + outcome + } else { + outcome.with_mouse_warp(screen.frame.mid()) + }) + } + + fn move_window_to_bound_workspace( + &mut self, + window: WindowId, + source: SpaceId, + owner: SpaceId, + index: usize, + follow: bool, + ) -> anyhow::Result { + let Some(screen) = self.space_state.screen_by_space(owner).cloned() else { + return Ok(EventOutcome::no_change()); + }; + let Some(state) = self.state.windows.window(window).filter(|_| !self.is_in_drag()) else { + return Ok(EventOutcome::no_change()); + }; + let (window_server_id, frame) = (state.info.sys_id, state.frame_monotonic); + let target_workspace = self + .layout_manager + .layout_engine + .workspaces_mut() + .workspace_id_at(owner, Some(index)); + let outcome = command_workflow::handle_command_reactor_move_window_to_display( + &mut self.state, + &mut self.layout_manager, + command_workflow::MoveWindowToDisplayPayload { + window, + window_server_id, + source_space: source, + target_space: owner, + target_screen: screen.frame, + target_frame: Self::center_frame_on_screen(frame, screen.frame), + target_workspace, + follow, + }, + )?; + self.note_display_move_in_flight(window, owner); + if follow { + self.workspace_switch_manager + .start_workspace_switch(super::WorkspaceSwitchOrigin::Manual); + self.adopt_display_context(&screen); + } + Ok(outcome) + } + + /// Whether a window sits in a workspace bound to the display now showing `space`. + pub(crate) fn window_returns_to_bound_display(&self, wid: WindowId, space: SpaceId) -> bool { + let Some(assignment) = self.state.windows.workspace_info_for_window(wid) else { + return false; + }; + self.workspace_ordinal(assignment.space, assignment.workspace_id) + .and_then(|index| self.bound_space_for_workspace_index(index)) + == Some(space) + } + + /// Re-apply bindings once the current event's outcome has settled. + pub(crate) fn check_display_bindings_later(&mut self) { + if self.has_workspace_display_bindings() { + self.bindings_need_check = true; + } + } + + /// Re-apply bindings if an event asked for it: displays showing a workspace + /// bound elsewhere switch back to one of their own, and windows in a bound + /// workspace on another display move to its owner. + pub(crate) fn apply_pending_display_bindings(&mut self) { + // The moves emit layout events that ask for another check, which finds + // everything in place. The bound guards against two displays handing a + // window back and forth. + for _ in 0..3 { + if !self.bindings_need_check { + return; + } + if self.refreshes_blocked() || self.is_in_drag() || self.is_mission_control_active() { + // During sleep, wake, lock and display churn window and space data + // is transient; moving windows on it fights macOS. The + // authoritative snapshot that ends the instability runs this. + return; + } + self.bindings_need_check = false; + let mut outcome = EventOutcome::no_change(); + let normalized = self.normalize_bound_active_workspaces(&mut outcome); + let rehomed = self.rehome_bound_windows(&mut outcome); + if normalized || rehomed { + self.apply_event_outcome(outcome); + } + } + } + + /// Switch displays showing a workspace bound to another connected display + /// back to their last own workspace, or the one they start on. When the owner + /// shows nothing, it takes the workspace over so the user keeps seeing it. + fn normalize_bound_active_workspaces(&mut self, outcome: &mut EventOutcome) -> bool { + let mut changed = false; + for screen in self.space_state.screens.clone() { + let Some(space) = screen.space.filter(|space| self.is_space_active(*space)) else { + continue; + }; + let workspaces = self.layout_manager.layout_engine.workspaces(); + let Some(index) = workspaces + .active_workspace(space) + .and_then(|active| self.workspace_ordinal(space, active)) + else { + continue; + }; + let Some(owner) = + self.bound_space_for_workspace_index(index).filter(|owner| *owner != space) + else { + continue; + }; + let target = workspaces + .last_workspace(space) + .and_then(|last| self.workspace_ordinal(space, last)) + .unwrap_or_else(|| workspaces.starting_workspace(space)); + if target == index { + continue; + } + let owner_workspaces = self.layout_manager.layout_engine.workspaces_mut(); + let owner_copy = owner_workspaces.workspace_id_at(owner, Some(index)); + let owner_active = owner_workspaces.active_workspace(owner); + let owner_idle = owner_active.is_none_or(|id| { + owner_workspaces.workspace_windows(&self.state.windows, owner, id).is_empty() + }); + if owner_copy.is_some() && owner_active != owner_copy && owner_idle { + match self + .dispatch_layout_command_on(LayoutCommand::SwitchToWorkspace(index), owner) + { + Ok(switched) => outcome.absorb(switched), + Err(error) => warn!(%error, "failed to show a bound workspace on its display"), + } + } + match self.dispatch_layout_command_on(LayoutCommand::SwitchToWorkspace(target), space) { + Ok(switched) => { + outcome.absorb(switched); + changed = true; + } + Err(error) => warn!(%error, "failed to leave a workspace bound to another display"), + } + } + changed + } + + /// Move windows sitting in a bound workspace on another display to its owner. + fn rehome_bound_windows(&mut self, outcome: &mut EventOutcome) -> bool { + let workspaces = self.layout_manager.layout_engine.workspaces(); + let moves: Vec<_> = self + .state + .windows + .iter_workspace_assignments() + .filter(|(window, assignment)| { + self.is_space_active(assignment.space) + // Visible where it is: a display only keeps showing a workspace + // bound elsewhere when it has none of its own. + && workspaces.active_workspace(assignment.space) != Some(assignment.workspace_id) + && self + .state + .windows + .window(*window) + .is_some_and(|state| state.is_admitted() && state.info.is_standard) + }) + .filter_map(|(window, assignment)| { + let index = self.workspace_ordinal(assignment.space, assignment.workspace_id)?; + let owner = self + .bound_space_for_workspace_index(index) + .filter(|owner| *owner != assignment.space)?; + Some((window, assignment.space, owner, index)) + }) + .collect(); + let mut changed = false; + for (window, source, owner, index) in moves { + match self.move_window_to_bound_workspace(window, source, owner, index, false) { + Ok(moved) => { + changed |= moved.arrange.passes > 0; + outcome.absorb(moved); + } + Err(error) => { + warn!(?window, %error, "failed to move a window to its workspace's display") + } + } + } + changed + } +} diff --git a/src/actor/wm_controller.rs b/src/actor/wm_controller.rs index 210f2a3a4..a780fd270 100644 --- a/src/actor/wm_controller.rs +++ b/src/actor/wm_controller.rs @@ -186,6 +186,8 @@ impl AppLifecycle { Some((handle, rx)) } + fn is_tracked(&self, pid: pid_t) -> bool { self.0.contains_key(&pid) } + fn terminate(&mut self, pid: pid_t) { if let Some((handle, phase)) = self.0.get_mut(&pid) { *phase = AppPhase::Stopping(None); @@ -319,6 +321,7 @@ impl WmController { AppGloballyActivated(pid) => { _ = self.input_tx.send(input::Request::EnforceHidden); self.events_tx.send(Event::ApplicationGloballyActivated(pid)); + self.retry_unattached_app(pid); } AppGloballyDeactivated(pid) => { self.events_tx.send(Event::ApplicationGloballyDeactivated(pid)); @@ -455,6 +458,21 @@ impl WmController { } } + /// Give an app without an app actor another attach attempt when the user + /// activates it. Attaching fails when an app refuses accessibility requests at + /// that moment, and otherwise nothing retries until the app relaunches or opens + /// a new window, leaving its windows unmanaged. + fn retry_unattached_app(&mut self, pid: pid_t) { + if self.apps.is_tracked(pid) { + return; + } + let Some(running_app) = NSRunningApplication::with_process_id(pid) else { + return; + }; + debug!(?pid, "Retrying attach to an activated app without an app actor"); + self.new_app(pid, AppInfo::from(&*running_app), AppDiscoverySource::Process); + } + fn new_app(&mut self, pid: pid_t, info: AppInfo, source: AppDiscoverySource) { let Some(running_app) = NSRunningApplication::with_process_id(pid) else { debug!(?pid, "Failed to resolve NSRunningApplication for new app"); diff --git a/src/common/config/tests.rs b/src/common/config/tests.rs index 297b15d4f..acaaed37a 100644 --- a/src/common/config/tests.rs +++ b/src/common/config/tests.rs @@ -76,6 +76,69 @@ fn virtual_workspace_prevent_wrapping_defaults_to_false_and_accepts_suggested_al assert!(settings.prevent_wrapping); } +#[test] +fn workspace_rules_bind_workspaces_to_displays() { + let settings: VirtualWorkspaceSettings = toml::from_str( + r#" + default_workspace_count = 4 + workspace_names = ["web", "code"] + workspace_rules = [ + { workspace = "web", display = "37D8832A-2D66-02CA-B9F7-8F30A301B230" }, + { workspace = 2, display = 1 }, + { workspace = 2, layout = "bsp" }, + { workspace = "Workspace 4", display = 0 }, + { workspace = 3, display = 1 }, + ] + "#, + ) + .unwrap(); + + assert!(settings.has_display_bindings()); + assert!(settings.validate().is_empty()); + let binding = |index| settings.display_binding_for_workspace(index).cloned(); + assert_eq!( + binding(0), + Some(DisplaySelector::Uuid( + "37D8832A-2D66-02CA-B9F7-8F30A301B230".into() + )) + ); + assert_eq!(binding(1), None); + assert_eq!( + binding(2), + Some(DisplaySelector::Index(1)), + "a later rule without a display keeps the binding" + ); + assert_eq!( + binding(3), + Some(DisplaySelector::Index(1)), + "unnamed workspaces match their default name, and the last rule wins" + ); +} + +#[test] +fn workspace_rules_report_rules_that_do_nothing_or_name_no_display() { + let settings: VirtualWorkspaceSettings = toml::from_str( + r#"workspace_rules = [ + { workspace = 0 }, + { workspace = 1, display = " " }, + { workspace = 2, display = "left" }, + ]"#, + ) + .unwrap(); + let issues = settings.validate(); + assert!( + issues + .iter() + .any(|issue| issue == "Workspace rule 0 sets neither layout nor display") + ); + assert!(issues.iter().any(|issue| issue == "Workspace rule 1 has an empty display UUID")); + assert!( + issues + .iter() + .any(|issue| issue == "Workspace rule 2 must name its display by UUID or index") + ); +} + #[test] fn app_rules_parse_placement_size_and_focus() { let settings: VirtualWorkspaceSettings = toml::from_str( @@ -462,7 +525,8 @@ fn typed_collections_keep_source_aliases_and_inline_commands() { }); source.virtual_workspaces.workspace_rules.push(WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(1), - layout: LayoutMode::Scrolling, + layout: Some(LayoutMode::Scrolling), + display: None, }); source.keys.insert("comb1 + Q".into(), source.keys["Alt + H"].clone()); source.binding_modes.insert( @@ -549,11 +613,13 @@ fn workspace_resize_cleans_removed_assignments_and_preserves_valid_rules() { settings.workspace_rules = vec![ WorkspaceLayoutRule { workspace: WorkspaceSelector::Name("three".into()), - layout: LayoutMode::Stack, + layout: Some(LayoutMode::Stack), + display: None, }, WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(0), - layout: LayoutMode::Bsp, + layout: Some(LayoutMode::Bsp), + display: None, }, ]; settings.app_rules = vec![AppWorkspaceRule { diff --git a/src/common/config/types.rs b/src/common/config/types.rs index fe9ebaba2..6aac698f6 100644 --- a/src/common/config/types.rs +++ b/src/common/config/types.rs @@ -2,7 +2,9 @@ use std::collections::BTreeMap; use std::path::PathBuf; use regex::RegexBuilder; -pub use rift_protocol::{AnimationEasing, ConfigCommand, LayoutMode, WorkspaceSelector}; +pub use rift_protocol::{ + AnimationEasing, ConfigCommand, DisplaySelector, LayoutMode, WorkspaceSelector, +}; use serde::{Deserialize, Serialize}; use super::{ConfigEnum, ConfigSchema}; @@ -92,11 +94,51 @@ pub struct VirtualWorkspaceSettings { pub workspace_rules: Vec, } +/// Display a newly created window opens on. +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy, Default, ConfigEnum)] +#[serde(rename_all = "snake_case")] +pub enum NewWindowDisplay { + /// Wherever macOS and the app put it. + #[default] + #[setting(label = "Where macOS puts it")] + Default, + /// The display that holds keyboard focus when the window appears. + #[setting(label = "Focused display")] + Focused, + /// The display under the mouse cursor when the window appears. + #[setting(label = "Display under the pointer")] + Cursor, +} + +/// Settings for one workspace. Where several rules match a workspace, the last +/// one giving a setting wins. #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(deny_unknown_fields)] pub struct WorkspaceLayoutRule { pub workspace: WorkspaceSelector, - pub layout: LayoutMode, + #[serde(default)] + pub layout: Option, + /// Display this workspace lives on, by UUID (stable across reconnects) or by + /// index in physical order. While that display is connected the workspace is + /// only shown there. + #[serde(default)] + pub display: Option, +} + +impl WorkspaceLayoutRule { + /// Whether the rule targets workspace `index`, named `name`. + pub fn matches(&self, index: usize, name: &str) -> bool { + match &self.workspace { + WorkspaceSelector::Index(target) => *target == index, + WorkspaceSelector::Name(target) => target == name, + } + } +} + +/// Name of workspace `index` as rift creates it: its entry in `names`, or +/// "Workspace N". +pub fn default_workspace_name(names: &[String], index: usize) -> String { + names.get(index).cloned().unwrap_or_else(|| format!("Workspace {}", index + 1)) } // Allow specifying a workspace by numeric index or by name in the config. @@ -191,6 +233,23 @@ impl Default for VirtualWorkspaceSettings { } impl VirtualWorkspaceSettings { + /// The display `workspace_rules` bind workspace `index` to, if any. Rules + /// match by index or by the workspace's name; the last one naming a display + /// wins. + pub fn display_binding_for_workspace(&self, index: usize) -> Option<&DisplaySelector> { + let name = default_workspace_name(&self.workspace_names, index); + self.workspace_rules + .iter() + .rev() + .filter(|rule| rule.matches(index, &name)) + .find_map(|rule| rule.display.as_ref()) + } + + /// Whether any workspace rule binds a workspace to a display. + pub fn has_display_bindings(&self) -> bool { + self.workspace_rules.iter().any(|rule| rule.display.is_some()) + } + pub fn resize(&mut self, count: usize) { self.default_workspace_count = count; // Invalid counts are left for the shared validator, without large allocations. @@ -236,6 +295,25 @@ impl VirtualWorkspaceSettings { )); } + for (index, rule) in self.workspace_rules.iter().enumerate() { + if rule.layout.is_none() && rule.display.is_none() { + issues.push(format!( + "Workspace rule {} sets neither layout nor display", + index + )); + } + match &rule.display { + Some(DisplaySelector::Uuid(uuid)) if uuid.trim().is_empty() => { + issues.push(format!("Workspace rule {} has an empty display UUID", index)); + } + Some(DisplaySelector::Direction(_)) => issues.push(format!( + "Workspace rule {} must name its display by UUID or index", + index + )), + _ => {} + } + } + // Validate rules and check duplicates in a single pass let mut seen_app_ids = crate::common::collections::HashSet::default(); let mut seen_app_names = crate::common::collections::HashSet::default(); @@ -476,6 +554,15 @@ pub struct Settings { aliases = "floating startup automatic tiling" )] pub default_disable: bool, + #[serde(default)] + /// Which display a newly created window opens on. + #[setting( + label = "Open new windows on", + group = "general", + choices, + aliases = "display monitor screen cursor pointer focused" + )] + pub new_window_display: NewWindowDisplay, #[serde(default = "yes")] /// Move the pointer into the window when focus changes. #[setting( diff --git a/src/layout_engine/engine.rs b/src/layout_engine/engine.rs index 3871d7f12..17c984b91 100644 --- a/src/layout_engine/engine.rs +++ b/src/layout_engine/engine.rs @@ -3121,20 +3121,55 @@ impl LayoutEngine { }; } + self.workspaces.ensure_space_initialized(target_space); + let Some(target_workspace_id) = self.workspaces.active_workspace(target_space) else { + return EventResponse::default(); + }; + self.move_window_to_workspace_on_space( + window_store, + source_space, + target_space, + target_screen_size, + window_id, + target_workspace_id, + false, + ) + } + + /// Moves one window into a specific workspace on another native space. + /// + /// Unlike [`Self::move_window_to_space`], the destination workspace is explicit + /// and need not be active on the target space. When it is active, or with + /// `follow`, the window receives focus there; otherwise focus stays on the + /// source display, mirroring `MoveWindowToWorkspace { follow: false }`. + pub fn move_window_to_workspace_on_space( + &mut self, + window_store: &mut WindowStore, + source_space: SpaceId, + target_space: SpaceId, + target_screen_size: CGSize, + window_id: WindowId, + target_workspace_id: VirtualWorkspaceId, + follow: bool, + ) -> EventResponse { + if source_space == target_space { + return EventResponse::default(); + } + self.workspaces.ensure_space_initialized(source_space); self.workspaces.ensure_space_initialized(target_space); let source_workspace = window_store .workspace_for_window(source_space, window_id) .or_else(|| self.workspaces.active_workspace(source_space)); - let Some(source_workspace_id) = source_workspace else { return EventResponse::default(); }; - - let Some(target_workspace_id) = self.workspaces.active_workspace(target_space) else { + if self.workspaces.workspaces.get(target_workspace_id).map(|ws| ws.space) + != Some(target_space) + { return EventResponse::default(); - }; + } self.ensure_workspace_layouts(target_space, target_screen_size); if !self.relocate_window_to_workspace( @@ -3147,23 +3182,49 @@ impl LayoutEngine { return EventResponse::default(); } - if self.focused_window == Some(window_id) { - self.focused_window = None; - } - - if self.workspaces.active_workspace(source_space) == Some(source_workspace_id) { + let source_was_active = + self.workspaces.active_workspace(source_space) == Some(source_workspace_id); + if source_was_active { self.workspaces.set_last_focused_window(source_space, source_workspace_id, None); } self.workspaces .set_last_focused_window(target_space, target_workspace_id, Some(window_id)); - self.focused_window = Some(window_id); self.broadcast_windows_changed(window_store, source_space); self.broadcast_windows_changed(window_store, target_space); + if follow { + return self.activate_workspace( + window_store, + target_space, + target_workspace_id, + Some(window_id), + ); + } + if self.workspaces.active_workspace(target_space) == Some(target_workspace_id) { + self.focused_window = Some(window_id); + return EventResponse { + changed: true, + raise_windows: vec![window_id], + focus_window: Some(window_id), + boundary_hit: None, + }; + } + + if self.focused_window == Some(window_id) { + self.focused_window = None; + } + let focus_window = source_was_active + .then(|| { + self.workspaces + .windows_in_active_workspace(window_store, source_space) + .into_iter() + .next() + }) + .flatten(); EventResponse { changed: true, - raise_windows: vec![window_id], - focus_window: Some(window_id), + raise_windows: vec![], + focus_window, boundary_hit: None, } } @@ -3344,6 +3405,13 @@ impl LayoutEngine { self.workspaces.workspace_info(space, workspace_id).map(|ws| ws.name.clone()) } + /// Whether an app rule names the workspace for a window with this context. + pub fn app_rule_names_workspace(&self, context: WindowRuleContext<'_>) -> bool { + self.app_rules + .evaluate(context) + .is_some_and(|decision| decision.workspace.is_some()) + } + pub fn is_window_floating(&self, window_id: WindowId) -> bool { self.floating.is_floating(window_id) || self.workspaces.workspaces.values().any(|ws| { @@ -4092,7 +4160,8 @@ mod tests { let mut settings = VirtualWorkspaceSettings::default(); settings.workspace_rules = vec![WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(0), - layout: LayoutMode::Scrolling, + layout: Some(LayoutMode::Scrolling), + display: None, }]; settings.app_rules = vec![AppWorkspaceRule { app_id: Some("com.example.Editor".into()), @@ -4469,7 +4538,8 @@ mod tests { let mut settings = VirtualWorkspaceSettings::default(); settings.workspace_rules = vec![WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(0), - layout: LayoutMode::Scrolling, + layout: Some(LayoutMode::Scrolling), + display: None, }]; let mut engine = LayoutEngine::new(&settings, &LayoutSettings::default(), None); let mut window_store = WindowStore::default(); @@ -4541,7 +4611,8 @@ mod tests { let mut settings = VirtualWorkspaceSettings::default(); settings.workspace_rules = vec![WorkspaceLayoutRule { workspace: WorkspaceSelector::Name(workspace_name), - layout: LayoutMode::Scrolling, + layout: Some(LayoutMode::Scrolling), + display: None, }]; engine.update_virtual_workspace_settings(&window_store, &settings); @@ -4564,7 +4635,8 @@ mod tests { if let Some(layout) = rule_layout { settings.workspace_rules.push(WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(0), - layout, + layout: Some(layout), + display: None, }); } let mut engine = LayoutEngine::new(&settings, &layouts, None); diff --git a/src/model/hidden_window_placement.rs b/src/model/hidden_window_placement.rs index 499f2a8f9..0bb4fd491 100644 --- a/src/model/hidden_window_placement.rs +++ b/src/model/hidden_window_placement.rs @@ -1,19 +1,54 @@ -use objc2_core_foundation::{CGPoint, CGRect}; +use objc2_core_foundation::{CGPoint, CGRect, CGSize}; +/// Where an inactive-workspace window is parked relative to its display. +/// +/// The bottom corners leave a one-pixel sliver of the window's top edge on +/// screen and push the rest below the display. That only works when nothing +/// sits below the display: with a vertically stacked arrangement the parked +/// window would land on the lower display, and both macOS and Rift would then +/// treat it as belonging there. The edge placements keep the window inside the +/// display's vertical span and push it sideways instead. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum HideCorner { BottomLeft, #[default] BottomRight, + /// Just past the right edge, top-aligned with the display. + RightEdge, + /// Just past the left edge, top-aligned with the display. + LeftEdge, } impl HideCorner { + const ALL: [HideCorner; 4] = [ + HideCorner::BottomRight, + HideCorner::BottomLeft, + HideCorner::RightEdge, + HideCorner::LeftEdge, + ]; + pub fn opposite(self) -> Self { match self { Self::BottomLeft => Self::BottomRight, Self::BottomRight => Self::BottomLeft, + Self::RightEdge => Self::LeftEdge, + Self::LeftEdge => Self::RightEdge, } } + + /// Candidate order: the preferred corner, its mirror, then the remaining + /// placements. Earlier candidates win ties. + fn candidates(preferred: HideCorner) -> [HideCorner; 4] { + let mut ordered = [preferred, preferred.opposite(), preferred, preferred]; + let mut next = 2; + for corner in Self::ALL { + if corner != preferred && corner != preferred.opposite() { + ordered[next] = corner; + next += 1; + } + } + ordered + } } /// Pure geometry used to place inactive-workspace windows just offscreen. @@ -24,11 +59,35 @@ impl HiddenWindowPlacement { const VISIBLE_THRESHOLD_PX: f64 = 3.0; fn rect_for_corner(screen: CGRect, window: CGRect, corner: HideCorner) -> CGRect { - let x = match corner { - HideCorner::BottomLeft => screen.origin.x - window.size.width + Self::REVEAL_PX, - HideCorner::BottomRight => screen.max().x - Self::REVEAL_PX, - }; - CGRect::new(CGPoint::new(x, screen.max().y - Self::REVEAL_PX), window.size) + match corner { + HideCorner::BottomLeft => CGRect::new( + CGPoint::new( + screen.origin.x - window.size.width + Self::REVEAL_PX, + screen.max().y - Self::REVEAL_PX, + ), + window.size, + ), + HideCorner::BottomRight => CGRect::new( + CGPoint::new( + screen.max().x - Self::REVEAL_PX, + screen.max().y - Self::REVEAL_PX, + ), + window.size, + ), + // Edge placements must not poke out below the display, or they would + // reintroduce the overlap they exist to avoid; clamp the height. + HideCorner::RightEdge => CGRect::new( + CGPoint::new(screen.max().x - Self::REVEAL_PX, screen.origin.y), + CGSize::new(window.size.width, window.size.height.min(screen.size.height)), + ), + HideCorner::LeftEdge => CGRect::new( + CGPoint::new( + screen.origin.x - window.size.width + Self::REVEAL_PX, + screen.origin.y, + ), + CGSize::new(window.size.width, window.size.height.min(screen.size.height)), + ), + } } fn intersection_area(a: CGRect, b: CGRect) -> f64 { @@ -37,31 +96,43 @@ impl HiddenWindowPlacement { width * height } + /// Pick the parked frame that overlaps other displays the least, preferring + /// `preferred_corner` and then its mirror on ties. pub fn calculate( screen: CGRect, window: CGRect, preferred_corner: HideCorner, other_screens: &[CGRect], ) -> CGRect { - let preferred = Self::rect_for_corner(screen, window, preferred_corner); - let alternate = Self::rect_for_corner(screen, window, preferred_corner.opposite()); - let overlap = |candidate| { + let overlap = |candidate: CGRect| { other_screens .iter() + .filter(|other| **other != screen) .map(|other| Self::intersection_area(candidate, *other)) .sum::() }; - if overlap(alternate) < overlap(preferred) { - alternate - } else { - preferred + let mut best: Option<(CGRect, f64)> = None; + for corner in HideCorner::candidates(preferred_corner) { + let candidate = Self::rect_for_corner(screen, window, corner); + let area = overlap(candidate); + if best.is_none_or(|(_, best_area)| area < best_area) { + best = Some((candidate, area)); + } + if area == 0.0 { + break; + } } + best.map(|(rect, _)| rect).unwrap_or(window) } + /// Whether `window` is parked offscreen relative to `screen`: it matches one + /// of the parked placements, or at most a sliver of it is visible in either + /// dimension. pub fn is_hidden(screen: CGRect, window: CGRect, other_screens: &[CGRect]) -> bool { - [HideCorner::BottomLeft, HideCorner::BottomRight] + HideCorner::ALL .into_iter() - .any(|corner| Self::calculate(screen, window, corner, other_screens) == window) + .any(|corner| Self::rect_for_corner(screen, window, corner) == window) + || Self::calculate(screen, window, HideCorner::BottomRight, other_screens) == window || { let visible_width = (window.max().x.min(screen.max().x) - window.origin.x.max(screen.origin.x)) @@ -70,7 +141,7 @@ impl HiddenWindowPlacement { - window.origin.y.max(screen.origin.y)) .max(0.0); visible_width <= Self::VISIBLE_THRESHOLD_PX - && visible_height <= Self::VISIBLE_THRESHOLD_PX + || visible_height <= Self::VISIBLE_THRESHOLD_PX } } } @@ -107,4 +178,63 @@ mod tests { ); assert_eq!(hidden.origin.x, -199.0); } + + #[test] + fn parks_beside_the_display_when_another_display_sits_below() { + // External display stacked directly above a laptop panel, as macOS + // reports it: the upper display has negative y and its bottom edge + // touches the lower display's top. + let upper = rect(81.0, -1049.0, 1920.0, 1049.0); + let lower = rect(0.0, 40.0, 2056.0, 1289.0); + let window = rect(91.0, -1049.0, 1900.0, 1050.0); + + let hidden = + HiddenWindowPlacement::calculate(upper, window, HideCorner::BottomRight, &[lower]); + + assert_eq!(hidden.origin, CGPoint::new(upper.max().x - 1.0, upper.origin.y)); + assert_eq!( + hidden.size, + CGSize::new(1900.0, 1049.0), + "edge placement clamps the height to the display" + ); + assert_eq!(HiddenWindowPlacement::intersection_area(hidden, lower), 0.0); + assert!(HiddenWindowPlacement::is_hidden(upper, hidden, &[lower])); + + // Bottom placements stay the default when nothing is below. + let alone = HiddenWindowPlacement::calculate(upper, window, HideCorner::BottomRight, &[]); + assert_eq!(alone.origin.y, upper.max().y - 1.0); + } + + #[test] + fn tall_windows_do_not_spill_onto_the_display_below() { + let upper = rect(81.0, -1049.0, 1920.0, 1049.0); + let lower = rect(0.0, 40.0, 2056.0, 1289.0); + let tall = rect(16.0, 100.0, 1007.0, 1219.0); + + let hidden = + HiddenWindowPlacement::calculate(upper, tall, HideCorner::BottomRight, &[lower]); + + assert_eq!(HiddenWindowPlacement::intersection_area(hidden, lower), 0.0); + assert!(hidden.max().y <= upper.max().y); + } + + #[test] + fn sliver_visible_in_either_dimension_counts_as_hidden() { + let screen = rect(0.0, 0.0, 1000.0, 800.0); + assert!(HiddenWindowPlacement::is_hidden( + screen, + rect(998.0, 100.0, 300.0, 300.0), + &[] + )); + assert!(HiddenWindowPlacement::is_hidden( + screen, + rect(100.0, 798.0, 300.0, 300.0), + &[] + )); + assert!(!HiddenWindowPlacement::is_hidden( + screen, + rect(900.0, 700.0, 300.0, 300.0), + &[] + )); + } } diff --git a/src/model/virtual_workspace.rs b/src/model/virtual_workspace.rs index 767a102d9..974818eb2 100644 --- a/src/model/virtual_workspace.rs +++ b/src/model/virtual_workspace.rs @@ -9,6 +9,7 @@ use crate::common::collections::{HashMap, HashSet}; use crate::common::config::AppWorkspaceRule; use crate::common::config::{ LayoutMode, LayoutSettings, MAX_WORKSPACES, VirtualWorkspaceSettings, WorkspaceSelector, + default_workspace_name, }; use crate::common::log::trace_misc; use crate::layout_engine::systems::LayoutSystemKind; @@ -175,6 +176,13 @@ pub struct WorkspaceStore { default_workspace_names: Vec, #[serde(skip)] default_workspace: usize, + /// Workspace display bindings, per native space on screen: the workspace the + /// space starts on when first initialized, and the workspaces (by position) + /// bound to another connected display, which cycling and back-and-forth skip. + #[serde(skip)] + preferred_default_workspace: HashMap, + #[serde(skip)] + foreign_workspaces: HashMap>, #[serde(skip)] pub workspace_auto_back_and_forth: bool, #[serde(skip)] @@ -321,6 +329,8 @@ impl WorkspaceStore { default_workspace_count: config.default_workspace_count, default_workspace_names: config.workspace_names.clone(), default_workspace, + preferred_default_workspace: HashMap::default(), + foreign_workspaces: HashMap::default(), workspace_auto_back_and_forth: config.workspace_auto_back_and_forth, prevent_wrapping: config.prevent_wrapping, workspace_rules: config.workspace_rules.clone(), @@ -394,18 +404,19 @@ impl WorkspaceStore { let mut ids = Vec::new(); let count = self.default_workspace_count.max(1).min(self.max_workspaces); for i in 0..count { - let name = self - .default_workspace_names - .get(i) - .cloned() - .unwrap_or_else(|| format!("Workspace {}", i + 1)); + let name = default_workspace_name(&self.default_workspace_names, i); let mode = self.resolve_layout_mode_for_workspace(i, &name); let ws = VirtualWorkspace::new(name, space, mode, &self.layout_settings); let id = self.workspaces.insert(ws); ids.push(id); } - let default_id = ids.get(self.default_workspace.min(ids.len() - 1)).copied(); + let default_index = self + .preferred_default_workspace + .get(&space) + .copied() + .unwrap_or(self.default_workspace); + let default_id = ids.get(default_index.min(ids.len() - 1)).copied(); ids.sort_unstable(); self.workspaces_by_space.insert(space, ids); @@ -414,13 +425,46 @@ impl WorkspaceStore { } } + /// Replace the workspace display bindings with `spaces`: for each native + /// space on screen, the workspace it starts on and the workspaces bound to + /// another connected display. + pub(crate) fn set_display_bindings( + &mut self, + spaces: impl IntoIterator, HashSet)>, + ) { + self.preferred_default_workspace.clear(); + self.foreign_workspaces.clear(); + for (space, default, foreign) in spaces { + if let Some(default) = default { + self.preferred_default_workspace.insert(space, default); + } + if !foreign.is_empty() { + self.foreign_workspaces.insert(space, foreign); + } + } + } + + /// The workspace `space` starts on, by position. + pub(crate) fn starting_workspace(&self, space: SpaceId) -> usize { + self.preferred_default_workspace + .get(&space) + .copied() + .unwrap_or(self.default_workspace) + } + + fn is_foreign_workspace(&self, space: SpaceId, index: usize) -> bool { + self.foreign_workspaces + .get(&space) + .is_some_and(|foreign| foreign.contains(&index)) + } + fn resolve_layout_mode_for_workspace(&self, index: usize, name: &str) -> LayoutMode { // Check workspace_rules (last matching rule wins, like app_rules) for rule in self.workspace_rules.iter().rev() { - match &rule.workspace { - WorkspaceSelector::Index(idx) if *idx == index => return rule.layout, - WorkspaceSelector::Name(n) if n == name => return rule.layout, - _ => continue, + if rule.matches(index, name) + && let Some(layout) = rule.layout + { + return layout; } } // Fall back to global default @@ -598,8 +642,11 @@ impl WorkspaceStore { Ok(workspace_id) } + /// The back-and-forth target of `space`, unless it lives on another display. pub fn last_workspace(&self, space: SpaceId) -> Option { - self.active_workspace_per_space.get(&space)?.0 + let last = self.active_workspace_per_space.get(&space)?.0?; + let index = self.workspace_ids(space).iter().position(|id| *id == last); + (!index.is_some_and(|index| self.is_foreign_workspace(space, index))).then_some(last) } pub fn active_workspace(&self, space: SpaceId) -> Option { @@ -664,6 +711,9 @@ impl WorkspaceStore { _ => return None, }; + if self.is_foreign_workspace(space, index) { + continue; + } let id = ids[index]; if !require_non_empty || !self.workspace_windows(window_store, space, id).is_empty() { return Some(id); diff --git a/src/sys/window_server.rs b/src/sys/window_server.rs index c7b594780..c4aafcf2f 100644 --- a/src/sys/window_server.rs +++ b/src/sys/window_server.rs @@ -45,6 +45,7 @@ thread_local! { static TEST_WINDOW_ORDER_QUERY_COUNT: std::cell::Cell = const { std::cell::Cell::new(0) }; static TEST_WINDOW_SPACES_OVERRIDE: RefCell>> = RefCell::new(HashMap::default()); static TEST_WINDOW_ORDERED_IN_OVERRIDE: RefCell> = RefCell::new(HashMap::default()); + static TEST_CURSOR_LOCATION_OVERRIDE: std::cell::Cell> = const { std::cell::Cell::new(None) }; } pub const WINDOWSERVER_QUIET_US: u64 = 350_000; @@ -702,6 +703,10 @@ pub fn is_point_occluded_by_external_window(mut point: CGPoint) -> bool { } pub fn current_cursor_location() -> Result { + #[cfg(test)] + if let Some(point) = TEST_CURSOR_LOCATION_OVERRIDE.with(std::cell::Cell::get) { + return Ok(point); + } let mut point = CGPoint::new(0.0, 0.0); cg_ok(unsafe { SLSGetCurrentCursorLocation(*G_CONNECTION, &mut point) })?; Ok(point) @@ -875,6 +880,11 @@ pub fn key_focused_window(space: SpaceId) -> Option { /// The space on the display currently holding WindowServer focus. pub fn active_space() -> SpaceId { SpaceId::new(unsafe { CGSGetActiveSpace(*G_CONNECTION) }) } +#[cfg(test)] +pub fn set_cursor_location_override(point: Option) { + TEST_CURSOR_LOCATION_OVERRIDE.with(|location| location.set(point)); +} + #[cfg(test)] pub fn set_space_window_list_for_connection_override(ids: Option>) { TEST_SPACE_WINDOW_LIST_OVERRIDE.with(|override_ids| *override_ids.borrow_mut() = ids); diff --git a/src/ui/settings/editors.rs b/src/ui/settings/editors.rs index d06b131b0..4c20fb254 100644 --- a/src/ui/settings/editors.rs +++ b/src/ui/settings/editors.rs @@ -293,13 +293,19 @@ fn workspace_detail(ui: Ui, model: &Rc, i: usize) -> Page { |s| s.settings.layout.mode, move |s, mode| { let name = s.virtual_workspaces.workspace_names.get(i).cloned(); - s.virtual_workspaces - .workspace_rules - .retain(|r| !selector_matches(&r.workspace, i, name.as_deref())); + // Display bindings stay; only the layout moves to the new rule. + s.virtual_workspaces.workspace_rules.retain_mut(|r| { + if !selector_matches(&r.workspace, i, name.as_deref()) { + return true; + } + r.layout = None; + r.display.is_some() + }); if let Some(layout) = mode { s.virtual_workspaces.workspace_rules.push(WorkspaceLayoutRule { workspace: WorkspaceSelector::Index(i), - layout, + layout: Some(layout), + display: None, }); } }, @@ -317,14 +323,15 @@ fn workspace_layout(s: &ConfigSource, i: usize) -> Option { s.virtual_workspaces .workspace_rules .iter() - .find(|r| { + .rev() + .filter(|r| { selector_matches( &r.workspace, i, s.virtual_workspaces.workspace_names.get(i).map(String::as_str), ) }) - .map(|r| r.layout) + .find_map(|r| r.layout) } pub(super) fn workspace_name(s: &ConfigSource, i: usize) -> String { s.virtual_workspaces