diff --git a/crates/warpui/Cargo.toml b/crates/warpui/Cargo.toml index 81d36a6e..6655ffec 100644 --- a/crates/warpui/Cargo.toml +++ b/crates/warpui/Cargo.toml @@ -148,7 +148,9 @@ winreg.workspace = true windows = { workspace = true, features = [ "Win32_Graphics_Dwm", "Win32_Graphics_DirectWrite", + "Win32_Graphics_Gdi", "Win32_UI_Shell", + "Win32_UI_WindowsAndMessaging", "Win32_Networking", "Win32_System_Com", "Win32_Networking_NetworkListManager", diff --git a/crates/warpui/src/windowing/winit/window.rs b/crates/warpui/src/windowing/winit/window.rs index 733291e6..d3320d1c 100644 --- a/crates/warpui/src/windowing/winit/window.rs +++ b/crates/warpui/src/windowing/winit/window.rs @@ -1159,15 +1159,13 @@ impl Window { pub fn focus(&self) { if let Some(Inner { window, level, .. }) = self.inner.borrow().as_ref() { // Winit is a bit quirky here. Trying to focus a window which isn't visible will not - // make it visible. So, call `focus_window` if the window is visible, otherwise make it - // visible. + // make it visible. So, make it visible first if needed, then explicitly focus it. if window.is_visible().unwrap_or(true) { window.set_minimized(false); - window.focus_window(); } else { - // Setting visible to `true` will also focus it. window.set_visible(true); } + window.focus_window(); window.set_window_level(*level); } } diff --git a/crates/warpui/src/windowing/winit/window/windows_wm.rs b/crates/warpui/src/windowing/winit/window/windows_wm.rs index 213ebeb9..93019297 100644 --- a/crates/warpui/src/windowing/winit/window/windows_wm.rs +++ b/crates/warpui/src/windowing/winit/window/windows_wm.rs @@ -9,9 +9,14 @@ use winit::monitor::MonitorHandle; use winit::platform::windows::MonitorHandleExtWindows; use winit::window::Window as WinitWindow; +use windows::Win32::Graphics::Gdi::{MonitorFromWindow, MONITOR_DEFAULTTONEAREST}; +use windows::Win32::UI::WindowsAndMessaging::GetForegroundWindow; + use super::get_monitor_logical_bounds; impl WindowManager { + /// Returns the active Warp window. This will return an error if a different app's window is + /// active. fn get_active_window_handle(&self) -> Result> { let window_id = &self .active_window_id() @@ -27,11 +32,42 @@ impl WindowManager { Ok(winit_window_ref.window.clone()) } - fn get_current_monitor_handle(&self) -> Result { - let winit_window_ref = self.get_active_window_handle()?; - winit_window_ref - .current_monitor() - .ok_or(anyhow::anyhow!("Unable to get current monitor")) + fn get_any_window_handle(&self) -> Result> { + self.windows + .values() + .find_map(|window| { + window + .inner + .try_borrow() + .ok() + .and_then(|borrow| borrow.as_ref().map(|inner| inner.window.clone())) + }) + .ok_or_else(|| anyhow::anyhow!("No window handles available")) + } + + /// Returns the monitor which contains the focused window ("key window" in MacOS parlance). It's + /// the window that receives and handles the keypress events. + fn get_foreground_monitor(&self) -> Result { + let any_window = self.get_any_window_handle()?; + + // Even if no window has foreground focus, MonitorFromWindow with + // MONITOR_DEFAULTTONEAREST will return the nearest/primary monitor. + let fg_hwnd = unsafe { GetForegroundWindow() }; + let target_hmonitor = unsafe { MonitorFromWindow(fg_hwnd, MONITOR_DEFAULTTONEAREST) }; + + any_window + .available_monitors() + .find(|monitor| monitor.hmonitor() == target_hmonitor.0 as isize) + .ok_or_else(|| anyhow::anyhow!("Could not match foreground window's monitor")) + } + + fn get_active_monitor(&self) -> Result { + self.get_active_window_handle() + .and_then(|w| { + w.current_monitor() + .ok_or_else(|| anyhow::anyhow!("Unable to get current monitor")) + }) + .or_else(|_| self.get_foreground_monitor()) } pub(super) fn get_monitor_bounds_for_display_idx(&self, idx: DisplayIdx) -> Result { @@ -57,30 +93,30 @@ impl WindowManager { } fn get_primary_monitor_handle(&self) -> Result { - let winit_window_ref = self.get_active_window_handle()?; + let winit_window_ref = self.get_any_window_handle()?; winit_window_ref .primary_monitor() .ok_or(anyhow::anyhow!("No primary monitor found")) } pub(super) fn get_current_monitor_id(&self) -> Result { - let active_monitor = self.get_current_monitor_handle()?; + let active_monitor = self.get_active_monitor()?; let active_monitor_id = active_monitor.hmonitor(); Ok(DisplayId::from(active_monitor_id as usize)) } fn get_available_monitors(&self) -> Result> { - let winit_window_ref = self.get_active_window_handle()?; + let winit_window_ref = self.get_any_window_handle()?; Ok(winit_window_ref.available_monitors().collect_vec()) } pub(super) fn get_available_monitor_count(&self) -> Result { - let winit_window_ref = self.get_active_window_handle()?; + let winit_window_ref = self.get_any_window_handle()?; Ok(winit_window_ref.available_monitors().count()) } pub(super) fn get_active_monitor_logical_bounds(&self) -> Result { - let active_monitor = self.get_current_monitor_handle()?; + let active_monitor = self.get_active_monitor()?; Ok(get_monitor_logical_bounds(&active_monitor)) } } diff --git a/specs/CODE-1787/PRODUCT.md b/specs/CODE-1787/PRODUCT.md new file mode 100644 index 00000000..6fbcf491 --- /dev/null +++ b/specs/CODE-1787/PRODUCT.md @@ -0,0 +1,19 @@ +# Windows Quake Mode: Focus and Sizing Fix — Product Spec +Linear: CODE-1787 + +## Summary +When the quake mode hotkey is pressed while a non-Warp application has focus on Windows, the quake window should appear with correct size and receive keyboard focus. Currently, the window appears but does not receive focus, and its size falls back to a hardcoded default instead of matching the configured screen percentage. + +## Behavior + +1. Pressing the quake mode hotkey while a non-Warp application has foreground focus must show the quake window and transfer keyboard focus to it. The previously focused application must lose foreground focus. + +2. Pressing the quake mode hotkey while a Warp window (non-quake) has focus must continue to show the quake window with correct focus, as it does today. + +3. When the quake window is shown from a hidden state, its size must match the configured width and height percentages of the display, regardless of whether Warp or another application had focus before the hotkey was pressed. + +4. The quake window must appear on the monitor that contains the application which had keyboard focus when the hotkey was pressed — not necessarily the monitor the cursor is on, and not a hardcoded fallback. + +5. On single-monitor setups, invariants 3 and 4 reduce to: the quake window always appears at the correct configured size on the only display. + +6. These invariants apply on Windows only. macOS quake mode behavior is unchanged. diff --git a/specs/CODE-1787/TECH.md b/specs/CODE-1787/TECH.md new file mode 100644 index 00000000..86e7e736 --- /dev/null +++ b/specs/CODE-1787/TECH.md @@ -0,0 +1,55 @@ +# Windows Quake Mode: Focus and Sizing Fix — Tech Spec +Product spec: `specs/CODE-1787/PRODUCT.md` + +## Context +Two independent bugs prevent quake mode from working correctly on Windows when triggered while a non-Warp application has foreground focus. + +### Bug 1: focus not transferred +`WinitWindow::focus()` in `crates/warpui/src/windowing/winit/window.rs:1080` had two branches: if the window was already visible it called `focus_window()`, otherwise it called `set_visible(true)` and relied on visibility implying focus. On Windows, `set_visible(true)` does not steal foreground focus from another application — an explicit `SetForegroundWindow` (via winit's `focus_window()`) is required. The quake window was hidden via `set_visible(false)`, so re-showing it always took the `set_visible(true)` branch and never called `focus_window()`. + +### Bug 2: incorrect window size +All Windows monitor queries in `crates/warpui/src/windowing/winit/window/windows_wm.rs` routed through `get_active_window_handle()`, which requires a focused + visible Warp window. When no Warp window has focus, this fails and `active_display_bounds()` falls back to a hardcoded `DEFAULT_WINDOW_SIZE` (1280×800). The quake window is then sized as a percentage of that default instead of the actual display dimensions. + +### Relevant code +- `crates/warpui/src/windowing/winit/window.rs:1080-1092` — `WinitWindow::focus()` +- `crates/warpui/src/windowing/winit/window/windows_wm.rs` — all Windows monitor query methods +- `crates/warpui/src/windowing/winit/window.rs:222-227` — `WindowManager::show_window_and_focus_app` (calls `focus()`) +- `app/src/root_view.rs:1481-1507` — quake mode toggle, hidden→visible branch + +## Proposed changes + +### 1. Always call `focus_window()` in `WinitWindow::focus()` +Restructure `focus()` so `focus_window()` is called unconditionally after the window is made visible or un-minimized. The previous code only called it in the already-visible branch. + +Before: +``` +if visible → set_minimized(false); focus_window() +else → set_visible(true) // hoped this would also focus +``` + +After: +``` +if visible → set_minimized(false) +else → set_visible(true) +focus_window() // always, regardless of prior visibility +``` + +This fixes Behavior 1 and 2. + +### 2. Decouple monitor queries from active-window requirement +Split the Windows monitor methods into two categories: + +**Global queries** (don't care which monitor): `get_primary_monitor_handle`, `get_available_monitors`, `get_available_monitor_count`. These only need *any* winit window handle to access platform APIs. Add `get_any_window_handle()` which returns the first available window regardless of focus, and call it directly from these methods. + +**Active-monitor queries** (need to know which monitor the user is on): `get_active_monitor`, `get_current_monitor_id`, `get_active_monitor_logical_bounds`. When a Warp window has focus, use its `current_monitor()`. When no Warp window has focus, fall back to `get_foreground_monitor()`, which uses Win32 `GetForegroundWindow` + `MonitorFromWindow(hwnd, MONITOR_DEFAULTTONEAREST)` to find the monitor of the app that has keyboard focus. This is the window that triggered the global hotkey. + +This fixes Behavior 3 and 4. The foreground-window approach is preferred over cursor position because the cursor may be on a different monitor than the window handling keypress events. + +### 3. Add Win32 feature dependencies +Enable `Win32_Graphics_Gdi` (for `MonitorFromWindow`, `MONITOR_DEFAULTTONEAREST`) and `Win32_UI_WindowsAndMessaging` (for `GetForegroundWindow`) in `crates/warpui/Cargo.toml`. + +## Testing and validation +- Manual: configure quake mode with 100% width, focus a non-Warp app, press the hotkey. Verify the quake window receives focus and spans the full display width. (Behavior 1, 3) +- Manual: with a Warp window focused, press the hotkey. Verify existing behavior is preserved. (Behavior 2) +- Manual (multi-monitor): focus an app on monitor B, press the hotkey. Verify the quake window appears on monitor B at the correct size. (Behavior 4) +- Manual: verify macOS quake mode is unaffected — changes are behind `#[cfg(windows)]` and winit-only code paths. (Behavior 6)