Skip to content

Commit

Permalink
fix(wm): skip layout calc for empty workspaces
Browse files Browse the repository at this point in the history
While investigating issue #2 I was able to reproduce it and view the
panic that causes the komorebi process to become non-responsive.

When switching to a columnar layout (which is the default for the 2nd
workspace in the sample ahk config), there is the possibility to cause a
divide by zero panic if the len passed to Layout::calculate is 0.

I have remedied this by changing the type of len from usize to
NonZeroUsize, and also by ensuring that Layout::calculate is only called
from within the komorebi crate if the workspace has at least one
container.

While moving containers around I also noticed that creating a new
container for a window may also cause a panic if focused_idx + 1 is
greater than the length of the VecDeque of containers, so this was
addressed by pushing to the back of the VecDeque in that case.

re #2
  • Loading branch information
LGUG2Z committed Aug 15, 2021
1 parent a550c08 commit a53b2cc
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 15 deletions.
5 changes: 4 additions & 1 deletion komorebi-core/src/layout.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::num::NonZeroUsize;

use clap::Clap;
use serde::Deserialize;
use serde::Serialize;
Expand Down Expand Up @@ -127,11 +129,12 @@ impl Layout {
pub fn calculate(
&self,
area: &Rect,
len: usize,
len: NonZeroUsize,
container_padding: Option<i32>,
layout_flip: Option<LayoutFlip>,
resize_dimensions: &[Option<Rect>],
) -> Vec<Rect> {
let len = usize::from(len);
let mut dimensions = match self {
Layout::BSP => recursive_fibonacci(
0,
Expand Down
6 changes: 0 additions & 6 deletions komorebi/src/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,6 @@ impl PartialEq for &Container {
}

impl Container {
pub fn hide(&mut self) {
for window in self.windows_mut() {
window.hide();
}
}

pub fn load_focused_window(&mut self) {
let focused_idx = self.focused_window_idx();
for (i, window) in self.windows_mut().iter_mut().enumerate() {
Expand Down
1 change: 1 addition & 0 deletions komorebi/src/ring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ macro_rules! impl_ring_elements {
self.[<$element:lower s>].elements_mut()
}

#[allow(dead_code)]
pub fn [<focused_ $element:lower>](&self) -> Option<&$element> {
self.[<$element:lower s>].focused()
}
Expand Down
1 change: 1 addition & 0 deletions komorebi/src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ impl Window {
WindowsApi::set_focus(self.hwnd())
}

#[allow(dead_code)]
pub fn update_style(self, style: GwlStyle) -> Result<()> {
WindowsApi::update_style(self.hwnd(), isize::try_from(style.bits())?)
}
Expand Down
5 changes: 4 additions & 1 deletion komorebi/src/window_manager.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::VecDeque;
use std::io::ErrorKind;
use std::num::NonZeroUsize;
use std::sync::Arc;
use std::sync::Mutex;

Expand Down Expand Up @@ -126,7 +127,9 @@ impl WindowManager {
) {
let unaltered = workspace.layout().calculate(
&work_area,
len,
NonZeroUsize::new(len).context(
"there must be at least one container to calculate a workspace layout",
)?,
workspace.container_padding(),
workspace.layout_flip(),
&[],
Expand Down
2 changes: 2 additions & 0 deletions komorebi/src/windows_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ impl WindowsApi {
}
}

#[allow(dead_code)]
fn set_window_long_ptr_w(
hwnd: HWND,
index: WINDOW_LONG_PTR_INDEX,
Expand All @@ -396,6 +397,7 @@ impl WindowsApi {
}))
}

#[allow(dead_code)]
pub fn update_style(hwnd: HWND, new_value: isize) -> Result<()> {
Self::set_window_long_ptr_w(hwnd, GWL_STYLE, new_value)
}
Expand Down
5 changes: 4 additions & 1 deletion komorebi/src/winevent.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use strum::Display;

use bindings::Windows::Win32::UI::WindowsAndMessaging::EVENT_AIA_END;
use bindings::Windows::Win32::UI::WindowsAndMessaging::EVENT_AIA_START;
use bindings::Windows::Win32::UI::WindowsAndMessaging::EVENT_CONSOLE_CARET;
Expand Down Expand Up @@ -83,8 +85,9 @@ use bindings::Windows::Win32::UI::WindowsAndMessaging::EVENT_UIA_EVENTID_START;
use bindings::Windows::Win32::UI::WindowsAndMessaging::EVENT_UIA_PROPID_END;
use bindings::Windows::Win32::UI::WindowsAndMessaging::EVENT_UIA_PROPID_START;

#[derive(Clone, Copy, PartialEq, Debug, strum::Display)]
#[derive(Clone, Copy, PartialEq, Debug, Display)]
#[repr(u32)]
#[allow(dead_code)]
pub enum WinEvent {
AiaEnd = EVENT_AIA_END,
AiaStart = EVENT_AIA_START,
Expand Down
25 changes: 19 additions & 6 deletions komorebi/src/workspace.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::VecDeque;
use std::num::NonZeroUsize;

use color_eyre::eyre::ContextCompat;
use color_eyre::Result;
Expand Down Expand Up @@ -110,10 +111,12 @@ impl Workspace {
if let Some(window) = container.focused_window_mut() {
window.set_position(&adjusted_work_area, true)?;
}
} else {
} else if !self.containers().is_empty() {
let layouts = self.layout().calculate(
&adjusted_work_area,
self.containers().len(),
NonZeroUsize::new(self.containers().len()).context(
"there must be at least one container to calculate a workspace layout",
)?,
self.container_padding(),
self.layout_flip(),
self.resize_dimensions(),
Expand Down Expand Up @@ -409,14 +412,24 @@ impl Workspace {
}

pub fn new_container_for_window(&mut self, window: Window) {
let focused_idx = self.focused_container_idx();
let next_idx = self.focused_container_idx() + 1;

let mut container = Container::default();
container.add_window(window);

self.containers_mut().insert(focused_idx + 1, container);
self.resize_dimensions_mut().insert(focused_idx + 1, None);
self.focus_container(focused_idx + 1);
if next_idx > self.containers().len() {
self.containers_mut().push_back(container);
} else {
self.containers_mut().insert(next_idx, container);
}

if next_idx > self.resize_dimensions().len() {
self.resize_dimensions_mut().push(None);
} else {
self.resize_dimensions_mut().insert(next_idx, None);
}

self.focus_container(next_idx);
}

pub fn new_floating_window(&mut self) -> Result<()> {
Expand Down

0 comments on commit a53b2cc

Please sign in to comment.