From 55033236951391c6356895dcf6e8dc1e73ccf57a Mon Sep 17 00:00:00 2001 From: alex-ds13 <145657253+alex-ds13@users.noreply.github.com> Date: Thu, 17 Oct 2024 19:21:18 +0100 Subject: [PATCH] fix(wm): handle moving windows to/from floating workspaces This commit fixes the issue related to moving windows to/from a floating workspace to a tiled workspace. Previously the start of the move would be ignored however when moving back from a tiled workspace since it didn't know about the existance of that window it would also "move" that workspace focused tiled window without physically moving it, leaving it in a weird state that seemed like it was unmanaged. This commit changes the way this mouse moves are handled and now also handles moving `floating_windows` and even monocle or maximized windows. --- komorebi/src/process_event.rs | 324 +++++++++++++++++---------------- komorebi/src/static_config.rs | 1 + komorebi/src/window_manager.rs | 131 ++++++++++++- 3 files changed, 297 insertions(+), 159 deletions(-) diff --git a/komorebi/src/process_event.rs b/komorebi/src/process_event.rs index ea98a154..d8606aee 100644 --- a/komorebi/src/process_event.rs +++ b/komorebi/src/process_event.rs @@ -279,166 +279,170 @@ impl WindowManager { WindowManagerEvent::Show(_, window) | WindowManagerEvent::Manage(window) | WindowManagerEvent::Uncloak(_, window) => { - let focused_monitor_idx = self.focused_monitor_idx(); - let focused_workspace_idx = - self.focused_workspace_idx_for_monitor_idx(focused_monitor_idx)?; + if matches!(event, WindowManagerEvent::Uncloak(_, _)) + && self.uncloack_to_ignore >= 1 + { + tracing::info!("ignoring uncloak after monocle move by mouse across monitors"); + self.uncloack_to_ignore = self.uncloack_to_ignore.saturating_sub(1); + } else { + let focused_monitor_idx = self.focused_monitor_idx(); + let focused_workspace_idx = + self.focused_workspace_idx_for_monitor_idx(focused_monitor_idx)?; - let focused_pair = (focused_monitor_idx, focused_workspace_idx); + let focused_pair = (focused_monitor_idx, focused_workspace_idx); - let mut needs_reconciliation = false; + let mut needs_reconciliation = false; - for (i, monitors) in self.monitors().iter().enumerate() { - for (j, workspace) in monitors.workspaces().iter().enumerate() { - if workspace.contains_window(window.hwnd) && focused_pair != (i, j) { - // At this point we know we are going to send a notification to the workspace reconciliator - // So we get the topmost window returned by EnumWindows, which is almost always the window - // that has been selected by alt-tab - if let Ok(alt_tab_windows) = WindowsApi::alt_tab_windows() { - if let Some(first) = - alt_tab_windows.iter().find(|w| w.title().is_ok()) + for (i, monitors) in self.monitors().iter().enumerate() { + for (j, workspace) in monitors.workspaces().iter().enumerate() { + if workspace.contains_window(window.hwnd) && focused_pair != (i, j) { + // At this point we know we are going to send a notification to the workspace reconciliator + // So we get the topmost window returned by EnumWindows, which is almost always the window + // that has been selected by alt-tab + if let Ok(alt_tab_windows) = WindowsApi::alt_tab_windows() { + if let Some(first) = + alt_tab_windows.iter().find(|w| w.title().is_ok()) + { + // If our record of this HWND hasn't been updated in over a minute + let mut instant = ALT_TAB_HWND_INSTANT.lock(); + if instant.elapsed().gt(&Duration::from_secs(1)) { + // Update our record with the HWND we just found + ALT_TAB_HWND.store(Some(first.hwnd)); + // Update the timestamp of our record + *instant = Instant::now(); + } + } + } + + workspace_reconciliator::send_notification(i, j); + needs_reconciliation = true; + } + } + } + + // There are some applications such as Firefox where, if they are focused when a + // workspace switch takes place, it will fire an additional Show event, which will + // result in them being associated with both the original workspace and the workspace + // being switched to. This loop is to try to ensure that we don't end up with + // duplicates across multiple workspaces, as it results in ghost layout tiles. + let mut proceed = true; + + for (i, monitor) in self.monitors().iter().enumerate() { + for (j, workspace) in monitor.workspaces().iter().enumerate() { + if workspace.contains_window(window.hwnd) + && i != self.focused_monitor_idx() + && j != monitor.focused_workspace_idx() + { + tracing::debug!( + "ignoring show event for window already associated with another workspace" + ); + + window.hide(); + proceed = false; + } + } + } + + if proceed { + let mut behaviour = self.window_management_behaviour( + focused_monitor_idx, + focused_workspace_idx, + ); + let workspace = self.focused_workspace_mut()?; + let workspace_contains_window = workspace.contains_window(window.hwnd); + let monocle_container = workspace.monocle_container().clone(); + + if !workspace_contains_window && !needs_reconciliation { + let floating_applications = FLOATING_APPLICATIONS.lock(); + let regex_identifiers = REGEX_IDENTIFIERS.lock(); + let mut should_float = false; + + if !floating_applications.is_empty() { + if let (Ok(title), Ok(exe_name), Ok(class), Ok(path)) = + (window.title(), window.exe(), window.class(), window.path()) { - // If our record of this HWND hasn't been updated in over a minute - let mut instant = ALT_TAB_HWND_INSTANT.lock(); - if instant.elapsed().gt(&Duration::from_secs(1)) { - // Update our record with the HWND we just found - ALT_TAB_HWND.store(Some(first.hwnd)); - // Update the timestamp of our record - *instant = Instant::now(); + should_float = should_act( + &title, + &exe_name, + &class, + &path, + &floating_applications, + ®ex_identifiers, + ) + .is_some(); + } + } + + behaviour.float_override = behaviour.float_override + || (should_float + && !matches!(event, WindowManagerEvent::Manage(_))); + + if behaviour.float_override { + workspace.floating_windows_mut().push(window); + self.update_focused_workspace(false, false)?; + } else { + match behaviour.current_behaviour { + WindowContainerBehaviour::Create => { + workspace.new_container_for_window(window); + self.update_focused_workspace(false, false)?; + } + WindowContainerBehaviour::Append => { + workspace + .focused_container_mut() + .ok_or_else(|| { + anyhow!("there is no focused container") + })? + .add_window(window); + self.update_focused_workspace(true, false)?; + stackbar_manager::send_notification(); } } } - workspace_reconciliator::send_notification(i, j); - needs_reconciliation = true; - } - } - } - - // There are some applications such as Firefox where, if they are focused when a - // workspace switch takes place, it will fire an additional Show event, which will - // result in them being associated with both the original workspace and the workspace - // being switched to. This loop is to try to ensure that we don't end up with - // duplicates across multiple workspaces, as it results in ghost layout tiles. - let mut proceed = true; - - for (i, monitor) in self.monitors().iter().enumerate() { - for (j, workspace) in monitor.workspaces().iter().enumerate() { - if workspace.contains_window(window.hwnd) - && i != self.focused_monitor_idx() - && j != monitor.focused_workspace_idx() - { - tracing::debug!( - "ignoring show event for window already associated with another workspace" - ); - - window.hide(); - proceed = false; - } - } - } - - if proceed { - let mut behaviour = self - .window_management_behaviour(focused_monitor_idx, focused_workspace_idx); - let workspace = self.focused_workspace_mut()?; - let workspace_contains_window = workspace.contains_window(window.hwnd); - let monocle_container = workspace.monocle_container().clone(); - - if !workspace_contains_window && !needs_reconciliation { - let floating_applications = FLOATING_APPLICATIONS.lock(); - let regex_identifiers = REGEX_IDENTIFIERS.lock(); - let mut should_float = false; - - if !floating_applications.is_empty() { - if let (Ok(title), Ok(exe_name), Ok(class), Ok(path)) = - (window.title(), window.exe(), window.class(), window.path()) + if (self.focused_workspace()?.containers().len() == 1 + && self.focused_workspace()?.floating_windows().is_empty()) + || (self.focused_workspace()?.containers().is_empty() + && self.focused_workspace()?.floating_windows().len() == 1) { - should_float = should_act( - &title, - &exe_name, - &class, - &path, - &floating_applications, - ®ex_identifiers, - ) - .is_some(); + // If after adding this window the workspace only contains 1 window, it + // means it was previously empty and we focused the desktop to unfocus + // any previous window from other workspace, so now we need to focus + // this window again. This is needed because sometimes some windows + // first send the `FocusChange` event and only the `Show` event after + // and we will be focusing the desktop on the `FocusChange` event since + // it is still empty. + window.focus(self.mouse_follows_focus)?; } } - behaviour.float_override = behaviour.float_override - || (should_float && !matches!(event, WindowManagerEvent::Manage(_))); - - if behaviour.float_override { - workspace.floating_windows_mut().push(window); - self.update_focused_workspace(false, false)?; - } else { - match behaviour.current_behaviour { - WindowContainerBehaviour::Create => { - workspace.new_container_for_window(window); - self.update_focused_workspace(false, false)?; - } - WindowContainerBehaviour::Append => { - workspace - .focused_container_mut() - .ok_or_else(|| anyhow!("there is no focused container"))? - .add_window(window); - self.update_focused_workspace(true, false)?; - stackbar_manager::send_notification(); + if workspace_contains_window { + let mut monocle_window_event = false; + if let Some(ref monocle) = monocle_container { + if let Some(monocle_window) = monocle.focused_window() { + if monocle_window.hwnd == window.hwnd { + monocle_window_event = true; + } } } - } - if (self.focused_workspace()?.containers().len() == 1 - && self.focused_workspace()?.floating_windows().is_empty()) - || (self.focused_workspace()?.containers().is_empty() - && self.focused_workspace()?.floating_windows().len() == 1) - { - // If after adding this window the workspace only contains 1 window, it - // means it was previously empty and we focused the desktop to unfocus - // any previous window from other workspace, so now we need to focus - // this window again. This is needed because sometimes some windows - // first send the `FocusChange` event and only the `Show` event after - // and we will be focusing the desktop on the `FocusChange` event since - // it is still empty. - window.focus(self.mouse_follows_focus)?; - } - } - - if workspace_contains_window { - let mut monocle_window_event = false; - if let Some(ref monocle) = monocle_container { - if let Some(monocle_window) = monocle.focused_window() { - if monocle_window.hwnd == window.hwnd { - monocle_window_event = true; - } + if !monocle_window_event && monocle_container.is_some() { + window.hide(); } } - - if !monocle_window_event && monocle_container.is_some() { - window.hide(); - } } } } WindowManagerEvent::MoveResizeStart(_, window) => { - if *self.focused_workspace()?.tile() { - let monitor_idx = self.focused_monitor_idx(); - let workspace_idx = self - .focused_monitor() - .ok_or_else(|| anyhow!("there is no monitor with this idx"))? - .focused_workspace_idx(); - let container_idx = self - .focused_monitor() - .ok_or_else(|| anyhow!("there is no monitor with this idx"))? - .focused_workspace() - .ok_or_else(|| anyhow!("there is no workspace with this idx"))? - .focused_container_idx(); + let monitor_idx = self.focused_monitor_idx(); + let workspace_idx = self + .focused_monitor() + .ok_or_else(|| anyhow!("there is no monitor with this idx"))? + .focused_workspace_idx(); - WindowsApi::bring_window_to_top(window.hwnd)?; + WindowsApi::bring_window_to_top(window.hwnd)?; - let pending_move_op = Arc::make_mut(&mut self.pending_move_op); - *pending_move_op = Option::from((monitor_idx, workspace_idx, container_idx)); - } + let pending_move_op = Arc::make_mut(&mut self.pending_move_op); + *pending_move_op = Option::from((monitor_idx, workspace_idx, window.hwnd)); } WindowManagerEvent::MoveResizeEnd(_, window) => { // We need this because if the event ends on a different monitor, @@ -448,6 +452,18 @@ impl WindowManager { let pending_move_op = Arc::make_mut(&mut self.pending_move_op); *pending_move_op = None; + // If the window handles don't match then something went wrong and the pending move + // is not related to this current move, if so abort this operation. + if let Some((_, _, w_hwnd)) = pending { + if w_hwnd != window.hwnd { + color_eyre::eyre::bail!( + "window handles for move operation don't match: {} != {}", + w_hwnd, + window.hwnd + ); + } + } + let target_monitor_idx = self .monitor_idx_from_current_pos() .ok_or_else(|| anyhow!("cannot get monitor idx from current position"))?; @@ -492,8 +508,7 @@ impl WindowManager { .get(origin_workspace_idx) .ok_or_else(|| anyhow!("cannot get workspace idx"))?; - let managed_window = - origin_workspace.contains_managed_window(window.hwnd); + let managed_window = origin_workspace.contains_window(window.hwnd); if !managed_window { moved_across_monitors = false; @@ -523,11 +538,8 @@ impl WindowManager { tracing::info!("moving with mouse"); if moved_across_monitors { - if let Some(( - origin_monitor_idx, - origin_workspace_idx, - origin_container_idx, - )) = pending + if let Some((origin_monitor_idx, origin_workspace_idx, w_hwnd)) = + pending { let target_workspace_idx = self .monitors() @@ -547,18 +559,13 @@ impl WindowManager { // Default to 0 in the case of an empty workspace .unwrap_or(0); - self.transfer_container( - ( - origin_monitor_idx, - origin_workspace_idx, - origin_container_idx, - ), - ( - target_monitor_idx, - target_workspace_idx, - target_container_idx, - ), - )?; + let origin = (origin_monitor_idx, origin_workspace_idx, w_hwnd); + let target = ( + target_monitor_idx, + target_workspace_idx, + target_container_idx, + ); + self.transfer_window(origin, target)?; // We want to make sure both the origin and target monitors are updated, // so that we don't have ghost tiles until we force an interaction on @@ -570,9 +577,10 @@ impl WindowManager { self.focus_monitor(target_monitor_idx)?; self.focus_workspace(target_workspace_idx)?; self.update_focused_workspace(false, false)?; + + // Make sure to give focus to the moved window again + window.focus(self.mouse_follows_focus)?; } - // Here we handle a simple move on the same monitor which is treated as - // a container swap } else if window_management_behaviour.float_override { workspace.floating_windows_mut().push(window); self.update_focused_workspace(false, false)?; diff --git a/komorebi/src/static_config.rs b/komorebi/src/static_config.rs index 86c28e55..59317b90 100644 --- a/komorebi/src/static_config.rs +++ b/komorebi/src/static_config.rs @@ -1051,6 +1051,7 @@ impl StaticConfig { has_pending_raise_op: false, pending_move_op: Arc::new(None), already_moved_window_handles: Arc::new(Mutex::new(HashSet::new())), + uncloack_to_ignore: 0, }; match value.focus_follows_mouse { diff --git a/komorebi/src/window_manager.rs b/komorebi/src/window_manager.rs index 6f6fb2a5..9f4916b3 100644 --- a/komorebi/src/window_manager.rs +++ b/komorebi/src/window_manager.rs @@ -102,8 +102,9 @@ pub struct WindowManager { pub hotwatch: Hotwatch, pub virtual_desktop_id: Option>, pub has_pending_raise_op: bool, - pub pending_move_op: Arc>, + pub pending_move_op: Arc>, pub already_moved_window_handles: Arc>>, + pub uncloack_to_ignore: usize, } #[allow(clippy::struct_excessive_bools)] @@ -341,6 +342,7 @@ impl WindowManager { has_pending_raise_op: false, pending_move_op: Arc::new(None), already_moved_window_handles: Arc::new(Mutex::new(HashSet::new())), + uncloack_to_ignore: 0, }) } @@ -819,6 +821,133 @@ impl WindowManager { Ok(()) } + #[tracing::instrument(skip(self))] + pub fn transfer_window( + &mut self, + origin: (usize, usize, isize), + target: (usize, usize, usize), + ) -> Result<()> { + let (origin_monitor_idx, origin_workspace_idx, w_hwnd) = origin; + let (target_monitor_idx, target_workspace_idx, target_container_idx) = target; + + let origin_workspace = self + .monitors_mut() + .get_mut(origin_monitor_idx) + .ok_or_else(|| anyhow!("cannot get monitor idx"))? + .workspaces_mut() + .get_mut(origin_workspace_idx) + .ok_or_else(|| anyhow!("cannot get workspace idx"))?; + + let origin_container_idx = origin_workspace + .container_for_window(w_hwnd) + .and_then(|c| origin_workspace.containers().iter().position(|cc| cc == c)); + + if let Some(origin_container_idx) = origin_container_idx { + // Moving normal container window + self.transfer_container( + ( + origin_monitor_idx, + origin_workspace_idx, + origin_container_idx, + ), + ( + target_monitor_idx, + target_workspace_idx, + target_container_idx, + ), + )?; + } else if let Some(idx) = origin_workspace + .floating_windows() + .iter() + .position(|w| w.hwnd == w_hwnd) + { + // Moving floating window + // There is no need to physically move the floating window between areas with + // `move_to_area` because the user already did that, so we only need to transfer the + // window to the target `floating_windows` + let floating_window = origin_workspace.floating_windows_mut().remove(idx); + + let target_workspace = self + .monitors_mut() + .get_mut(target_monitor_idx) + .ok_or_else(|| anyhow!("there is no monitor at this idx"))? + .focused_workspace_mut() + .ok_or_else(|| anyhow!("there is no focused workspace for this monitor"))?; + + target_workspace + .floating_windows_mut() + .push(floating_window); + } else if origin_workspace + .monocle_container() + .as_ref() + .and_then(|monocle| monocle.focused_window().map(|w| w.hwnd == w_hwnd)) + .unwrap_or_default() + { + // Moving monocle container + if let Some(monocle_idx) = origin_workspace.monocle_container_restore_idx() { + let origin_workspace = self + .monitors_mut() + .get_mut(origin_monitor_idx) + .ok_or_else(|| anyhow!("there is no monitor at this idx"))? + .workspaces_mut() + .get_mut(origin_workspace_idx) + .ok_or_else(|| anyhow!("there is no workspace for this monitor"))?; + let mut uncloack_amount = 0; + for container in origin_workspace.containers_mut() { + container.restore(); + uncloack_amount += 1; + } + origin_workspace.reintegrate_monocle_container()?; + + self.transfer_container( + (origin_monitor_idx, origin_workspace_idx, monocle_idx), + ( + target_monitor_idx, + target_workspace_idx, + target_container_idx, + ), + )?; + // After we restore the origin workspace, some windows that were cloacked + // by the monocle might now be uncloacked which would trigger a workspace + // reconciliation since the focused monitor would be different from origin. + // That workspace reconciliation would focus the window on the origin monitor. + // So we need to ignore the uncloak events produced by the origin workspace + // restore to avoid that issue. + self.uncloack_to_ignore = uncloack_amount; + } + } else if origin_workspace + .maximized_window() + .as_ref() + .map(|max| max.hwnd == w_hwnd) + .unwrap_or_default() + { + // Moving maximized_window + if let Some(maximized_idx) = origin_workspace.maximized_window_restore_idx() { + self.focus_monitor(origin_monitor_idx)?; + let origin_monitor = self + .focused_monitor_mut() + .ok_or_else(|| anyhow!("there is no origin monitor"))?; + origin_monitor.focus_workspace(origin_workspace_idx)?; + self.unmaximize_window()?; + self.focus_monitor(target_monitor_idx)?; + let target_monitor = self + .focused_monitor_mut() + .ok_or_else(|| anyhow!("there is no target monitor"))?; + target_monitor.focus_workspace(target_workspace_idx)?; + + self.transfer_container( + (origin_monitor_idx, origin_workspace_idx, maximized_idx), + ( + target_monitor_idx, + target_workspace_idx, + target_container_idx, + ), + )?; + } + } + Ok(()) + } + #[tracing::instrument(skip(self))] pub fn transfer_container( &mut self,