fix(wm): cross-monitor ops for floating windows

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.

With these changes now all the code related to moving a window across
monitors using a command should be on one place only making sure that in
the future any change required only needs to be done on one place,
instead of having to do it on `move_container_to_monitor` and
`move_container_in_direction` as before!

This commit is an interactive squashed rebase of the following commits
from PR #1154:

8f4bc101bc
fix(wm): move floats in direction across monitors

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.

3b20e4b2fe
refactor(wm): use helper function on move to workspace

Use the same `add_container_with_direction` function on
`move_container_to_workspace` as it is being used on
`move_container_to_monitor` or `move_container_in_direction`.

This way we bring parity between all methods and make it easier to
change the way a container is added on a monitor workspace when taking
the move direction into consideration.
This commit is contained in:
alex-ds13
2024-11-28 16:14:17 +00:00
committed by LGUG2Z
parent 01367f59e2
commit 8743cdd292
3 changed files with 161 additions and 138 deletions

View File

@@ -139,6 +139,86 @@ 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. The move direction should be from the origin monitor
/// towards the target monitor or from the origin workspace towards the target workspace.
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);
@@ -215,54 +295,14 @@ impl Monitor {
Some(workspace) => workspace,
};
match direction {
Some(OperationDirection::Left) => match target_workspace.layout() {
Layout::Default(layout) => match layout {
DefaultLayout::RightMainVerticalStack => {
target_workspace.add_container_to_front(container);
}
DefaultLayout::UltrawideVerticalStack => {
if target_workspace.containers().len() == 1 {
target_workspace.insert_container_at_idx(0, container);
} else {
target_workspace.add_container_to_back(container);
}
}
_ => {
target_workspace.add_container_to_back(container);
}
},
Layout::Custom(_) => {
target_workspace.add_container_to_back(container);
}
},
Some(OperationDirection::Right) => 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(container);
} else {
target_workspace
.insert_container_at_idx(target_index, container);
}
}
_ => {
target_workspace.insert_container_at_idx(target_index, container);
}
}
}
Layout::Custom(_) => {
target_workspace.add_container_to_front(container);
}
},
_ => {
target_workspace.add_container_to_back(container);
}
if let Some(direction) = direction {
self.add_container_with_direction(
container,
Some(target_workspace_idx),
direction,
)?;
} else {
target_workspace.add_container_to_back(container);
}
}

View File

@@ -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)?;
@@ -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)?;
@@ -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(
@@ -579,22 +582,37 @@ 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,
)?;
}
}
@@ -602,7 +620,13 @@ impl WindowManager {
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,
)?;
}
}

View File

@@ -498,6 +498,35 @@ 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(
@@ -1383,6 +1412,7 @@ impl WindowManager {
monitor_idx: usize,
workspace_idx: Option<usize>,
follow: bool,
move_direction: Option<OperationDirection>,
) -> Result<()> {
self.handle_unmanaged_window_behaviour()?;
@@ -1459,7 +1489,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() {
@@ -1771,15 +1805,13 @@ 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)?;
@@ -1799,79 +1831,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