Skip to content

Commit

Permalink
fix(wm): move floats in direction across monitors
Browse files Browse the repository at this point in the history 
This commit fixes an issue where when trying to move floating windows or
windows on a floating workspace across boundaries to another monitor
using the `move_container_in_direction` it wouldn't move the floating
windows physically, although it moved them internally on komorebi,
resulting in weird and wrong behavior.

This commit creates a new method on `Monitor` to
`add_container_with_direction` which takes a move direction and then
uses the same logic that was previously on the
`move_container_in_direction` function.
It changes the `move_container_to_monitor` function to take an optional
move direction which if it is some will have this function call the new
method `add_container_with_direction` instead of just `add_container`.
Lastly the `move_container_in_direction` function now when it realizes
the move will be across monitors simply calls the
`move_container_to_monitor` with the direction that was initially given
to it.
These changes require that all callers of `move_container_to_monitor`
add an direction option, instead of passing `None` on all of them, a new
helper function was created, named `direction_from_monitor_idx` which
calculates the direction a move will have from the currently focused
monitor and the target monitor return `None` if they are the same or
returning `Some(direction)` if not. This way now all commands that call
a move across monitor will use the logic to check from the direction if
it should add the container on front or end.
  • Loading branch information
@alex-ds13
alex-ds13 committed Nov 28, 2024
1 parent 1ef0630 commit 8798320
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 90 deletions.
79 changes: 79 additions & 0 deletions komorebi/src/monitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,85 @@ impl Monitor {
Ok(())
}

/// Adds a container to this `Monitor` using the move direction to calculate if the container
/// should be added in front of all containers, in the back or in place of the focused
/// container, moving the rest along.
pub fn add_container_with_direction(
&mut self,
container: Container,
workspace_idx: Option<usize>,
direction: OperationDirection,
) -> Result<()> {
let workspace = if let Some(idx) = workspace_idx {
self.workspaces_mut()
.get_mut(idx)
.ok_or_else(|| anyhow!("there is no workspace at index {}", idx))?
} else {
self.focused_workspace_mut()
.ok_or_else(|| anyhow!("there is no workspace"))?
};

match direction {
OperationDirection::Left => {
// insert the container into the workspace on the monitor at the back (or rightmost position)
// if we are moving across a boundary to the left (back = right side of the target)
match workspace.layout() {
Layout::Default(layout) => match layout {
DefaultLayout::RightMainVerticalStack => {
workspace.add_container_to_front(container);
}
DefaultLayout::UltrawideVerticalStack => {
if workspace.containers().len() == 1 {
workspace.insert_container_at_idx(0, container);
} else {
workspace.add_container_to_back(container);
}
}
_ => {
workspace.add_container_to_back(container);
}
},
Layout::Custom(_) => {
workspace.add_container_to_back(container);
}
}
}
OperationDirection::Right => {
// insert the container into the workspace on the monitor at the front (or leftmost position)
// if we are moving across a boundary to the right (front = left side of the target)
match workspace.layout() {
Layout::Default(layout) => {
let target_index = layout.leftmost_index(workspace.containers().len());

match layout {
DefaultLayout::RightMainVerticalStack
| DefaultLayout::UltrawideVerticalStack => {
if workspace.containers().len() == 1 {
workspace.add_container_to_back(container);
} else {
workspace.insert_container_at_idx(target_index, container);
}
}
_ => {
workspace.insert_container_at_idx(target_index, container);
}
}
}
Layout::Custom(_) => {
workspace.add_container_to_front(container);
}
}
}
OperationDirection::Up | OperationDirection::Down => {
// insert the container into the workspace on the monitor at the position
// where the currently focused container on that workspace is
workspace.insert_container_at_idx(workspace.focused_container_idx(), container);
}
};

Ok(())
}

