From 7533807071b73a2e2823ff3d9a54a1fd0b413269 Mon Sep 17 00:00:00 2001 From: Logan Nguyen Date: Tue, 26 May 2026 12:59:11 -0500 Subject: [PATCH] fix(screenshot): capture display containing Thuki window on multi-monitor setups Signed-off-by: Logan Nguyen --- src-tauri/src/cg_displays.rs | 76 +++++++++++++++ src-tauri/src/lib.rs | 48 +--------- src-tauri/src/screenshot.rs | 177 ++++++++++++++++++++++++++++++++--- 3 files changed, 243 insertions(+), 58 deletions(-) create mode 100644 src-tauri/src/cg_displays.rs diff --git a/src-tauri/src/cg_displays.rs b/src-tauri/src/cg_displays.rs new file mode 100644 index 00000000..d5ba69df --- /dev/null +++ b/src-tauri/src/cg_displays.rs @@ -0,0 +1,76 @@ +/*! + * CoreGraphics display lookup helpers (macOS). + * + * Wraps `CGGetDisplaysWithPoint` for hit-testing and `CGDisplayBounds` for + * resolving a display's Quartz-coordinate rectangle. All coordinates are in + * the Quartz display coordinate space (top-left of the primary display, + * Y-down), matching the AX API and `CGEventGetLocation`. + * + * Used by the activator to position the overlay on the correct monitor, and + * by the screenshot pipeline to capture the display the user is actually on + * (rather than always capturing the primary display). + */ + +#![cfg(target_os = "macos")] + +use core_graphics::geometry::{CGPoint, CGRect}; + +type CGDirectDisplayID = u32; + +extern "C" { + fn CGGetDisplaysWithPoint( + point: CGPoint, + max_displays: u32, + displays: *mut CGDirectDisplayID, + matching_display_count: *mut u32, + ) -> i32; + fn CGDisplayBounds(display: CGDirectDisplayID) -> CGRect; + fn CGMainDisplayID() -> CGDirectDisplayID; +} + +/// Returns `(origin_x, origin_y, width, height)` in Quartz points for the +/// display containing `(global_x, global_y)`. Returns `None` when the point +/// lies outside every active display. +/// +/// Excluded from coverage: thin wrapper over CoreGraphics FFI that requires a +/// live window server to exercise. +#[cfg_attr(coverage_nightly, coverage(off))] +pub fn display_for_point(global_x: f64, global_y: f64) -> Option<(f64, f64, f64, f64)> { + unsafe { + let point = CGPoint::new(global_x, global_y); + let mut ids = [0u32; 4]; + let mut count: u32 = 0; + let err = CGGetDisplaysWithPoint(point, 4, ids.as_mut_ptr(), &mut count); + if err != 0 || count == 0 { + return None; + } + let r = CGDisplayBounds(ids[0]); + Some((r.origin.x, r.origin.y, r.size.width, r.size.height)) + } +} + +/// Returns `(origin_x, origin_y, width, height)` of the main (menu-bar) display. +/// +/// Excluded from coverage: thin wrapper over CoreGraphics FFI that requires a +/// live window server to exercise. +#[cfg_attr(coverage_nightly, coverage(off))] +pub fn main_display() -> (f64, f64, f64, f64) { + unsafe { + let r = CGDisplayBounds(CGMainDisplayID()); + (r.origin.x, r.origin.y, r.size.width, r.size.height) + } +} + +/// Returns `(origin_x, origin_y, width, height)` in Quartz points for a +/// specific `CGDirectDisplayID`. Used by the screenshot pipeline once the +/// display ID of the window's NSScreen is known. +/// +/// Excluded from coverage: thin wrapper over CoreGraphics FFI that requires a +/// live window server to exercise. +#[cfg_attr(coverage_nightly, coverage(off))] +pub fn bounds_for_display(display_id: u32) -> (f64, f64, f64, f64) { + unsafe { + let r = CGDisplayBounds(display_id); + (r.origin.x, r.origin.y, r.size.width, r.size.height) + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 3f5947d9..e95bb34a 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -33,6 +33,8 @@ pub mod warmup; #[cfg(target_os = "macos")] mod activator; +#[cfg(target_os = "macos")] +mod cg_displays; pub mod context; pub mod permissions; @@ -214,52 +216,6 @@ fn emit_overlay_restore(app_handle: &tauri::AppHandle) { ); } -/// CoreGraphics display lookup - uses macOS-native `CGGetDisplaysWithPoint` -/// for hit-testing instead of manual iteration + containment checks. -/// All coordinates are in the Quartz display coordinate space (top-left of -/// primary display, Y-down), matching the AX API and `CGEventGetLocation`. -#[cfg(target_os = "macos")] -mod cg_displays { - use core_graphics::geometry::{CGPoint, CGRect}; - - type CGDirectDisplayID = u32; - - extern "C" { - fn CGGetDisplaysWithPoint( - point: CGPoint, - max_displays: u32, - displays: *mut CGDirectDisplayID, - matching_display_count: *mut u32, - ) -> i32; - fn CGDisplayBounds(display: CGDirectDisplayID) -> CGRect; - fn CGMainDisplayID() -> CGDirectDisplayID; - } - - fn rect_to_tuple(r: CGRect) -> (f64, f64, f64, f64) { - (r.origin.x, r.origin.y, r.size.width, r.size.height) - } - - /// Returns `(origin_x, origin_y, width, height)` in Quartz points for - /// the display containing `(global_x, global_y)`. - pub fn display_for_point(global_x: f64, global_y: f64) -> Option<(f64, f64, f64, f64)> { - unsafe { - let point = CGPoint::new(global_x, global_y); - let mut ids = [0u32; 4]; - let mut count: u32 = 0; - let err = CGGetDisplaysWithPoint(point, 4, ids.as_mut_ptr(), &mut count); - if err != 0 || count == 0 { - return None; - } - Some(rect_to_tuple(CGDisplayBounds(ids[0]))) - } - } - - /// Returns `(origin_x, origin_y, width, height)` of the main (menu-bar) display. - pub fn main_display() -> (f64, f64, f64, f64) { - unsafe { rect_to_tuple(CGDisplayBounds(CGMainDisplayID())) } - } -} - /// Returns the Quartz-coordinate bounds of the display containing /// `(global_x, global_y)`, falling back to the main display. #[cfg(target_os = "macos")] diff --git a/src-tauri/src/screenshot.rs b/src-tauri/src/screenshot.rs index 99db107f..099ee9f6 100644 --- a/src-tauri/src/screenshot.rs +++ b/src-tauri/src/screenshot.rs @@ -117,6 +117,13 @@ pub async fn capture_screenshot_command( /// effectively excluding Thuki from the screenshot without hiding the window. /// Returns `(width, height, rgba_bytes)` on success. /// +/// `anchor` is a logical-point coordinate (Quartz space) used to pick which +/// display to capture in multi-monitor setups: the display containing the +/// anchor is captured. When `None` or the anchor lies outside every active +/// display, falls back to the main (menu-bar) display. The typical anchor is +/// the center of Thuki's own window, which is the display the user is +/// actually looking at when they invoke `/screen`. +/// /// MUST run on the macOS main thread. CoreGraphics APIs internally dispatch /// to the main thread; calling them from a background thread deadlocks. /// @@ -128,7 +135,7 @@ pub async fn capture_screenshot_command( /// requires Screen Recording permission and a running display server. #[cfg(target_os = "macos")] #[cfg_attr(coverage_nightly, coverage(off))] -fn capture_full_screen_raw() -> Result<(u32, u32, Vec), String> { +fn capture_full_screen_raw(anchor: Option<(f64, f64)>) -> Result<(u32, u32, Vec), String> { use core_foundation::base::TCFType; use core_foundation::string::CFString; use core_graphics::geometry::{CGPoint, CGRect, CGSize}; @@ -173,8 +180,6 @@ fn capture_full_screen_raw() -> Result<(u32, u32, Vec), String> { relativeToWindow: u32, imageOption: u32, ) -> *const c_void; - fn CGMainDisplayID() -> u32; - fn CGDisplayBounds(display: u32) -> CGRect; fn CGImageGetWidth(image: *const c_void) -> usize; fn CGImageGetHeight(image: *const c_void) -> usize; fn CGImageRelease(image: *const c_void); @@ -205,10 +210,25 @@ fn capture_full_screen_raw() -> Result<(u32, u32, Vec), String> { let our_pid = std::process::id() as i32; unsafe { - // Use the actual main display bounds instead of abstract CGRectNull - // or CGRectInfinite, which have platform-dependent representations - // that can cause CGWindowListCreateImage to return null. - let screen_bounds = CGDisplayBounds(CGMainDisplayID()); + // Resolve which display to capture. CGWindowListCreateImage requires a + // concrete CGRect: passing CGRectNull/CGRectInfinite has platform- + // dependent representations that can return null, so we always pass + // the bounds of a specific display. + // + // In multi-monitor setups, capture the display containing the anchor + // point (typically the center of Thuki's own window). This matches the + // monitor the user is actually looking at when they invoke `/screen`. + // If no anchor is provided or it lies outside every active display, + // fall back to the main (menu-bar) display. + let (sb_x, sb_y, sb_w, sb_h) = match anchor { + Some((x, y)) => crate::cg_displays::display_for_point(x, y) + .unwrap_or_else(crate::cg_displays::main_display), + None => crate::cg_displays::main_display(), + }; + let screen_bounds = CGRect { + origin: CGPoint::new(sb_x, sb_y), + size: CGSize::new(sb_w, sb_h), + }; // Two-stage permission check for Screen Recording. // @@ -377,19 +397,81 @@ fn capture_full_screen_raw() -> Result<(u32, u32, Vec), String> { /// main thread because CoreGraphics APIs internally dispatch there and will /// deadlock if called from a background thread. /// +/// `anchor` selects which display to capture in multi-monitor setups. See +/// `capture_full_screen_raw` for the resolution rules. +/// /// Returns `(width, height, rgba_bytes)` on success. #[cfg(target_os = "macos")] #[cfg_attr(coverage_nightly, coverage(off))] -fn capture_full_screen_pixels() -> Result<(u32, u32, Vec), String> { - capture_full_screen_raw() +fn capture_full_screen_pixels(anchor: Option<(f64, f64)>) -> Result<(u32, u32, Vec), String> { + capture_full_screen_raw(anchor) } /// Non-macOS stub: full-screen capture is macOS-only. #[cfg(not(target_os = "macos"))] -fn capture_full_screen_pixels() -> Result<(u32, u32, Vec), String> { +fn capture_full_screen_pixels(_anchor: Option<(f64, f64)>) -> Result<(u32, u32, Vec), String> { Err("full-screen capture is only supported on macOS".to_string()) } +/// Reads the `CGDirectDisplayID` of the `NSScreen` the given `NSWindow` lives +/// on. This is the canonical way to ask "which monitor is this window +/// currently shown on?" on macOS, and it avoids the coordinate-conversion +/// mismatches that arise from manually computing logical points across +/// mixed-DPI multi-monitor setups (e.g. a 2x retina primary + 1x secondary). +/// +/// Uses raw Objective-C runtime messaging so we do not need to enable extra +/// `objc2-app-kit` features. MUST be called on the macOS main thread: AppKit +/// window/screen APIs are main-thread-only. +/// +/// Returns `None` when: +/// - the pointer is null, +/// - the window has no current screen (offscreen / mid-transition), +/// - the device-description dictionary lacks `NSScreenNumber`, or +/// - any runtime message returns nil. +/// +/// Excluded from coverage: pure Objective-C runtime messaging that requires a +/// live window server and a real `NSWindow` instance. +#[cfg(target_os = "macos")] +#[cfg_attr(coverage_nightly, coverage(off))] +unsafe fn nswindow_display_id(ns_window: *mut std::ffi::c_void) -> Option { + use objc2::msg_send; + use objc2::runtime::AnyObject; + use objc2_foundation::NSString; + + if ns_window.is_null() { + return None; + } + let ns_window: *mut AnyObject = ns_window.cast(); + + let ns_screen: *mut AnyObject = msg_send![ns_window, screen]; + if ns_screen.is_null() { + return None; + } + + let device_desc: *mut AnyObject = msg_send![ns_screen, deviceDescription]; + if device_desc.is_null() { + return None; + } + + let key = NSString::from_str("NSScreenNumber"); + let key_ref: *const NSString = &*key; + let value: *mut AnyObject = msg_send![device_desc, objectForKey: key_ref]; + if value.is_null() { + return None; + } + + let display_id: u32 = msg_send![value, unsignedIntValue]; + Some(display_id) +} + +/// Returns the Quartz-coordinate center of a display rectangle expressed as +/// `(origin_x, origin_y, width, height)`. Pure helper, used to derive an +/// anchor point from a known display's bounds. +fn display_bounds_center(bounds: (f64, f64, f64, f64)) -> (f64, f64) { + let (x, y, w, h) = bounds; + (x + w / 2.0, y + h / 2.0) +} + /// Tauri command: silently captures the full screen (excluding Thuki's own /// windows) and returns the absolute file path of the saved image. /// @@ -411,12 +493,35 @@ pub async fn capture_full_screen_command( .app_data_dir() .map_err(|e| format!("failed to resolve app data dir: {e}"))?; + // Resolve the Thuki window so we can ask AppKit which display it lives on. + // The handle is read here (off the main thread) but only dereferenced + // inside the main-thread closure below: AppKit window/screen APIs are + // strictly main-thread-only. + let main_window = app_handle.get_webview_window("main"); + // Phase 1: Capture raw RGBA pixels on the main thread (CoreGraphics // requirement). Returns (width, height, rgba_bytes). + // + // The anchor point steers multi-monitor capture: we look up the + // `CGDirectDisplayID` of the `NSScreen` the Thuki window is on, then take + // the center of that display's bounds. Fallback chain: window missing → + // `ns_window` unavailable → `NSScreen` nil → `None`, which downstream + // resolves to the main (menu-bar) display. let (tx, rx) = tokio::sync::oneshot::channel::), String>>(); app_handle .run_on_main_thread(move || { - tx.send(capture_full_screen_pixels()).ok(); + #[cfg(target_os = "macos")] + let anchor = main_window + .as_ref() + .and_then(|w| w.ns_window().ok()) + .and_then(|p| unsafe { nswindow_display_id(p) }) + .map(|id| display_bounds_center(crate::cg_displays::bounds_for_display(id))); + #[cfg(not(target_os = "macos"))] + let anchor: Option<(f64, f64)> = { + let _ = &main_window; + None + }; + tx.send(capture_full_screen_pixels(anchor)).ok(); }) .map_err(|e| format!("failed to dispatch capture to main thread: {e}"))?; @@ -526,8 +631,56 @@ mod tests { #[cfg(not(target_os = "macos"))] #[test] fn capture_full_screen_returns_err_on_non_macos() { - let result = capture_full_screen_pixels(); + let result = capture_full_screen_pixels(None); assert!(result.is_err()); assert!(result.unwrap_err().contains("only supported on macOS")); } + + #[cfg(not(target_os = "macos"))] + #[test] + fn capture_full_screen_returns_err_on_non_macos_with_anchor() { + let result = capture_full_screen_pixels(Some((100.0, 100.0))); + assert!(result.is_err()); + } + + #[test] + fn display_bounds_center_returns_midpoint_for_primary_display() { + // Primary display at origin (0, 0), 1920x1080: center is (960, 540). + assert_eq!( + display_bounds_center((0.0, 0.0, 1920.0, 1080.0)), + (960.0, 540.0) + ); + } + + #[test] + fn display_bounds_center_returns_midpoint_for_offset_display() { + // Secondary display at (1920, 0), 1920x1080: center is (2880, 540). + // This is the case that the multi-monitor fix targets: anchoring on + // a non-primary display so the screen capture picks the right one. + assert_eq!( + display_bounds_center((1920.0, 0.0, 1920.0, 1080.0)), + (2880.0, 540.0) + ); + } + + #[test] + fn display_bounds_center_handles_negative_origin() { + // Display positioned left of the primary has a negative origin in + // Quartz coordinates (origin at primary's top-left). + assert_eq!( + display_bounds_center((-1280.0, 0.0, 1280.0, 720.0)), + (-640.0, 360.0) + ); + } + + #[test] + fn display_bounds_center_handles_zero_size() { + // Defensive: a zero-sized rect collapses to its origin. We never + // expect to see this in practice (CGDisplayBounds returns a real + // rect), but the helper is pure and must not panic. + assert_eq!( + display_bounds_center((100.0, 200.0, 0.0, 0.0)), + (100.0, 200.0) + ); + } }