diff --git a/komorebi/src/process_command.rs b/komorebi/src/process_command.rs index eb59f84a..92b5a948 100644 --- a/komorebi/src/process_command.rs +++ b/komorebi/src/process_command.rs @@ -13,6 +13,7 @@ use std::sync::Arc; use std::time::Duration; use color_eyre::eyre::anyhow; +use color_eyre::eyre::bail; use color_eyre::Result; use miow::pipe::connect; use net2::TcpStreamExt; @@ -1371,7 +1372,33 @@ impl WindowManager { | SocketMessage::FocusMonitorNumber(_) | SocketMessage::FocusMonitorWorkspaceNumber(_, _) | SocketMessage::FocusWorkspaceNumber(_) => { - let foreground = WindowsApi::foreground_window()?; + // The foreground window might be de-activating if we've just + // set it as a result of our own actions, so wait until the new + // one returns. This particularly happens when switching monitors. + // + // TODO(raggi): re-evaluate this branch. I checked the + // suggestion from the comment above, that we don't get + // EVENT_SYSTEM_FOREGROUND, but if I print out trace events I + // see that we do. + // XXX(raggi) We drop FocusChange events though for windows that + // we're not managing, so that's one of the ways that the border + // window gets stuck. We should stop overloading `should_manage` + // as an event filter, and separately filter events that we want + // to handle, and windows that we want to handle, as some events + // must be handled even if we're not managing the target window. + let mut attempts = 0; + let foreground = loop { + match WindowsApi::foreground_window() { + Ok(foreground) => break foreground, + Err(_) => { + std::thread::sleep(std::time::Duration::from_millis(10)); + attempts+=1; + if attempts == 10 { + bail!("failed to get foreground window after 100ms") + } + } + }; + }; let foreground_window = Window { hwnd: foreground }; let monocle = BORDER_COLOUR_MONOCLE.load(Ordering::SeqCst); diff --git a/komorebi/src/window.rs b/komorebi/src/window.rs index eb2e98d9..55b29634 100644 --- a/komorebi/src/window.rs +++ b/komorebi/src/window.rs @@ -218,77 +218,20 @@ impl Window { } pub fn focus(self, mouse_follows_focus: bool) -> Result<()> { - // Attach komorebi thread to Window thread - let (_, window_thread_id) = WindowsApi::window_thread_process_id(self.hwnd()); - let current_thread_id = WindowsApi::current_thread_id(); - - // This can be allowed to fail if a window doesn't have a message queue or if a journal record - // hook has been installed - // https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-attachthreadinput#remarks - match WindowsApi::attach_thread_input(current_thread_id, window_thread_id, true) { - Ok(()) => {} - Err(error) => { - tracing::error!( - "could not attach to window thread input processing mechanism, but continuing execution of focus(): {}", - error - ); + // If the target window is already focused, do nothing. + if let Ok(ihwnd) = WindowsApi::foreground_window() { + if HWND(ihwnd) == self.hwnd() { + return Ok(()); } - }; - - // Raise Window to foreground - let mut foregrounded = false; - let mut tried_resetting_foreground_access = false; - let mut max_attempts = 10; - - while !foregrounded && max_attempts > 0 { - match WindowsApi::set_foreground_window(self.hwnd()) { - Ok(()) => { - foregrounded = true; - } - Err(error) => { - max_attempts -= 1; - tracing::error!( - "could not set as foreground window, but continuing execution of focus(): {}", - error - ); - - // If this still doesn't work then maybe try https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-locksetforegroundwindow - if !tried_resetting_foreground_access { - let process_id = WindowsApi::current_process_id(); - if WindowsApi::allow_set_foreground_window(process_id).is_ok() { - tried_resetting_foreground_access = true; - } - } - } - }; } + WindowsApi::raise_and_focus_window(self.hwnd())?; + // Center cursor in Window if mouse_follows_focus { WindowsApi::center_cursor_in_rect(&WindowsApi::window_rect(self.hwnd())?)?; } - // This isn't really needed when the above command works as expected via AHK - match WindowsApi::set_focus(self.hwnd()) { - Ok(()) => {} - Err(error) => { - tracing::error!( - "could not set focus, but continuing execution of focus(): {}", - error - ); - } - }; - - match WindowsApi::attach_thread_input(current_thread_id, window_thread_id, false) { - Ok(()) => {} - Err(error) => { - tracing::error!( - "could not detach from window thread input processing mechanism, but continuing execution of focus(): {}", - error - ); - } - }; - Ok(()) } diff --git a/komorebi/src/window_manager.rs b/komorebi/src/window_manager.rs index 49b7d055..7dbda4cb 100644 --- a/komorebi/src/window_manager.rs +++ b/komorebi/src/window_manager.rs @@ -822,10 +822,7 @@ impl WindowManager { let rect = self.focused_monitor_size()?; WindowsApi::center_cursor_in_rect(&rect)?; - // Calling this directly instead of the window.focus() wrapper because trying to - // attach to the thread of the desktop window always seems to result in "Access is - // denied (os error 5)" - match WindowsApi::set_foreground_window(desktop_window.hwnd()) { + match WindowsApi::raise_and_focus_window(desktop_window.hwnd()) { Ok(()) => {} Err(error) => { tracing::warn!("{} {}:{}", error, file!(), line!()); diff --git a/komorebi/src/windows_api.rs b/komorebi/src/windows_api.rs index b4c8339e..35f394e1 100644 --- a/komorebi/src/windows_api.rs +++ b/komorebi/src/windows_api.rs @@ -1,6 +1,7 @@ use std::collections::VecDeque; use std::convert::TryFrom; use std::ffi::c_void; +use std::mem::size_of; use std::sync::atomic::Ordering; use color_eyre::eyre::anyhow; @@ -47,9 +48,7 @@ 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; -use windows::Win32::System::Threading::AttachThreadInput; use windows::Win32::System::Threading::GetCurrentProcessId; -use windows::Win32::System::Threading::GetCurrentThreadId; use windows::Win32::System::Threading::OpenProcess; use windows::Win32::System::Threading::QueryFullProcessImageNameW; use windows::Win32::System::Threading::PROCESS_ACCESS_RIGHTS; @@ -61,7 +60,6 @@ use windows::Win32::UI::HiDpi::DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2; use windows::Win32::UI::HiDpi::MDT_EFFECTIVE_DPI; use windows::Win32::UI::Input::KeyboardAndMouse::GetKeyState; use windows::Win32::UI::Input::KeyboardAndMouse::SendInput; -use windows::Win32::UI::Input::KeyboardAndMouse::SetFocus; use windows::Win32::UI::Input::KeyboardAndMouse::INPUT; use windows::Win32::UI::Input::KeyboardAndMouse::INPUT_0; use windows::Win32::UI::Input::KeyboardAndMouse::INPUT_MOUSE; @@ -115,6 +113,9 @@ use windows::Win32::UI::WindowsAndMessaging::SPI_GETACTIVEWINDOWTRACKING; use windows::Win32::UI::WindowsAndMessaging::SPI_GETFOREGROUNDLOCKTIMEOUT; use windows::Win32::UI::WindowsAndMessaging::SPI_SETACTIVEWINDOWTRACKING; use windows::Win32::UI::WindowsAndMessaging::SPI_SETFOREGROUNDLOCKTIMEOUT; +use windows::Win32::UI::WindowsAndMessaging::SWP_NOMOVE; +use windows::Win32::UI::WindowsAndMessaging::SWP_NOSIZE; +use windows::Win32::UI::WindowsAndMessaging::SWP_SHOWWINDOW; use windows::Win32::UI::WindowsAndMessaging::SW_HIDE; use windows::Win32::UI::WindowsAndMessaging::SW_MAXIMIZE; use windows::Win32::UI::WindowsAndMessaging::SW_MINIMIZE; @@ -368,8 +369,10 @@ impl WindowsApi { unsafe { BringWindowToTop(hwnd) }.process() } + // Raise the window to the top of the Z order, but do not activate or focus + // it. Use raise_and_focus_window to activate and focus a window. pub fn raise_window(hwnd: HWND) -> Result<()> { - let flags = SetWindowPosition::NO_MOVE; + let flags = SetWindowPosition::NO_MOVE | SetWindowPosition::NO_ACTIVATE; let position = HWND_TOP; Self::set_window_pos(hwnd, &Rect::default(), position, flags.bits()) @@ -462,8 +465,31 @@ impl WindowsApi { unsafe { GetForegroundWindow() }.process() } - pub fn set_foreground_window(hwnd: HWND) -> Result<()> { - unsafe { SetForegroundWindow(hwnd) }.ok().process() + pub fn raise_and_focus_window(hwnd: HWND) -> Result<()> { + let event = [INPUT { + r#type: INPUT_MOUSE, + ..Default::default() + }]; + + unsafe { + // Send an input event to our own process first so that we pass the + // foreground lock check + SendInput(&event, size_of::() as i32); + // Error ignored, as the operation is not always necessary. + let _ = SetWindowPos( + hwnd, + HWND_TOP, + 0, + 0, + 0, + 0, + SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW, + ) + .process(); + SetForegroundWindow(hwnd) + } + .ok() + .process() } #[allow(dead_code)] @@ -583,10 +609,6 @@ impl WindowsApi { (process_id, thread_id) } - pub fn current_thread_id() -> u32 { - unsafe { GetCurrentThreadId() } - } - pub fn current_process_id() -> u32 { unsafe { GetCurrentProcessId() } } @@ -604,16 +626,6 @@ impl WindowsApi { } } - pub fn attach_thread_input(thread_id: u32, target_thread_id: u32, attach: bool) -> Result<()> { - unsafe { AttachThreadInput(thread_id, target_thread_id, attach) } - .ok() - .process() - } - - pub fn set_focus(hwnd: HWND) -> Result<()> { - unsafe { SetFocus(hwnd) }.process().map(|_| ()) - } - #[allow(dead_code)] fn set_window_long_ptr_w( hwnd: HWND,