pub fn remove_workspace_by_idx(&mut self, idx: usize) -> Option<Workspace> {
if idx < self.workspaces().len() {
return self.workspaces_mut().remove(idx);
Expand Down
38 changes: 31 additions & 7 deletions komorebi/src/process_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,8 @@ impl WindowManager {
self.move_container_to_workspace(workspace_idx, true, None)?;
}
SocketMessage::MoveContainerToMonitorNumber(monitor_idx) => {
self.move_container_to_monitor(monitor_idx, None, true)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(monitor_idx, None, true, direction)?;
}
SocketMessage::SwapWorkspacesToMonitorNumber(monitor_idx) => {
self.swap_focused_monitor(monitor_idx)?;
Expand All @@ -548,7 +549,8 @@ impl WindowManager {
.ok_or_else(|| anyhow!("there must be at least one monitor"))?,
);

self.move_container_to_monitor(monitor_idx, None, true)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(monitor_idx, None, true, direction)?;
}
SocketMessage::SendContainerToWorkspaceNumber(workspace_idx) => {
self.move_container_to_workspace(workspace_idx, false, None)?;
Expand All @@ -570,7 +572,8 @@ impl WindowManager {
self.move_container_to_workspace(workspace_idx, false, None)?;
}
SocketMessage::SendContainerToMonitorNumber(monitor_idx) => {
self.move_container_to_monitor(monitor_idx, None, false)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(monitor_idx, None, false, direction)?;
}
SocketMessage::CycleSendContainerToMonitor(direction) => {
let monitor_idx = direction.next_idx(
Expand All @@ -579,30 +582,51 @@ impl WindowManager {
.ok_or_else(|| anyhow!("there must be at least one monitor"))?,
);

self.move_container_to_monitor(monitor_idx, None, false)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(monitor_idx, None, false, direction)?;
}
SocketMessage::SendContainerToMonitorWorkspaceNumber(monitor_idx, workspace_idx) => {
self.move_container_to_monitor(monitor_idx, Option::from(workspace_idx), false)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(
monitor_idx,
Option::from(workspace_idx),
false,
direction,
)?;
}
SocketMessage::MoveContainerToMonitorWorkspaceNumber(monitor_idx, workspace_idx) => {
self.move_container_to_monitor(monitor_idx, Option::from(workspace_idx), true)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(
monitor_idx,
Option::from(workspace_idx),
true,
direction,
)?;
}
SocketMessage::SendContainerToNamedWorkspace(ref workspace) => {
if let Some((monitor_idx, workspace_idx)) =
self.monitor_workspace_index_by_name(workspace)
{
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(
monitor_idx,
Option::from(workspace_idx),
false,
direction,
)?;
}
}
SocketMessage::MoveContainerToNamedWorkspace(ref workspace) => {
if let Some((monitor_idx, workspace_idx)) =
self.monitor_workspace_index_by_name(workspace)
{
self.move_container_to_monitor(monitor_idx, Option::from(workspace_idx), true)?;
let direction = self.direction_from_monitor_idx(monitor_idx);
self.move_container_to_monitor(
monitor_idx,
Option::from(workspace_idx),
true,
direction,
)?;
}
}

Expand Down
122 changes: 39 additions & 83 deletions komorebi/src/window_manager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,37 @@ impl WindowManager {
None
}

/// Calculates the direction of a move across monitors given a specific monitor index
pub fn direction_from_monitor_idx(&self, target_monitor_idx: usize) -> Option<OperationDirection> {
let current_monitor_idx = self.focused_monitor_idx();
if current_monitor_idx == target_monitor_idx {
return None;
}

let current_monitor_size = self.focused_monitor_size().ok()?;
let target_monitor_size = *self
.monitors()
.get(target_monitor_idx)?
.size();

if target_monitor_size.left + target_monitor_size.right == current_monitor_size.left {
return Some(OperationDirection::Left);
}
if current_monitor_size.right + current_monitor_size.left == target_monitor_size.left
{
return Some(OperationDirection::Right);
}
if target_monitor_size.top + target_monitor_size.bottom == current_monitor_size.top {
return Some(OperationDirection::Up);
}
if current_monitor_size.top + current_monitor_size.bottom == target_monitor_size.top
{
return Some(OperationDirection::Down);
}

None
}

#[allow(clippy::too_many_arguments)]
#[tracing::instrument(skip(self), level = "debug")]
fn add_window_handle_to_move_based_on_workspace_rule(
Expand Down Expand Up @@ -1383,6 +1414,7 @@ impl WindowManager {
monitor_idx: usize,
workspace_idx: Option<usize>,
follow: bool,
move_direction: Option<OperationDirection>,
) -> Result<()> {
self.handle_unmanaged_window_behaviour()?;

Expand Down Expand Up @@ -1455,7 +1487,11 @@ impl WindowManager {
.map(|w| w.hwnd)
.collect::<Vec<_>>();

target_monitor.add_container(container, workspace_idx)?;
if let Some(direction) = move_direction {
target_monitor.add_container_with_direction(container, workspace_idx, direction)?;
} else {
target_monitor.add_container(container, workspace_idx)?;
}

if let Some(workspace) = target_monitor.focused_workspace() {
if !*workspace.tile() {
Expand Down Expand Up @@ -1765,15 +1801,8 @@ impl WindowManager {
.ok_or_else(|| anyhow!("there is no container or monitor in this direction"))?;

{
// remove the container from the origin monitor workspace
let origin_container = self
.focused_workspace_mut()?
.remove_container_by_idx(origin_container_idx)
.ok_or_else(|| {
anyhow!("could not remove container at given origin index")
})?;

self.focused_workspace_mut()?.focus_previous_container();
// actually move the container to target monitor using the direction
self.move_container_to_monitor(target_monitor_idx, None, true, Some(direction))?;

// focus the target monitor
self.focus_monitor(target_monitor_idx)?;
Expand All @@ -1793,79 +1822,6 @@ impl WindowManager {
// get a mutable ref to the focused workspace on the target monitor
let target_workspace = self.focused_workspace_mut()?;

match direction {
OperationDirection::Left => {
// insert the origin container into the focused workspace on the target monitor
// at the back (or rightmost position) if we are moving across a boundary to
// the left (back = right side of the target)
match target_workspace.layout() {
Layout::Default(layout) => match layout {
DefaultLayout::RightMainVerticalStack => {
target_workspace.add_container_to_front(origin_container);
}
DefaultLayout::UltrawideVerticalStack => {
if target_workspace.containers().len() == 1 {
target_workspace
.insert_container_at_idx(0, origin_container);
} else {
target_workspace
.add_container_to_back(origin_container);
}
}
_ => {
target_workspace.add_container_to_back(origin_container);
}
},
Layout::Custom(_) => {
target_workspace.add_container_to_back(origin_container);
}
}
}
OperationDirection::Right => {
// insert the origin container into the focused workspace on the target monitor
// at the front (or leftmost position) if we are moving across a boundary to the
// right (front = left side of the target)
match target_workspace.layout() {
Layout::Default(layout) => {
let target_index =
layout.leftmost_index(target_workspace.containers().len());

match layout {
DefaultLayout::RightMainVerticalStack
| DefaultLayout::UltrawideVerticalStack => {
if target_workspace.containers().len() == 1 {
target_workspace
.add_container_to_back(origin_container);
} else {
target_workspace.insert_container_at_idx(
target_index,
origin_container,
);
}
}
_ => {
target_workspace.insert_container_at_idx(
target_index,
origin_container,
);
}
}
}
Layout::Custom(_) => {
target_workspace.add_container_to_front(origin_container);
}
}
}
OperationDirection::Up | OperationDirection::Down => {
// insert the origin container into the focused workspace on the target monitor
// at the position where the currently focused container on that workspace is
target_workspace.insert_container_at_idx(
target_workspace.focused_container_idx(),
origin_container,
);
}
};

// if there is only one container on the target workspace after the insertion
// it means that there won't be one swapped back, so we have to decrement the
// focused position
Expand Down

0 comments on commit 8798320

Please sign in to comment.