From cb569b8926e661987afe14175aae2b12c22b6573 Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:08:24 +0000 Subject: [PATCH 1/2] fix(scap-targets): keep partially visible X11 windows selectable --- crates/scap-targets/src/platform/linux.rs | 99 ++++++++++++++++-- crates/scap-targets/tests/linux_targets.rs | 111 +++++++++++++++++++++ 2 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 crates/scap-targets/tests/linux_targets.rs diff --git a/crates/scap-targets/src/platform/linux.rs b/crates/scap-targets/src/platform/linux.rs index 1dbb6c3b966..510a37cbf02 100644 --- a/crates/scap-targets/src/platform/linux.rs +++ b/crates/scap-targets/src/platform/linux.rs @@ -294,17 +294,7 @@ impl WindowImpl { } pub fn display(&self) -> Option { - let bounds = self.physical_bounds()?; - let center = PhysicalPosition::new( - bounds.position().x() + bounds.size().width() / 2.0, - bounds.position().y() + bounds.size().height() / 2.0, - ); - - DisplayImpl::list().into_iter().find(|display| { - display - .physical_bounds() - .is_some_and(|display_bounds| display_bounds.contains_point(center)) - }) + display_for_bounds(self.physical_bounds()?, &DisplayImpl::list()) } pub fn name(&self) -> Option { @@ -322,6 +312,24 @@ impl WindowImpl { } } +fn display_for_bounds(bounds: PhysicalBounds, displays: &[DisplayImpl]) -> Option { + displays + .iter() + .filter_map(|display| { + let display_bounds = display.physical_bounds()?; + let left = bounds.position().x().max(display_bounds.position().x()); + let top = bounds.position().y().max(display_bounds.position().y()); + let right = (bounds.position().x() + bounds.size().width()) + .min(display_bounds.position().x() + display_bounds.size().width()); + let bottom = (bounds.position().y() + bounds.size().height()) + .min(display_bounds.position().y() + display_bounds.size().height()); + let area = (right - left).max(0.0) * (bottom - top).max(0.0); + (area > 0.0).then_some((*display, area)) + }) + .max_by(|(_, a), (_, b)| a.total_cmp(b)) + .map(|(display, _)| display) +} + fn filter_window_candidates(windows: Vec) -> Vec { windows .into_iter() @@ -491,3 +499,72 @@ fn get_cursor_position() -> Option { reply.root_y.into(), )) } + +#[cfg(test)] +mod tests { + use super::*; + + fn display(id: u32, x: i32, y: i32, width: u32, height: u32) -> DisplayImpl { + DisplayImpl { + id, + x, + y, + width, + height, + refresh_rate: 60.0, + logical_bounds: None, + wayland_uuid: None, + } + } + + fn bounds(x: f64, y: f64, width: f64, height: f64) -> PhysicalBounds { + PhysicalBounds::new( + PhysicalPosition::new(x, y), + PhysicalSize::new(width, height), + ) + } + + #[test] + fn window_center_in_monitor_gap_still_resolves_to_visible_display() { + let displays = [display(0, 0, 0, 1000, 800), display(1, 1200, 0, 1000, 800)]; + let window = bounds(900.0, 100.0, 350.0, 200.0); + assert_eq!(display_for_bounds(window, &displays).unwrap().id, 0); + } + + #[test] + fn window_uses_largest_overlap_on_offset_monitors() { + let displays = [ + display(0, -1920, 0, 1920, 1080), + display(1, 0, -900, 1440, 900), + ]; + let window = bounds(-150.0, -100.0, 400.0, 300.0); + assert_eq!(display_for_bounds(window, &displays).unwrap().id, 0); + assert_eq!( + display_for_bounds(window, &[displays[1], displays[0]]) + .unwrap() + .id, + 0 + ); + assert_eq!( + display_for_bounds(bounds(-150.0, -200.0, 400.0, 300.0), &displays) + .unwrap() + .id, + 1 + ); + } + + #[test] + fn nonoverlapping_and_edge_touching_windows_have_no_display() { + let displays = [display(0, 0, 0, 1920, 1080)]; + for window in [ + bounds(-320.0, 100.0, 320.0, 240.0), + bounds(1920.0, 100.0, 320.0, 240.0), + bounds(100.0, -240.0, 320.0, 240.0), + bounds(100.0, 1080.0, 320.0, 240.0), + bounds(-400.0, -400.0, 320.0, 240.0), + ] { + assert!(display_for_bounds(window, &displays).is_none()); + } + assert!(display_for_bounds(bounds(0.0, 0.0, 320.0, 240.0), &[]).is_none()); + } +} diff --git a/crates/scap-targets/tests/linux_targets.rs b/crates/scap-targets/tests/linux_targets.rs new file mode 100644 index 00000000000..0d099bb287c --- /dev/null +++ b/crates/scap-targets/tests/linux_targets.rs @@ -0,0 +1,111 @@ +#![cfg(target_os = "linux")] + +use scap_targets::{Display, Window, WindowId}; +use x11rb::{ + COPY_DEPTH_FROM_PARENT, + connection::Connection, + protocol::xproto::{ + AtomEnum, ConfigureWindowAux, ConnectionExt as _, CreateWindowAux, PropMode, WindowClass, + }, + wrapper::ConnectionExt as _, +}; + +#[test] +#[ignore = "requires an isolated X11 server, run with xvfb-run"] +fn partially_offscreen_window_remains_a_recording_target() { + let (conn, screen_num) = x11rb::connect(None).unwrap(); + let screen = &conn.setup().roots[screen_num]; + let id = conn.generate_id().unwrap(); + conn.create_window( + COPY_DEPTH_FROM_PARENT, + id, + screen.root, + 100, + 100, + 320, + 240, + 0, + WindowClass::INPUT_OUTPUT, + 0, + &CreateWindowAux::new(), + ) + .unwrap() + .check() + .unwrap(); + conn.change_property8( + PropMode::REPLACE, + id, + AtomEnum::WM_NAME, + AtomEnum::STRING, + b"Cap offscreen target regression", + ) + .unwrap() + .check() + .unwrap(); + conn.change_property8( + PropMode::REPLACE, + id, + AtomEnum::WM_CLASS, + AtomEnum::STRING, + b"cap-target-fixture\0CapTargetFixture\0", + ) + .unwrap() + .check() + .unwrap(); + conn.map_window(id).unwrap().check().unwrap(); + + let target_id: WindowId = id.to_string().parse().unwrap(); + let display_id = Display::primary().id(); + let width = i32::from(screen.width_in_pixels); + let height = i32::from(screen.height_in_pixels); + for (x, y) in [ + (100, 100), + (-250, 100), + (width - 70, 100), + (100, -200), + (100, height - 40), + (-250, -200), + ] { + conn.configure_window(id, &ConfigureWindowAux::new().x(x).y(y)) + .unwrap() + .check() + .unwrap(); + let target = Window::from_id(&target_id) + .unwrap_or_else(|| panic!("visible window at ({x}, {y}) must remain selectable")); + assert_eq!(target.owner_name().as_deref(), Some("cap-target-fixture")); + assert_eq!(target.display().unwrap().id(), display_id); + let bounds = target.display_relative_logical_bounds().unwrap(); + assert_eq!(bounds.position().x(), f64::from(x)); + assert_eq!(bounds.position().y(), f64::from(y)); + assert_eq!(bounds.size().width(), 320.0); + assert_eq!(bounds.size().height(), 240.0); + conn.warp_pointer( + x11rb::NONE, + screen.root, + 0, + 0, + 0, + 0, + i16::try_from(x.max(0) + 10).unwrap(), + i16::try_from(y.max(0) + 10).unwrap(), + ) + .unwrap() + .check() + .unwrap(); + assert_eq!(Window::get_topmost_at_cursor().unwrap().id(), target_id); + } + + conn.configure_window(id, &ConfigureWindowAux::new().x(-320).y(100)) + .unwrap() + .check() + .unwrap(); + assert!(Window::from_id(&target_id).is_none()); + + conn.configure_window(id, &ConfigureWindowAux::new().x(100).y(100)) + .unwrap() + .check() + .unwrap(); + assert!(Window::from_id(&target_id).is_some()); + conn.destroy_window(id).unwrap().check().unwrap(); + assert!(Window::from_id(&target_id).is_none()); +} From 09a22a2dbfe8a7704aeb2e54daf5202ffe60959a Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Thu, 22 Oct 2026 08:39:25 +0000 Subject: [PATCH 2/2] test(scap-targets): run X11 target regression in Linux CI --- .github/workflows/ci.yml | 1 + crates/scap-targets/tests/linux_targets.rs | 59 +++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 57810c00587..84d9ad4e343 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -282,6 +282,7 @@ jobs: cargo test --locked -p cap --bin cap record::tests cargo test --locked -p cap-recording --lib -- --test-threads=1 cargo test --locked -p cap-recording --test x11_shm_capture -- --test-threads=1 + cargo test --locked -p scap-targets --lib --test linux_targets cargo test --locked -p cap-enc-ffmpeg build-gpui: diff --git a/crates/scap-targets/tests/linux_targets.rs b/crates/scap-targets/tests/linux_targets.rs index 0d099bb287c..73327eebfaa 100644 --- a/crates/scap-targets/tests/linux_targets.rs +++ b/crates/scap-targets/tests/linux_targets.rs @@ -1,5 +1,10 @@ #![cfg(target_os = "linux")] +use std::{ + io::{BufRead, BufReader}, + process::{Child, Command, Stdio}, +}; + use scap_targets::{Display, Window, WindowId}; use x11rb::{ COPY_DEPTH_FROM_PARENT, @@ -10,10 +15,60 @@ use x11rb::{ wrapper::ConnectionExt as _, }; +struct Xvfb(Child); + +impl Xvfb { + fn start() -> Option<(Self, String)> { + let (reader, writer) = std::io::pipe().ok()?; + let child = Command::new("Xvfb") + .args([ + "-displayfd", + "1", + "-screen", + "0", + "1280x720x24", + "-nolisten", + "tcp", + ]) + .stdin(Stdio::null()) + .stdout(writer) + .stderr(Stdio::null()) + .spawn(); + let child = match child { + Ok(child) => child, + Err(error) => { + eprintln!("skipping X11 targets test: cannot start Xvfb: {error}"); + return None; + } + }; + let server = Self(child); + let mut line = String::new(); + BufReader::new(reader) + .read_line(&mut line) + .expect("Xvfb display number"); + let number: u32 = line.trim().parse().expect("Xvfb display number"); + Some((server, format!(":{number}"))) + } +} + +impl Drop for Xvfb { + fn drop(&mut self) { + let _ = self.0.kill(); + let _ = self.0.wait(); + } +} + #[test] -#[ignore = "requires an isolated X11 server, run with xvfb-run"] fn partially_offscreen_window_remains_a_recording_target() { - let (conn, screen_num) = x11rb::connect(None).unwrap(); + let Some((_server, display)) = Xvfb::start() else { + return; + }; + // SAFETY: this binary has one test, which sets the variables before calling scap_targets. + unsafe { + std::env::set_var("DISPLAY", &display); + std::env::set_var("XDG_SESSION_TYPE", "x11"); + } + let (conn, screen_num) = x11rb::connect(Some(&display)).unwrap(); let screen = &conn.setup().roots[screen_num]; let id = conn.generate_id().unwrap(); conn.create_window(