From b1015fa7b627221f9d04219f3da6612215de5eef Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Tue, 29 Sep 2026 16:02:15 +1000 Subject: [PATCH 01/10] fix: retry attaching to apps that refused accessibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An app actor gives up when registering for an app's accessibility notifications fails, and nothing tries again until the app relaunches or opens a new window, so its windows stay unmanaged. Registration had no grace period outside JetBrains IDEs and Emacs. - give every app a two second grace period for the first registration - retry attaching whenever the user activates an app that has no app actor 🤖 Generated with Claude Code --- src/actor/app.rs | 5 ++++- src/actor/wm_controller.rs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) 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/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"); From 1214e5003cd00dd0ca82a5f1c2ee18612b629851 Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Wed, 30 Sep 2026 11:06:34 +1000 Subject: [PATCH 02/10] fix: park hidden windows where they overlap no other display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rift hides the windows of inactive workspaces by parking them past the bottom-left or bottom-right corner of their display, leaving a sliver on screen. With a display arranged above another, a window parked past the upper display's bottom edge sits on the lower display. Rift then adopts it there, and it turns up in the wrong display's workspace. - add right-edge and left-edge placements, top-aligned with the display and clamped to its height - pick the placement that overlaps other displays least - treat a sliver in either dimension as hidden 🤖 Generated with Claude Code --- src/model/hidden_window_placement.rs | 162 ++++++++++++++++++++++++--- 1 file changed, 146 insertions(+), 16 deletions(-) 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), + &[] + )); + } } From 673e1476867c790b02fa0e773fdc93c59bbf3ffe Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Tue, 29 Sep 2026 16:02:16 +1000 Subject: [PATCH 03/10] fix: act on the display move-node just carried the focused window to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After move-node carried the focused window onto another display, the next command still acted on the display the window had left: macOS moves its active display along with the key window only later. Make the window's new display the command context, as focus_display would, so a second move-node or a focus command acts where the window now is. 🤖 Generated with Claude Code --- src/actor/reactor.rs | 32 ++++++++++++++++++++++++++++++-- src/actor/reactor/testing.rs | 13 +++++++++++++ src/actor/reactor/tests.rs | 31 +++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index 8d2181f05..ccbae3d18 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -2323,8 +2323,9 @@ impl Reactor { 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(_)); 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 +2336,11 @@ impl Reactor { visible_space_frames, post_arrange_mouse_warp, }, - ); + )?; + if is_move_node { + self.follow_focused_window_to_its_display(command_space); + } + return Ok(outcome); } Event::Command(Command::Reactor(ReactorCommand::MoveWindowToDisplay { selector, @@ -5292,6 +5297,29 @@ impl Reactor { CGRect::new(origin, frame.size) } + /// 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); + } + fn screens_in_physical_order(&self) -> Vec<&ScreenInfo> { let mut screens: Vec<&ScreenInfo> = self.space_state.screens.iter().collect(); screens.sort_by(|a, b| { diff --git a/src/actor/reactor/testing.rs b/src/actor/reactor/testing.rs index 10b29ca3b..5d4d3d82a 100644 --- a/src/actor/reactor/testing.rs +++ b/src/actor/reactor/testing.rs @@ -685,3 +685,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..3159e855e 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(); From 0cf50094a1fcd1ef04d22b74d75c4c3812364a6b Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Tue, 29 Sep 2026 17:26:29 +1000 Subject: [PATCH 04/10] fix: keep windows rift just moved across displays from snapping back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After rift moves a window to another display, macOS and the app take a moment to catch up. Until then the window server still reports the old display, and the app can send frame reports from before the move. Rift treated a live report of the old display as the user moving the window back, so it followed the window there, then followed its own move again once the window landed, and the window flipped between displays. move_window_to_display, move_workspace_to_display, overview drops and move-node across displays all go through this path. Record every cross-display move rift starts and, for two seconds, resolve the window to its new display instead of following reports of the old one. After that, a real move by the user is followed as before. 🤖 Generated with Claude Code --- src/actor/reactor.rs | 77 ++++++++++++++++++++++++++++++++++-- src/actor/reactor/testing.rs | 8 ++++ src/actor/reactor/tests.rs | 53 +++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 4 deletions(-) diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index ccbae3d18..d88fe4625 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -434,6 +434,9 @@ 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, #[cfg(test)] event_outcome_phase_trace: Vec<&'static str>, #[cfg(test)] @@ -445,6 +448,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 +579,7 @@ impl Reactor { animation_tx: None, viewport_gesture: None, presentations: HashMap::default(), + in_flight_display_moves: HashMap::default(), #[cfg(test)] event_outcome_phase_trace: Vec::new(), #[cfg(test)] @@ -1959,6 +1968,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 @@ -2324,6 +2334,15 @@ impl Reactor { 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); let outcome = command_workflow::handle_command_layout( &mut self.state, @@ -2338,6 +2357,13 @@ impl Reactor { }, )?; 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); @@ -2424,7 +2450,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 { @@ -2435,7 +2461,9 @@ impl Reactor { target_screen: target_screen.frame, target_frame, }, - ); + )?; + self.note_display_move_in_flight(window, target_space); + return Ok(outcome); } Event::Command(Command::Reactor(ReactorCommand::MoveWorkspaceToDisplay { selector, @@ -2506,7 +2534,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, @@ -2516,7 +2546,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); } _ => (), } @@ -3695,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)?; @@ -3867,6 +3930,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)) { diff --git a/src/actor/reactor/testing.rs b/src/actor/reactor/testing.rs index 5d4d3d82a..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 diff --git a/src/actor/reactor/tests.rs b/src/actor/reactor/tests.rs index 3159e855e..ac1cdc36c 100644 --- a/src/actor/reactor/tests.rs +++ b/src/actor/reactor/tests.rs @@ -2264,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) = From 87760fb9623f18d81750347352d38b6ea5195aa1 Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Tue, 29 Sep 2026 16:52:05 +1000 Subject: [PATCH 05/10] feat: settings.new_window_display to open new windows on the focused or cursor display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A new window opens wherever macOS and the app put it. Launchers that activate an app before asking it for a window get the window next to that app's other windows, even while the user works on another display. Add settings.new_window_display: - "default": unchanged behaviour - "focused": move a new window to the display holding keyboard focus - "cursor": move a new window to the display under the mouse cursor A window whose app rule names a workspace stays where the rule puts it. The move is recorded as a display move in flight, so reports of the old display that arrive while macOS catches up do not pull the window back. 🤖 Generated with Claude Code --- rift.default.toml | 8 ++ src/actor/reactor.rs | 65 ++++++++++++++++ src/actor/reactor/tests.rs | 145 ++++++++++++++++++++++++++++++++++++ src/common/config/types.rs | 25 +++++++ src/layout_engine/engine.rs | 7 ++ src/sys/window_server.rs | 10 +++ 6 files changed, 260 insertions(+) diff --git a/rift.default.toml b/rift.default.toml index 1501ceadd..6dc7e738a 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. diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index d88fe4625..8dc2ed9d7 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -2619,6 +2619,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); } } } @@ -5366,6 +5367,70 @@ impl Reactor { CGRect::new(origin, frame.size) } + /// 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, + }, + ) { + 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 diff --git a/src/actor/reactor/tests.rs b/src/actor/reactor/tests.rs index ac1cdc36c..1eacc7d1b 100644 --- a/src/actor/reactor/tests.rs +++ b/src/actor/reactor/tests.rs @@ -3501,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); diff --git a/src/common/config/types.rs b/src/common/config/types.rs index fe9ebaba2..ad4b2f85e 100644 --- a/src/common/config/types.rs +++ b/src/common/config/types.rs @@ -92,6 +92,22 @@ 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, +} + #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)] #[serde(deny_unknown_fields)] pub struct WorkspaceLayoutRule { @@ -476,6 +492,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..b51410753 100644 --- a/src/layout_engine/engine.rs +++ b/src/layout_engine/engine.rs @@ -3344,6 +3344,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| { 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); From 486eeb17968fc25b392e9a514cf9d0787783467a Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Wed, 30 Sep 2026 11:33:58 +1000 Subject: [PATCH 06/10] refactor: move the focus_display command into a reactor method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workspace display bindings need to focus a display from inside other commands. Move the body of the focus_display handler into Reactor::focus_display_by_selector unchanged; the handler now calls it. No behaviour change. 🤖 Generated with Claude Code --- src/actor/reactor.rs | 85 ++++++++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index 8dc2ed9d7..d23c1e259 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -2290,44 +2290,7 @@ 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)) => { let post_arrange_mouse_warp = @@ -5454,6 +5417,52 @@ impl Reactor { 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() + }) + }); + 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> { let mut screens: Vec<&ScreenInfo> = self.space_state.screens.iter().collect(); screens.sort_by(|a, b| { From 475bb49d51458b69e03d0e6e844422d16b7f27e3 Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Wed, 30 Sep 2026 12:39:33 +1000 Subject: [PATCH 07/10] feat(config): bind workspaces to displays in workspace_rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let a workspace rule name the display its workspace lives on: workspace_rules = [{ workspace = "web", display = "37D8832A-…" }] `display` is a DisplaySelector, the type display commands already take: a display UUID (stable across reconnects) or an index in physical order. Directions make no sense for a binding and are reported by validation, as are empty UUIDs and rules that set neither layout nor display. `layout` becomes optional so a rule can set only a display; rules keep matching by index or name, the last one giving a setting winning. This only adds the setting; the following commits make rift honour it. 🤖 Generated with Claude Code --- rift.default.toml | 19 ++++++++- src/actor/config.rs | 3 +- src/common/config/tests.rs | 72 ++++++++++++++++++++++++++++++++-- src/common/config/types.rs | 66 ++++++++++++++++++++++++++++++- src/layout_engine/engine.rs | 12 ++++-- src/model/virtual_workspace.rs | 15 +++---- src/ui/settings/editors.rs | 19 ++++++--- 7 files changed, 179 insertions(+), 27 deletions(-) diff --git a/rift.default.toml b/rift.default.toml index 6dc7e738a..75faf751d 100644 --- a/rift.default.toml +++ b/rift.default.toml @@ -309,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/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/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 ad4b2f85e..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}; @@ -108,11 +110,35 @@ pub enum NewWindowDisplay { 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. @@ -207,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. @@ -252,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(); diff --git a/src/layout_engine/engine.rs b/src/layout_engine/engine.rs index b51410753..0dc7c28fc 100644 --- a/src/layout_engine/engine.rs +++ b/src/layout_engine/engine.rs @@ -4099,7 +4099,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()), @@ -4476,7 +4477,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(); @@ -4548,7 +4550,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); @@ -4571,7 +4574,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/virtual_workspace.rs b/src/model/virtual_workspace.rs index 767a102d9..be1e675e0 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; @@ -394,11 +395,7 @@ 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); @@ -417,10 +414,10 @@ impl WorkspaceStore { 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 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 From 288c4ec6ddef62e1806d80d0601e3214671b9d69 Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Wed, 30 Sep 2026 12:41:21 +1000 Subject: [PATCH 08/10] feat: start displays on their own workspaces and skip others' when cycling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With workspace display bindings, every display started on default_workspace, so a display could come up showing a workspace bound to another one, and windows found on it were filed there. Cycling and back-and-forth could land on such workspaces too. On every snapshot and config reload the reactor now tells the workspace store, for each display, which workspace it starts on (default_workspace if it may live there, else its first own workspace, else the first unbound one) and which workspaces are bound to another connected display. The store starts new spaces on the former, and step_workspace and last_workspace skip the latter, so next/prev (commands, swipes and scroll overscroll), relative window moves, back-and-forth and switch_to_last_workspace stay on a display's own workspaces with no further routing. 🤖 Generated with Claude Code --- src/actor/reactor.rs | 23 +++-- src/actor/reactor/tests.rs | 117 ++++++++++++++++++++++++ src/actor/reactor/workspace_bindings.rs | 99 ++++++++++++++++++++ src/model/virtual_workspace.rs | 49 +++++++++- 4 files changed, 274 insertions(+), 14 deletions(-) create mode 100644 src/actor/reactor/workspace_bindings.rs diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index d23c1e259..104811471 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; @@ -2128,13 +2129,17 @@ 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); + return Ok(outcome); } Event::Command(Command::Metrics(cmd)) => { return command_workflow::handle_command_metrics(cmd); @@ -3247,6 +3252,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(); @@ -5464,16 +5472,7 @@ impl Reactor { } 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 - } - }); - screens + workspace_bindings::physical_order(&self.space_state.screens) } fn store_current_floating_positions(&mut self, space: SpaceId) { diff --git a/src/actor/reactor/tests.rs b/src/actor/reactor/tests.rs index 1eacc7d1b..0e5e7a5b3 100644 --- a/src/actor/reactor/tests.rs +++ b/src/actor/reactor/tests.rs @@ -8271,3 +8271,120 @@ 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])); +} diff --git a/src/actor/reactor/workspace_bindings.rs b/src/actor/reactor/workspace_bindings.rs new file mode 100644 index 000000000..2a37940b8 --- /dev/null +++ b/src/actor/reactor/workspace_bindings.rs @@ -0,0 +1,99 @@ +//! 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. +//! +//! When the owning display is not connected the workspace behaves like an +//! unbound one on whatever display the user is on. + +use super::{DisplaySelector, Reactor, ScreenInfo}; +use crate::common::collections::HashSet; + +/// 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); + } +} diff --git a/src/model/virtual_workspace.rs b/src/model/virtual_workspace.rs index be1e675e0..82747b2ce 100644 --- a/src/model/virtual_workspace.rs +++ b/src/model/virtual_workspace.rs @@ -176,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)] @@ -322,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(), @@ -402,7 +411,12 @@ impl WorkspaceStore { 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); @@ -411,6 +425,31 @@ 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); + } + } + } + + 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() { @@ -595,8 +634,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 { @@ -661,6 +703,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); From 8a8cb0048e70817ac2c0905464b2ace91290d3ba Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Wed, 30 Sep 2026 12:47:35 +1000 Subject: [PATCH 09/10] feat: route workspace commands to the display a workspace is bound to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While a bound workspace's display is connected, keep the workspace there: - switch_to_workspace, or picking the workspace in the overview, from another display focuses the owning display and switches there; when the workspace is already showing there it just focuses the display - move_window_to_workspace moves the window into the owning display's copy of the workspace, with or without follow; a window named by id may live on any display - move_workspace_to_display refuses to move a bound workspace off its display Bound moves go through the existing move-window-to-display workflow, which now takes an optional target workspace and follow flag, and LayoutEngine::move_window_to_workspace_on_space, which move_window_to_space now delegates to instead of repeating the relocation bookkeeping. 🤖 Generated with Claude Code --- src/actor/reactor.rs | 17 ++ src/actor/reactor/events/command.rs | 31 +++- src/actor/reactor/tests.rs | 161 +++++++++++++++++++ src/actor/reactor/workspace_bindings.rs | 196 +++++++++++++++++++++++- src/layout_engine/engine.rs | 85 ++++++++-- 5 files changed, 469 insertions(+), 21 deletions(-) diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index 104811471..f8023a554 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -1903,6 +1903,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) { @@ -2298,6 +2301,9 @@ impl Reactor { 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(); @@ -2428,6 +2434,8 @@ 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); @@ -2474,6 +2482,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 @@ -5390,6 +5405,8 @@ impl Reactor { target_space, target_screen: target_screen.frame, target_frame, + target_workspace: None, + follow: false, }, ) { Ok(outcome) => { 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/tests.rs b/src/actor/reactor/tests.rs index 0e5e7a5b3..ad7263535 100644 --- a/src/actor/reactor/tests.rs +++ b/src/actor/reactor/tests.rs @@ -8388,3 +8388,164 @@ fn cycling_and_back_and_forth_skip_workspaces_bound_to_other_displays() { 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) + ); + } +} diff --git a/src/actor/reactor/workspace_bindings.rs b/src/actor/reactor/workspace_bindings.rs index 2a37940b8..1fd09d63d 100644 --- a/src/actor/reactor/workspace_bindings.rs +++ b/src/actor/reactor/workspace_bindings.rs @@ -8,12 +8,21 @@ //! - 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. //! //! When the owning display is not connected the workspace behaves like an //! unbound one on whatever display the user is on. -use super::{DisplaySelector, Reactor, ScreenInfo}; +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. /// @@ -96,4 +105,189 @@ impl Reactor { .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) + } } diff --git a/src/layout_engine/engine.rs b/src/layout_engine/engine.rs index 0dc7c28fc..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, } } From 4e27f541de826a84bdcaf472a6a980b40e18eaec Mon Sep 17 00:00:00 2001 From: Swoorup Joshi Date: Wed, 30 Sep 2026 12:51:53 +1000 Subject: [PATCH 10/10] feat: move windows home when a bound workspace's display comes back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A window can end up in a workspace bound to a display it is not on: an app rule placed it there, it was dropped there in the overview, or macOS parked it on the remaining display while the owner was unplugged. And after a display joins, leaves or moves, a display can be left showing a workspace whose owner is back. After each event that can cause this (window discovery and placement, a display change, an overview drop, a config reload) re-apply the bindings once the event's outcome has settled window membership: - a display showing a workspace bound elsewhere switches back to its last own workspace, or the one it starts on; when the owner shows nothing it takes the workspace over, so the user keeps seeing it - windows in a bound workspace on another display move to the owner's copy of that workspace, without taking focus - a window macOS moves back onto its workspace's display keeps its workspace number instead of joining the workspace that display shows The work waits while the native topology is invalidated (sleep, wake, lock, display churn), since moving windows on transient data fights macOS; the authoritative snapshot that ends the instability runs it. 🤖 Generated with Claude Code --- src/actor/reactor.rs | 27 ++- src/actor/reactor/tests.rs | 262 ++++++++++++++++++++++++ src/actor/reactor/workspace_bindings.rs | 144 +++++++++++++ src/model/virtual_workspace.rs | 8 + 4 files changed, 440 insertions(+), 1 deletion(-) diff --git a/src/actor/reactor.rs b/src/actor/reactor.rs index f8023a554..7d6dca183 100644 --- a/src/actor/reactor.rs +++ b/src/actor/reactor.rs @@ -438,6 +438,9 @@ pub struct Reactor { /// 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)] @@ -581,6 +584,7 @@ impl Reactor { 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)] @@ -1207,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() @@ -1959,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); @@ -2142,6 +2150,7 @@ impl Reactor { // 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)) => { @@ -3377,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) } @@ -3878,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); } } @@ -4147,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); } diff --git a/src/actor/reactor/tests.rs b/src/actor/reactor/tests.rs index ad7263535..c6a01ed17 100644 --- a/src/actor/reactor/tests.rs +++ b/src/actor/reactor/tests.rs @@ -8549,3 +8549,265 @@ fn moving_a_bound_workspace_to_another_display_is_refused() { ); } } + +/// 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 index 1fd09d63d..521d64a8c 100644 --- a/src/actor/reactor/workspace_bindings.rs +++ b/src/actor/reactor/workspace_bindings.rs @@ -12,10 +12,16 @@ //! 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; @@ -290,4 +296,142 @@ impl Reactor { } 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/model/virtual_workspace.rs b/src/model/virtual_workspace.rs index 82747b2ce..974818eb2 100644 --- a/src/model/virtual_workspace.rs +++ b/src/model/virtual_workspace.rs @@ -444,6 +444,14 @@ impl WorkspaceStore { } } + /// 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)