From 37f1a163cc9e56c590756d1e555038d2f4bbf659 Mon Sep 17 00:00:00 2001 From: LGUG2Z Date: Mon, 24 Oct 2022 16:19:10 -0700 Subject: [PATCH] fix(wm): match display name / avoid id volatility This commit replaces all usages of MONITORINFO with MONITORINFOEX in order to retrieve a name for each connected display device. This display device name makes for easier deduping during monitor reconciliation, so that matching display monitor names can simply have their hmonitor id updated instead of trying to figure out which id corresponds to which monitor by looking at the windows currently visible on each. fix #267 --- komorebi/src/monitor.rs | 5 +- komorebi/src/window_manager.rs | 90 +++++++++++++++++-------------- komorebi/src/windows_api.rs | 50 +++++++++++------ komorebi/src/windows_callbacks.rs | 7 ++- 4 files changed, 92 insertions(+), 60 deletions(-) diff --git a/komorebi/src/monitor.rs b/komorebi/src/monitor.rs index ead80f01..80aebcb1 100644 --- a/komorebi/src/monitor.rs +++ b/komorebi/src/monitor.rs @@ -21,6 +21,8 @@ pub struct Monitor { #[getset(get_copy = "pub", set = "pub")] id: isize, #[getset(get = "pub", set = "pub")] + name: String, + #[getset(get = "pub", set = "pub")] size: Rect, #[getset(get = "pub", set = "pub")] work_area_size: Rect, @@ -32,12 +34,13 @@ pub struct Monitor { impl_ring_elements!(Monitor, Workspace); -pub fn new(id: isize, size: Rect, work_area_size: Rect) -> Monitor { +pub fn new(id: isize, size: Rect, work_area_size: Rect, name: String) -> Monitor { let mut workspaces = Ring::default(); workspaces.elements_mut().push_back(Workspace::default()); Monitor { id, + name, size, work_area_size, workspaces, diff --git a/komorebi/src/window_manager.rs b/komorebi/src/window_manager.rs index 6b140b00..56e8cdb5 100644 --- a/komorebi/src/window_manager.rs +++ b/komorebi/src/window_manager.rs @@ -327,65 +327,63 @@ impl WindowManager { #[tracing::instrument(skip(self))] pub fn reconcile_monitors(&mut self) -> Result<()> { let valid_hmonitors = WindowsApi::valid_hmonitors()?; - let mut invalid = vec![]; - let mut updated = vec![]; - let mut overlapping = vec![]; + let mut valid_names = vec![]; + let before_count = self.monitors().len(); for monitor in self.monitors_mut() { - if !valid_hmonitors.contains(&monitor.id()) { - let mut mark_as_invalid = true; - - // If an invalid hmonitor has at least one window in the window manager state, - // we can attempt to update its hmonitor id in-place so that it doesn't get reaped - // - // This needs to be done because when monitors are attached and detached, even - // monitors that remained connected get assigned new HMONITOR values - if let Some(workspace) = monitor.focused_workspace() { - if let Some(container) = workspace.focused_container() { - if let Some(window) = container.focused_window() { - let actual_hmonitor = WindowsApi::monitor_from_window(window.hwnd()); - if actual_hmonitor != monitor.id() { - monitor.set_id(actual_hmonitor); - - if updated.contains(&actual_hmonitor) { - overlapping.push(monitor.clone()); - } else { - mark_as_invalid = false; - updated.push(actual_hmonitor); - } - } - } - } + for (valid_name, valid_id) in &valid_hmonitors { + let actual_name = monitor.name().clone(); + if actual_name == *valid_name { + monitor.set_id(*valid_id); + valid_names.push(actual_name); } + } + } - if mark_as_invalid { - invalid.push(monitor.id()); + let mut orphaned_containers = vec![]; + + for invalid in self + .monitors() + .iter() + .filter(|m| !valid_names.contains(m.name())) + { + for workspace in invalid.workspaces() { + for container in workspace.containers() { + // Save the orphaned containers from an invalid monitor + // (which has most likely been disconnected) + orphaned_containers.push(container.clone()); } } } // Remove any invalid monitors from our state - self.monitors_mut().retain(|m| !invalid.contains(&m.id())); + self.monitors_mut() + .retain(|m| valid_names.contains(m.name())); - // If monitor IDs are overlapping we are fucked and need to load - // monitor and workspace state again, followed by the configuration - if !overlapping.is_empty() { - WindowsApi::load_monitor_information(&mut self.monitors)?; - WindowsApi::load_workspace_information(&mut self.monitors)?; + let after_count = self.monitors().len(); - std::thread::spawn(|| { - load_configuration().expect("could not load configuration"); - }); + if let Some(primary) = self.monitors_mut().front_mut() { + if let Some(focused_ws) = primary.focused_workspace_mut() { + let focused_container_idx = focused_ws.focused_container_idx(); + + // Put the orphaned containers somewhere visible + for container in orphaned_containers { + focused_ws.add_container(container); + } + + // Gotta reset the focus or the movement will feel "off" + focused_ws.focus_container(focused_container_idx); + } } let invisible_borders = self.invisible_borders; let offset = self.work_area_offset; for monitor in self.monitors_mut() { - let mut should_update = false; - let reference = WindowsApi::monitor(monitor.id())?; - // TODO: If this is different, force a redraw + // If we have lost a monitor, update everything to filter out any jank + let mut should_update = before_count != after_count; + let reference = WindowsApi::monitor(monitor.id())?; if reference.work_area_size() != monitor.work_area_size() { monitor.set_work_area_size(Rect { left: reference.work_area_size().left, @@ -416,6 +414,16 @@ impl WindowManager { // Check for and add any new monitors that may have been plugged in WindowsApi::load_monitor_information(&mut self.monitors)?; + let final_count = self.monitors().len(); + if after_count != final_count { + self.retile_all(true)?; + // Second retile to fix DPI/resolution related jank when a window + // moves between monitors with different resolutions - this doesn't + // really get seen by the user since the screens are flickering anyway + // as a result of the display connections / disconnections + self.retile_all(true)?; + } + Ok(()) } diff --git a/komorebi/src/windows_api.rs b/komorebi/src/windows_api.rs index c51df37d..1e840f0b 100644 --- a/komorebi/src/windows_api.rs +++ b/komorebi/src/windows_api.rs @@ -1,6 +1,8 @@ use std::collections::VecDeque; use std::convert::TryFrom; use std::ffi::c_void; +use std::ffi::OsString; +use std::os::windows::ffi::OsStringExt; use std::sync::atomic::Ordering; use color_eyre::eyre::anyhow; @@ -36,7 +38,7 @@ use windows::Win32::Graphics::Gdi::HBRUSH; use windows::Win32::Graphics::Gdi::HDC; use windows::Win32::Graphics::Gdi::HMONITOR; use windows::Win32::Graphics::Gdi::MONITORENUMPROC; -use windows::Win32::Graphics::Gdi::MONITORINFO; +use windows::Win32::Graphics::Gdi::MONITORINFOEXW; use windows::Win32::Graphics::Gdi::MONITOR_DEFAULTTONEAREST; use windows::Win32::System::LibraryLoader::GetModuleHandleW; use windows::Win32::System::RemoteDesktop::ProcessIdToSessionId; @@ -202,12 +204,12 @@ impl WindowsApi { .process() } - pub fn valid_hmonitors() -> Result> { - let mut monitors: Vec = vec![]; - let monitors_ref: &mut Vec = monitors.as_mut(); + pub fn valid_hmonitors() -> Result> { + let mut monitors: Vec<(String, isize)> = vec![]; + let monitors_ref: &mut Vec<(String, isize)> = monitors.as_mut(); Self::enum_display_monitors( Option::Some(windows_callbacks::valid_display_monitors), - monitors_ref as *mut Vec as isize, + monitors_ref as *mut Vec<(String, isize)> as isize, )?; Ok(monitors) @@ -228,7 +230,7 @@ impl WindowsApi { pub fn load_workspace_information(monitors: &mut Ring) -> Result<()> { for monitor in monitors.elements_mut() { - let monitor_id = monitor.id(); + let monitor_name = monitor.name().clone(); if let Some(workspace) = monitor.workspaces_mut().front_mut() { // EnumWindows will enumerate through windows on all monitors Self::enum_windows( @@ -246,7 +248,7 @@ impl WindowsApi { for container in workspace.containers_mut() { for window in container.windows() { - if Self::monitor_from_window(window.hwnd()) != monitor_id { + if Self::monitor_name_from_window(window.hwnd())? != monitor_name { windows_on_other_monitors.push(window.hwnd().0); } } @@ -273,6 +275,16 @@ impl WindowsApi { unsafe { MonitorFromWindow(hwnd, MONITOR_DEFAULTTONEAREST) }.0 } + pub fn monitor_name_from_window(hwnd: HWND) -> Result { + // MONITOR_DEFAULTTONEAREST ensures that the return value will never be NULL + // https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-monitorfromwindow + Ok( + Self::monitor(unsafe { MonitorFromWindow(hwnd, MONITOR_DEFAULTTONEAREST) }.0)? + .name() + .to_string(), + ) + } + pub fn monitor_from_point(point: POINT) -> isize { // MONITOR_DEFAULTTONEAREST ensures that the return value will never be NULL // https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-monitorfromwindow @@ -612,24 +624,30 @@ impl WindowsApi { unsafe { IsIconic(hwnd) }.into() } - pub fn monitor_info_w(hmonitor: HMONITOR) -> Result { - let mut monitor_info: MONITORINFO = unsafe { std::mem::zeroed() }; - monitor_info.cbSize = u32::try_from(std::mem::size_of::())?; - - unsafe { GetMonitorInfoW(hmonitor, &mut monitor_info) } + pub fn monitor_info_w(hmonitor: HMONITOR) -> Result { + let mut ex_info = MONITORINFOEXW::default(); + ex_info.monitorInfo.cbSize = u32::try_from(std::mem::size_of::())?; + unsafe { GetMonitorInfoW(hmonitor, &mut ex_info.monitorInfo) } .ok() .process()?; - Ok(monitor_info) + Ok(ex_info) } pub fn monitor(hmonitor: isize) -> Result { - let monitor_info = Self::monitor_info_w(HMONITOR(hmonitor))?; + let ex_info = Self::monitor_info_w(HMONITOR(hmonitor))?; + let name = OsString::from_wide(&ex_info.szDevice); + let name = name + .to_string_lossy() + .replace('\u{0000}', "") + .trim_start_matches(r"\\.\") + .to_string(); Ok(monitor::new( hmonitor, - monitor_info.rcMonitor.into(), - monitor_info.rcWork.into(), + ex_info.monitorInfo.rcMonitor.into(), + ex_info.monitorInfo.rcWork.into(), + name, )) } diff --git a/komorebi/src/windows_callbacks.rs b/komorebi/src/windows_callbacks.rs index 38e88421..596d266c 100644 --- a/komorebi/src/windows_callbacks.rs +++ b/komorebi/src/windows_callbacks.rs @@ -47,8 +47,11 @@ pub extern "system" fn valid_display_monitors( _: *mut RECT, lparam: LPARAM, ) -> BOOL { - let monitors = unsafe { &mut *(lparam.0 as *mut Vec) }; - monitors.push(hmonitor.0); + let monitors = unsafe { &mut *(lparam.0 as *mut Vec<(String, isize)>) }; + if let Ok(m) = WindowsApi::monitor(hmonitor.0) { + monitors.push((m.name().to_string(), hmonitor.0)); + } + true.into() }