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

@@ -279,6 +279,12 @@ impl WindowManager {
             WindowManagerEvent::Show(_, window)
             | WindowManagerEvent::Manage(window)
             | WindowManagerEvent::Uncloak(_, window) => {
+                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)?;
@@ -338,8 +344,10 @@ impl WindowManager {
                     }
 
                     if proceed {
-                    let mut behaviour = self
+                        let mut behaviour = self.window_management_behaviour(
-                        .window_management_behaviour(focused_monitor_idx, focused_workspace_idx);
+                            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();
@@ -366,7 +374,8 @@ impl WindowManager {
                             }
 
                             behaviour.float_override = behaviour.float_override
-                            || (should_float && !matches!(event, WindowManagerEvent::Manage(_)));
+                                || (should_float
+                                    && !matches!(event, WindowManagerEvent::Manage(_)));
 
                             if behaviour.float_override {
                                 workspace.floating_windows_mut().push(window);
@@ -380,7 +389,9 @@ impl WindowManager {
                                     WindowContainerBehaviour::Append => {
                                         workspace
                                             .focused_container_mut()
-                                        .ok_or_else(|| anyhow!("there is no focused container"))?
+                                            .ok_or_else(|| {
+                                                anyhow!("there is no focused container")
+                                            })?
                                             .add_window(window);
                                         self.update_focused_workspace(true, false)?;
                                         stackbar_manager::send_notification();
@@ -420,25 +431,18 @@ impl WindowManager {
                         }
                     }
                 }
+            }
             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();
 
                 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));
+                *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 =
+                            let managed_window = origin_workspace.contains_window(window.hwnd);
-                                origin_workspace.contains_managed_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((
+                            if let Some((origin_monitor_idx, origin_workspace_idx, w_hwnd)) =
-                                origin_monitor_idx,
+                                pending
-                                origin_workspace_idx,
-                                origin_container_idx,
-                            )) = 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(
+                                let origin = (origin_monitor_idx, origin_workspace_idx, w_hwnd);
-                                    (
+                                let target = (
-                                        origin_monitor_idx,
-                                        origin_workspace_idx,
-                                        origin_container_idx,
-                                    ),
-                                    (
                                     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)?;

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 {

komorebi/src/window_manager.rs

@@ -102,8 +102,9 @@ pub struct WindowManager {
     pub hotwatch: Hotwatch,
     pub virtual_desktop_id: Option<Vec<u8>>,
     pub has_pending_raise_op: bool,
-    pub pending_move_op: Arc<Option<(usize, usize, usize)>>,
+    pub pending_move_op: Arc<Option<(usize, usize, isize)>>,
     pub already_moved_window_handles: Arc<Mutex<HashSet<isize>>>,
+    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,