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

@@ -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,

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(())
     }
 

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<Vec<isize>> {
-        let mut monitors: Vec<isize> = vec![];
-        let monitors_ref: &mut Vec<isize> = monitors.as_mut();
+    pub fn valid_hmonitors() -> Result<Vec<(String, isize)>> {
+        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<isize> 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<Monitor>) -> 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<String> {
+        // 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<MONITORINFO> {
-        let mut monitor_info: MONITORINFO = unsafe { std::mem::zeroed() };
-        monitor_info.cbSize = u32::try_from(std::mem::size_of::<MONITORINFO>())?;
-
-        unsafe { GetMonitorInfoW(hmonitor, &mut monitor_info) }
+    pub fn monitor_info_w(hmonitor: HMONITOR) -> Result<MONITORINFOEXW> {
+        let mut ex_info = MONITORINFOEXW::default();
+        ex_info.monitorInfo.cbSize = u32::try_from(std::mem::size_of::<MONITORINFOEXW>())?;
+        unsafe { GetMonitorInfoW(hmonitor, &mut ex_info.monitorInfo) }
             .ok()
             .process()?;
 
-        Ok(monitor_info)
+        Ok(ex_info)
     }
 
     pub fn monitor(hmonitor: isize) -> Result<Monitor> {
-        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,
         ))
     }
 

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<isize>) };
-    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()
 }