From 7cf86c0b9a872a2f41284069815b1a79ab541eb6 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:05:52 +0200 Subject: [PATCH 1/2] fix(freeze): support portable capture backends --- Cargo.lock | 1 + Cargo.toml | 5 +- README.md | 2 +- docs/SETUP.md | 2 +- docs/codebase-overview.md | 2 + .../wayland/backend/event_loop/capture.rs | 32 +- src/backend/wayland/backend/event_loop/mod.rs | 2 +- src/backend/wayland/backend/setup.rs | 32 +- src/backend/wayland/backend/state_init/mod.rs | 9 +- src/backend/wayland/frozen/capture.rs | 272 ++++++--- src/backend/wayland/frozen/ext_image_copy.rs | 528 ++++++++++++++++++ src/backend/wayland/frozen/image.rs | 103 +++- src/backend/wayland/frozen/mod.rs | 5 +- src/backend/wayland/frozen/portal.rs | 53 +- src/backend/wayland/frozen/state.rs | 320 ++++++++++- .../wayland/handlers/ext_image_copy.rs | 92 +++ src/backend/wayland/handlers/mod.rs | 1 + src/backend/wayland/handlers/screencopy.rs | 12 +- src/backend/wayland/portal_capture.rs | 12 +- src/backend/wayland/state.rs | 4 +- src/backend/wayland/state/capture.rs | 15 + src/backend/wayland/state/capture/barrier.rs | 12 +- src/backend/wayland/state/core/init.rs | 9 +- src/backend/wayland/zoom/mod.rs | 6 +- src/backend/wayland/zoom/portal.rs | 17 +- src/capture/portal.rs | 202 ++++++- src/capture/sources/portal.rs | 2 +- src/input/state/core/base/types.rs | 9 +- src/input/state/core/base/types/tests.rs | 20 + tools/check-nixpkgs-recipe.py | 1 + 30 files changed, 1599 insertions(+), 183 deletions(-) create mode 100644 src/backend/wayland/frozen/ext_image_copy.rs create mode 100644 src/backend/wayland/handlers/ext_image_copy.rs diff --git a/Cargo.lock b/Cargo.lock index 195cf000..2a44f864 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3230,6 +3230,7 @@ dependencies = [ "anyhow", "cairo-rs", "flate2", + "getrandom 0.3.4", "glib", "gtk4", "gtk4-layer-shell", diff --git a/Cargo.toml b/Cargo.toml index 9d94d4ab..ca972771 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ workspace = true [dependencies] # Wayland wayland-client = "0.31" -wayland-protocols = { version = "0.32", features = ["client", "unstable"] } +wayland-protocols = { version = "0.32", features = ["client", "staging", "unstable"] } wayland-protocols-wlr = { version = "0.3", features = ["client"] } smithay-client-toolkit = { version = "0.20", default-features = false, features = ["calloop", "xkbcommon"] } @@ -53,6 +53,7 @@ tokio = { version = "1.0", features = ["rt-multi-thread", "macros", "time", "syn # Screenshot capture zbus = { version = "5.0", optional = true, default-features = false, features = ["tokio"] } +getrandom = { version = "0.3", optional = true } serde_json = "1.0" png = "0.18" @@ -72,7 +73,7 @@ xkbcommon = { version = "0.8", optional = true } tablet-input = [] # D-Bus dependent functionality (portal capture, notifications, tray) dbus = ["zbus"] -portal = ["dbus"] +portal = ["dbus", "dep:getrandom"] tray = ["dbus", "ksni"] config-schema = ["dep:schemars"] # GTK4-rendered toolbars on layer-shell compositors; the built-in Cairo diff --git a/README.md b/README.md index 7ef21bcd..865a020d 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ The v0.9.23+ prebuilt `wayscriber` packages require glibc 2.39 and GTK 4.12 — - Presenter mode (Ctrl+Shift+M): hides UI, forces click highlights - Input HUD (Ctrl+Shift+K): on-screen keystroke and click chips for demos and screencasts (opt-in system-wide capture via the `input-monitor` build feature — see [docs/CONFIG.md](docs/CONFIG.md#uiinput_hud---input-hud-keystrokes-and-clicks)) - Light passthrough (layer-shell): draw while input passes through to the app underneath — see [Light passthrough mode](#light-passthrough-mode) -- Screen freeze (Ctrl+Shift+F): pause the display while apps keep running. On GNOME, this uses the screenshot portal when available +- Screen freeze (Ctrl+Shift+F): pause the display while apps keep running. Freeze prefers compositor-native `wlr-screencopy` or `ext-image-copy-capture` and falls back to the screenshot portal when available - Spotlight: drag an ellipse to dim everything around it; stack several to highlight multiple areas. Dim strength and edge softness are configurable under `[spotlight]` ### Callouts and zoom diff --git a/docs/SETUP.md b/docs/SETUP.md index b5e7a41a..2a2d01f3 100644 --- a/docs/SETUP.md +++ b/docs/SETUP.md @@ -181,7 +181,7 @@ Then use the configurator's Daemon tab, or create a GNOME custom shortcut that r wayscriber --daemon-toggle ``` -Freeze works on GNOME when the screenshot portal is available and responsive; the first use may show a desktop permission prompt. Portal capture can be slower than compositor screencopy, and mixed-DPI or multi-monitor setups may depend on client-side crop behavior. +Freeze prefers compositor-native `wlr-screencopy` or `ext-image-copy-capture` when either protocol is available, then falls back to the screenshot portal. On GNOME, Freeze works when that portal is available and responsive; the first use may show a desktop permission prompt. Portal capture can be slower than direct compositor capture, and mixed-DPI or multi-monitor setups may depend on client-side crop behavior. Light passthrough mode is not available in the regular app on stock GNOME Wayland. GNOME's xdg-shell fallback does not expose the shell-level overlay behavior needed to keep annotations visible while input goes to apps underneath, so `--light-toggle` is intentionally disabled instead of pretending to pass input through. A GNOME Shell extension companion would be the real path for that workflow. diff --git a/docs/codebase-overview.md b/docs/codebase-overview.md index 819f56f3..e0b07d85 100644 --- a/docs/codebase-overview.md +++ b/docs/codebase-overview.md @@ -76,6 +76,8 @@ Daemon mode therefore provides a persistent background service that reacts to us `WaylandState` centralizes everything the handlers need: current buffers, Cairo context, mouse positions, capture state, and tokio handle for async work. +Freeze capture waits for the overlay-suppression frame, then selects `wlr-screencopy`, `ext-image-copy-capture`, or the screenshot portal in that order. The two direct protocols capture the active output into shared memory; the portal captures the desktop and the client crops the selected output when needed. + --- ## 4. Input Handling & Drawing State diff --git a/src/backend/wayland/backend/event_loop/capture.rs b/src/backend/wayland/backend/event_loop/capture.rs index 229a1884..d371f9bd 100644 --- a/src/backend/wayland/backend/event_loop/capture.rs +++ b/src/backend/wayland/backend/event_loop/capture.rs @@ -23,18 +23,29 @@ pub(super) fn poll_portal_captures(state: &mut WaylandState, now: Instant) { state.apply_capture_completion(); } -pub(super) fn poll_capture_deadlines(state: &mut WaylandState, now: Instant) { +pub(super) fn poll_capture_deadlines( + state: &mut WaylandState, + qh: &wayland_client::QueueHandle, + now: Instant, +) { state.poll_overlay_capture_barrier_timeout(now); + if let Some(backend) = state.frozen.take_timed_out_direct_capture(now) { + warn!("{backend:?} frozen capture timed out; trying the next backend"); + state.continue_frozen_capture_after_failure(backend, qh); + } } pub(super) fn capture_timeout(state: &WaylandState, now: Instant) -> Option { super::min_timeout( state.overlay_capture_barrier_timeout(now), super::min_timeout( - state.frozen.portal_timeout(now), + state.frozen.direct_capture_timeout(now), super::min_timeout( - state.zoom.portal_timeout(now), - state.xdg_frozen_fullscreen_timeout(now), + state.frozen.portal_timeout(now), + super::min_timeout( + state.zoom.portal_timeout(now), + state.xdg_frozen_fullscreen_timeout(now), + ), ), ), ) @@ -146,7 +157,7 @@ fn handle_frozen_toggle(state: &mut WaylandState) { if !state.frozen_enabled() { warn!( - "Frozen mode unavailable: no screencopy backend and no screenshot portal backend; ignoring toggle" + "Frozen mode unavailable: no direct capture backend and no screenshot portal backend; ignoring toggle" ); state.input_state.push_toast( ToastPriority::Info, @@ -159,12 +170,6 @@ fn handle_frozen_toggle(state: &mut WaylandState) { state.restore_xdg_after_frozen(); state.frozen.unfreeze(&mut state.input_state); } else { - let use_fallback = !state.frozen.manager_available(); - if use_fallback { - warn!("Frozen mode: screencopy unavailable, using portal fallback"); - } else { - info!("Frozen mode: using screencopy fast path"); - } if !state.enter_overlay_suppression(OverlaySuppression::Frozen) { warn!("Frozen mode requested while overlay is suppressed; ignoring toggle"); state.input_state.push_toast( @@ -174,10 +179,7 @@ fn handle_frozen_toggle(state: &mut WaylandState) { ); return; } - if let Err(err) = state - .frozen - .start_capture(use_fallback, &state.tokio_handle) - { + if let Err(err) = state.frozen.start_capture() { warn!("Frozen capture failed to start: {}", err); state.exit_overlay_suppression(OverlaySuppression::Frozen); state.frozen.cancel(&mut state.input_state); diff --git a/src/backend/wayland/backend/event_loop/mod.rs b/src/backend/wayland/backend/event_loop/mod.rs index 548994a5..7675818e 100644 --- a/src/backend/wayland/backend/event_loop/mod.rs +++ b/src/backend/wayland/backend/event_loop/mod.rs @@ -182,7 +182,7 @@ pub(super) fn run_event_loop( // A capture-barrier deadline may be what woke dispatch. Apply its // recovery before this iteration reaches toolbar synchronization and // rendering so the restored frame is not delayed by another block. - capture::poll_capture_deadlines(state, Instant::now()); + capture::poll_capture_deadlines(state, qh, Instant::now()); if !state.input_state.should_exit { state.reconcile_live_source_interaction_if_idle( diff --git a/src/backend/wayland/backend/setup.rs b/src/backend/wayland/backend/setup.rs index 94f78005..5a6d8aa8 100644 --- a/src/backend/wayland/backend/setup.rs +++ b/src/backend/wayland/backend/setup.rs @@ -14,12 +14,19 @@ use smithay_client_toolkit::{ shm::Shm, }; use wayland_client::{Connection, EventQueue, globals::registry_queue_init}; +use wayland_protocols::ext::{ + image_capture_source::v1::client::ext_output_image_capture_source_manager_v1::ExtOutputImageCaptureSourceManagerV1, + image_copy_capture::v1::client::ext_image_copy_capture_manager_v1::ExtImageCopyCaptureManagerV1, +}; use wayland_protocols::wp::text_input::zv3::client::zwp_text_input_manager_v3::ZwpTextInputManagerV3; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; use crate::env_vars::{XDG_CURRENT_DESKTOP_ENV, XDG_SESSION_DESKTOP_ENV}; -use super::super::state::{WaylandGlobals, WaylandState}; +use super::super::{ + frozen::ExtImageCopyManagers, + state::{WaylandGlobals, WaylandState}, +}; // Freeze/zoom capture currently consumes wl_shm buffer events and ignores linux-dmabuf. // Version 3 can negotiate linux-dmabuf-only frames on newer wlroots/NVIDIA stacks, so @@ -34,6 +41,7 @@ pub(super) struct WaylandSetup { pub(super) qh: wayland_client::QueueHandle, pub(super) state_globals: WaylandGlobals, pub(super) screencopy_manager: Option, + pub(super) ext_image_copy_managers: Option, pub(super) text_input_manager: Option, pub(super) layer_shell_available: bool, } @@ -135,6 +143,27 @@ pub(super) fn setup_wayland() -> Result { } }; + let ext_image_copy_manager = globals + .bind::(&qh, 1..=1, ()) + .ok(); + let ext_output_source_manager = globals + .bind::(&qh, 1..=1, ()) + .ok(); + let ext_image_copy_managers = match (ext_image_copy_manager, ext_output_source_manager) { + (Some(capture), Some(output_source)) => { + debug!("Bound ext-image-copy-capture output backend"); + Some(ExtImageCopyManagers::new(capture, output_source)) + } + (capture, output_source) => { + debug!( + "ext-image-copy-capture output backend unavailable: capture_manager={}, output_source_manager={}", + capture.is_some(), + output_source.is_some() + ); + None + } + }; + // IME / text-input-v3 for the text and sticky-note tools. Optional: when // the compositor lacks it, editing falls back to the raw keysym path // (single-key characters only). @@ -175,6 +204,7 @@ pub(super) fn setup_wayland() -> Result { qh, state_globals, screencopy_manager, + ext_image_copy_managers, text_input_manager, layer_shell_available, }) diff --git a/src/backend/wayland/backend/state_init/mod.rs b/src/backend/wayland/backend/state_init/mod.rs index ec2df9e0..412fee6a 100644 --- a/src/backend/wayland/backend/state_init/mod.rs +++ b/src/backend/wayland/backend/state_init/mod.rs @@ -104,14 +104,17 @@ pub(super) fn init_state(backend: &WaylandBackend, setup: WaylandSetup) -> Resul }; input_state.set_session_preflight_options(session_options.clone()); let screencopy_supported = setup.screencopy_manager.is_some(); + let image_copy_capture_supported = setup.ext_image_copy_managers.is_some(); let portal_freeze_supported = screenshot_portal_available(&backend.tokio_runtime); - let frozen_supported = screencopy_supported || portal_freeze_supported; + let direct_capture_supported = screencopy_supported || image_copy_capture_supported; + let frozen_supported = direct_capture_supported || portal_freeze_supported; let tokio_handle = backend.tokio_runtime.handle().clone(); // Set compositor capabilities based on detected Wayland protocols input_state.compositor_capabilities = CompositorCapabilities { layer_shell: setup.layer_shell_available, screencopy: screencopy_supported, + image_copy_capture: image_copy_capture_supported, freeze_capture: frozen_supported, pointer_constraints: setup .state_globals @@ -190,7 +193,7 @@ pub(super) fn init_state(backend: &WaylandBackend, setup: WaylandSetup) -> Resul let freeze_on_start = if backend.freeze_on_start && !frozen_supported { warn!( - "Frozen mode unavailable: no screencopy backend and no screenshot portal backend; ignoring --freeze" + "Frozen mode unavailable: no direct capture backend and no screenshot portal backend; ignoring --freeze" ); false } else { @@ -218,6 +221,8 @@ pub(super) fn init_state(backend: &WaylandBackend, setup: WaylandSetup) -> Resul main_surface_uses_overlay_layer: output_prefs.main_surface_uses_overlay_layer, pending_freeze_on_start: freeze_on_start, screencopy_manager: setup.screencopy_manager, + ext_image_copy_managers: setup.ext_image_copy_managers, + portal_freeze_supported, text_input_manager: setup.text_input_manager, #[cfg(feature = "tablet-input")] tablet_manager, diff --git a/src/backend/wayland/frozen/capture.rs b/src/backend/wayland/frozen/capture.rs index 43fb3974..de240142 100644 --- a/src/backend/wayland/frozen/capture.rs +++ b/src/backend/wayland/frozen/capture.rs @@ -5,15 +5,26 @@ use smithay_client_toolkit::shm::{ slot::{Buffer, SlotPool}, }; use wayland_client::{Dispatch, QueueHandle, WEnum, protocol::wl_shm}; +use wayland_protocols::ext::{ + image_capture_source::v1::client::{ + ext_image_capture_source_v1::ExtImageCaptureSourceV1, + ext_output_image_capture_source_manager_v1::ExtOutputImageCaptureSourceManagerV1, + }, + image_copy_capture::v1::client::{ + ext_image_copy_capture_frame_v1::ExtImageCopyCaptureFrameV1, + ext_image_copy_capture_manager_v1::ExtImageCopyCaptureManagerV1, + ext_image_copy_capture_session_v1::ExtImageCopyCaptureSessionV1, + }, +}; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_frame_v1::{ Event as FrameEvent, Flags, ZwlrScreencopyFrameV1, }; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; -use crate::backend::wayland::frozen::FrozenImage; use crate::input::InputState; -use super::state::FrozenState; +use super::image::copy_shm_argb; +use super::state::{DirectCaptureBackend, DirectCaptureContext, FrozenCaptureBackend, FrozenState}; /// Internal capture session tracking a single screencopy frame. pub(super) struct CaptureSession { @@ -69,38 +80,134 @@ impl CaptureSession { impl FrozenState { /// Start a screencopy capture for the active output. - pub fn start_capture( - &mut self, - use_fallback: bool, - _tokio_handle: &tokio::runtime::Handle, - ) -> Result<()> { - if self.capture.is_some() || self.portal_in_progress || self.preflight_pending { + pub fn start_capture(&mut self) -> Result<()> { + if self.capture.is_some() + || self.ext_capture.is_some() + || self.direct_capture.is_some() + || self.portal_in_progress + || self.preflight_pending + { warn!("Frozen-mode capture already in progress; ignoring toggle"); return Ok(()); } self.capture_done = false; - self.preflight_use_fallback = use_fallback || self.manager.is_none(); + self.preflight_backend = Some( + self.preferred_backend() + .context("no frozen capture backend is available")?, + ); self.preflight_pending = true; Ok(()) } pub fn begin_preflight_capture( &mut self, - use_fallback: bool, + backend: FrozenCaptureBackend, shm: &Shm, qh: &QueueHandle, tokio_handle: &tokio::runtime::Handle, ) -> Result<()> where - State: - Dispatch + Dispatch + 'static, + State: Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + 'static, + { + self.begin_capture_chain(backend, shm, qh, tokio_handle) + } + + pub(in crate::backend::wayland) fn begin_fallback_capture( + &mut self, + failed_backend: FrozenCaptureBackend, + shm: &Shm, + qh: &QueueHandle, + tokio_handle: &tokio::runtime::Handle, + ) -> Result<()> + where + State: Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + 'static, + { + let backend = self + .next_backend_after(failed_backend) + .context("no remaining frozen capture backend is available")?; + self.begin_capture_chain(backend, shm, qh, tokio_handle) + } + + fn begin_capture_chain( + &mut self, + first_backend: FrozenCaptureBackend, + shm: &Shm, + qh: &QueueHandle, + tokio_handle: &tokio::runtime::Handle, + ) -> Result<()> + where + State: Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + 'static, { - if use_fallback || self.manager.is_none() { - info!("Suppression frame committed; using fallback portal capture for frozen mode"); - self.capture_via_portal(tokio_handle) - } else { - self.begin_screencopy(shm, qh) + let mut backend = Some(first_backend); + let mut last_error = None; + + while let Some(current) = backend { + match self.begin_capture_backend(current, shm, qh, tokio_handle) { + Ok(()) => return Ok(()), + Err(error) => { + warn!("Failed to start {current:?} frozen capture: {error:#}"); + last_error = Some(error); + backend = self.next_backend_after(current); + } + } + } + + Err(last_error + .unwrap_or_else(|| anyhow::anyhow!("no frozen capture backend was attempted"))) + } + + fn begin_capture_backend( + &mut self, + backend: FrozenCaptureBackend, + shm: &Shm, + qh: &QueueHandle, + tokio_handle: &tokio::runtime::Handle, + ) -> Result<()> + where + State: Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + 'static, + { + match backend { + FrozenCaptureBackend::WlrScreencopy => { + info!("Suppression frame committed; using wlr-screencopy for frozen mode"); + self.begin_screencopy(shm, qh) + } + FrozenCaptureBackend::ExtImageCopy => { + info!("Suppression frame committed; using ext-image-copy for frozen mode"); + self.begin_ext_image_copy(shm, qh) + } + FrozenCaptureBackend::Portal => { + info!("Suppression frame committed; using portal capture for frozen mode"); + self.capture_via_portal(tokio_handle) + } } } @@ -122,23 +229,28 @@ impl FrozenState { anyhow::bail!("No active output available for frozen capture"); } }; + let target_output_id = self + .active_output_id + .context("Active output has no stable identity for frozen capture")?; + let source_geometry = self.active_geometry.clone(); + let pool = SlotPool::new(4, shm).context("Failed to create frozen capture pool")?; debug!("Requesting screencopy frame for active output"); let frame = manager.capture_output(0, &output, qh, ()); - self.capture = Some(CaptureSession::new(frame)); - - // Pre-allocate a pool to avoid repeated allocations; size adjusted on buffer event - // (SlotPool resize is cheap, so start with minimal size). - if let Some(capture) = self.capture.as_mut() { - capture.pool = - Some(SlotPool::new(4, shm).context("Failed to create frozen capture pool")?); - } + let mut capture = CaptureSession::new(frame); + capture.pool = Some(pool); + self.capture = Some(capture); + self.direct_capture = Some(DirectCaptureContext::new( + DirectCaptureBackend::WlrScreencopy, + target_output_id, + source_geometry, + )); Ok(()) } /// Handle screencopy frame events. - pub fn handle_frame_event(&mut self, event: FrameEvent, input_state: &mut InputState) { + pub fn handle_frame_event(&mut self, event: FrameEvent, input_state: &mut InputState) -> bool { match event { FrameEvent::Buffer { format, @@ -148,7 +260,7 @@ impl FrozenState { } => { if let Err(err) = self.on_buffer(format, width, height, stride) { warn!("Failed to prepare screencopy buffer: {}", err); - self.cancel(input_state); + return self.fail_wlr_capture(); } } FrameEvent::LinuxDmabuf { .. } => { @@ -158,7 +270,7 @@ impl FrozenState { FrameEvent::BufferDone => { if let Err(err) = self.on_buffer_done() { warn!("Failed to issue screencopy copy: {}", err); - self.cancel(input_state); + return self.fail_wlr_capture(); } } FrameEvent::Flags { flags } => { @@ -172,21 +284,32 @@ impl FrozenState { .unwrap_or(false); } } - FrameEvent::Ready { .. } => { - if let Err(err) = self.on_ready() { + FrameEvent::Ready { .. } => match self.on_ready() { + Ok(true) => input_state.needs_redraw = true, + Ok(false) => { + warn!("Discarded WLR frozen capture because the active output changed"); + self.finish_stale_direct_capture(input_state); + } + Err(err) => { warn!("Frozen capture ready handling failed: {}", err); - self.cancel(input_state); - return; + return self.fail_wlr_capture(); } - - input_state.needs_redraw = true; - } + }, FrameEvent::Failed => { warn!("Frozen capture failed"); - self.cancel(input_state); + return self.fail_wlr_capture(); } _ => {} } + false + } + + fn fail_wlr_capture(&mut self) -> bool { + if let Some(capture) = self.capture.take() { + capture.frame.destroy(); + } + self.direct_capture = None; + true } fn on_buffer( @@ -236,57 +359,48 @@ impl FrozenState { Ok(()) } - fn on_ready(&mut self) -> Result<()> { + fn on_ready(&mut self) -> Result { let mut capture = self .capture .take() .context("No capture session present for ready event")?; - - let pool = capture.pool.as_mut().context("Capture pool missing")?; - let buffer = capture.buffer.as_ref().context("Capture buffer missing")?; - - let canvas = buffer - .canvas(pool) - .context("Unable to map capture buffer")?; - - let pixel_width = (capture.width * 4) as usize; - let stride = capture.stride as usize; - if stride < pixel_width { - anyhow::bail!("Capture stride smaller than expected pixel width"); + let result = (|| { + let pool = capture.pool.as_mut().context("Capture pool missing")?; + let buffer = capture.buffer.as_ref().context("Capture buffer missing")?; + let canvas = buffer + .canvas(pool) + .context("Unable to map capture buffer")?; + let format = capture.format.context("Capture format missing")?; + copy_shm_argb( + canvas, + capture.width, + capture.height, + capture.stride, + format, + capture.y_invert, + ) + })(); + capture.frame.destroy(); + let image = result?; + let context = self + .direct_capture + .take() + .context("WLR direct capture context missing")?; + if context.backend != DirectCaptureBackend::WlrScreencopy { + anyhow::bail!("WLR capture completed with a mismatched direct backend context"); } - - let mut data = vec![0u8; (capture.width * capture.height * 4) as usize]; - - for row in 0..capture.height as usize { - let src_row = &canvas[(row * stride)..(row * stride + pixel_width)]; - let dest_row_index = if capture.y_invert { - (capture.height as usize - 1 - row) * pixel_width - } else { - row * pixel_width - }; - data[dest_row_index..dest_row_index + pixel_width].copy_from_slice(src_row); + if !context.output_matches(self.active_output_id) { + return Ok(false); } - if matches!(capture.format, Some(wl_shm::Format::Xrgb8888)) { - for chunk in data.chunks_exact_mut(4) { - // Ensure alpha channel is opaque - chunk[3] = 0xFF; - } - } + self.set_pending_output_image(image, context.source_geometry); - capture.frame.destroy(); - - let source_geometry = self.active_geometry.clone(); - self.set_pending_output_image( - FrozenImage { - width: capture.width, - height: capture.height, - stride: (capture.width * 4) as i32, - data, - }, - source_geometry, - ); + Ok(true) + } - Ok(()) + pub(super) fn finish_stale_direct_capture(&mut self, input_state: &mut InputState) { + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; } } diff --git a/src/backend/wayland/frozen/ext_image_copy.rs b/src/backend/wayland/frozen/ext_image_copy.rs new file mode 100644 index 00000000..52512e06 --- /dev/null +++ b/src/backend/wayland/frozen/ext_image_copy.rs @@ -0,0 +1,528 @@ +use anyhow::{Context, Result}; +use log::{debug, warn}; +use smithay_client_toolkit::shm::{ + Shm, + slot::{Buffer, SlotPool}, +}; +use wayland_client::{ + Dispatch, QueueHandle, WEnum, + protocol::{wl_output, wl_shm}, +}; +use wayland_protocols::ext::{ + image_capture_source::v1::client::{ + ext_image_capture_source_v1::ExtImageCaptureSourceV1, + ext_output_image_capture_source_manager_v1::ExtOutputImageCaptureSourceManagerV1, + }, + image_copy_capture::v1::client::{ + ext_image_copy_capture_frame_v1::{ + Event as FrameEvent, ExtImageCopyCaptureFrameV1, FailureReason, + }, + ext_image_copy_capture_manager_v1::{ExtImageCopyCaptureManagerV1, Options}, + ext_image_copy_capture_session_v1::{Event as SessionEvent, ExtImageCopyCaptureSessionV1}, + }, +}; + +use crate::input::InputState; + +use super::image::copy_shm_argb; +use super::state::{DirectCaptureBackend, DirectCaptureContext, FrozenState}; + +const MAX_CONSTRAINT_RETRIES: u8 = 2; + +#[derive(Clone)] +pub(in crate::backend::wayland) struct ExtImageCopyManagers { + capture: ExtImageCopyCaptureManagerV1, + output_source: ExtOutputImageCaptureSourceManagerV1, +} + +impl ExtImageCopyManagers { + pub(in crate::backend::wayland) fn new( + capture: ExtImageCopyCaptureManagerV1, + output_source: ExtOutputImageCaptureSourceManagerV1, + ) -> Self { + Self { + capture, + output_source, + } + } +} + +pub(super) struct ExtImageCopySession { + source: ExtImageCaptureSourceV1, + session: ExtImageCopyCaptureSessionV1, + pool: SlotPool, + constraints: ConstraintTracker, + frame: Option, +} + +struct ExtImageCopyFrame { + proxy: ExtImageCopyCaptureFrameV1, + buffer: Buffer, + width: u32, + height: u32, + stride: i32, + format: wl_shm::Format, + transform: Option, +} + +impl ExtImageCopySession { + fn new( + source: ExtImageCaptureSourceV1, + session: ExtImageCopyCaptureSessionV1, + pool: SlotPool, + ) -> Self { + Self { + source, + session, + pool, + constraints: ConstraintTracker::default(), + frame: None, + } + } + + pub(super) fn destroy(mut self) { + if let Some(frame) = self.frame.take() { + frame.proxy.destroy(); + } + self.session.destroy(); + self.source.destroy(); + } +} + +#[derive(Default)] +struct ConstraintTracker { + pending: ExtBufferConstraints, + replacement: Option, + retries: u8, +} + +impl ConstraintTracker { + fn record_size(&mut self, width: u32, height: u32) { + self.pending.size = Some((width, height)); + } + + fn record_format(&mut self, format: WEnum) { + if let WEnum::Value(format) = format + && !self.pending.formats.contains(&format) + { + self.pending.formats.push(format); + } + } + + fn finish_batch(&mut self, frame_pending: bool) -> Option { + let batch = std::mem::take(&mut self.pending); + if frame_pending { + self.replacement = Some(batch); + None + } else { + Some(batch) + } + } + + fn take_replacement_for_retry(&mut self) -> Result { + if self.retries >= MAX_CONSTRAINT_RETRIES { + anyhow::bail!( + "ext-image-copy exceeded its {MAX_CONSTRAINT_RETRIES} buffer-constraint retries" + ); + } + self.retries += 1; + self.replacement.take().context( + "compositor reported a buffer-constraints failure without a complete replacement batch", + ) + } +} + +#[derive(Default)] +struct ExtBufferConstraints { + size: Option<(u32, u32)>, + formats: Vec, +} + +impl FrozenState { + pub(super) fn begin_ext_image_copy( + &mut self, + shm: &Shm, + qh: &QueueHandle, + ) -> Result<()> + where + State: Dispatch + + Dispatch + + Dispatch + + Dispatch + + Dispatch + + 'static, + { + let managers = self + .ext_managers + .as_ref() + .context("ext-image-copy-capture managers are unavailable")?; + let output = self + .active_output + .as_ref() + .context("No active output available for ext-image-copy capture")?; + let target_output_id = self + .active_output_id + .context("Active output has no stable identity for ext-image-copy capture")?; + let source_geometry = self.active_geometry.clone(); + + // Allocate the only fallible local resource before creating protocol + // objects so an allocation failure cannot leave a live source/session. + let pool = + SlotPool::new(4, shm).context("Failed to create ext-image-copy shared-memory pool")?; + let source = managers.output_source.create_source(output, qh, ()); + let session = managers + .capture + .create_session(&source, Options::empty(), qh, ()); + self.ext_capture = Some(ExtImageCopySession::new(source, session, pool)); + self.direct_capture = Some(DirectCaptureContext::new( + DirectCaptureBackend::ExtImageCopy, + target_output_id, + source_geometry, + )); + debug!("Requested ext-image-copy capture constraints for active output"); + Ok(()) + } + + pub(in crate::backend::wayland) fn handle_ext_session_event( + &mut self, + event: SessionEvent, + qh: &QueueHandle, + ) -> bool + where + State: Dispatch + 'static, + { + let result = match event { + SessionEvent::BufferSize { width, height } => { + let capture = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing"); + capture.map(|capture| capture.constraints.record_size(width, height)) + } + SessionEvent::ShmFormat { format } => { + let capture = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing"); + capture.map(|capture| capture.constraints.record_format(format)) + } + SessionEvent::Done => self.finish_constraint_batch(qh), + SessionEvent::Stopped => Err(anyhow::anyhow!( + "ext-image-copy capture session was stopped by the compositor" + )), + SessionEvent::DmabufDevice { .. } | SessionEvent::DmabufFormat { .. } => Ok(()), + _ => Ok(()), + }; + + if let Err(error) = result { + warn!("Ext-image-copy session failed: {error:#}"); + return self.fail_ext_capture(); + } + false + } + + fn finish_constraint_batch(&mut self, qh: &QueueHandle) -> Result<()> + where + State: Dispatch + 'static, + { + let constraints = { + let capture = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing")?; + capture.constraints.finish_batch(capture.frame.is_some()) + }; + if let Some(constraints) = constraints { + self.submit_ext_frame(qh, constraints)?; + } else { + debug!("Deferred replacement ext-image-copy constraints until the current frame ends"); + } + Ok(()) + } + + fn submit_ext_frame( + &mut self, + qh: &QueueHandle, + constraints: ExtBufferConstraints, + ) -> Result<()> + where + State: Dispatch + 'static, + { + let capture = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing")?; + if capture.frame.is_some() { + anyhow::bail!("ext-image-copy frame submitted while another frame is pending"); + } + let (width, height) = constraints + .size + .context("compositor omitted the ext-image-copy buffer size")?; + if width == 0 || height == 0 { + anyhow::bail!("compositor advertised an empty ext-image-copy buffer"); + } + let format = select_shm_format(&constraints.formats) + .context("compositor did not advertise an ARGB/XRGB shared-memory format")?; + let stride = width + .checked_mul(4) + .and_then(|value| i32::try_from(value).ok()) + .context("ext-image-copy buffer stride overflow")?; + let total_size = usize::try_from(stride) + .ok() + .and_then(|stride| { + usize::try_from(height) + .ok() + .and_then(|height| stride.checked_mul(height)) + }) + .context("ext-image-copy buffer size overflow")?; + let buffer_width = i32::try_from(width) + .context("ext-image-copy buffer width exceeds the Wayland limit")?; + let buffer_height = i32::try_from(height) + .context("ext-image-copy buffer height exceeds the Wayland limit")?; + if total_size > capture.pool.len() { + capture.pool.resize(total_size)?; + } + let (buffer, _) = capture + .pool + .create_buffer(buffer_width, buffer_height, stride, format) + .context("Failed to create ext-image-copy buffer")?; + let frame = capture.session.create_frame(qh, ()); + frame.attach_buffer(buffer.wl_buffer()); + frame.damage_buffer(0, 0, buffer_width, buffer_height); + frame.capture(); + capture.frame = Some(ExtImageCopyFrame { + proxy: frame, + buffer, + width, + height, + stride, + format, + transform: None, + }); + Ok(()) + } + + pub(in crate::backend::wayland) fn handle_ext_frame_event( + &mut self, + event: FrameEvent, + qh: &QueueHandle, + input_state: &mut InputState, + ) -> bool + where + State: Dispatch + 'static, + { + match event { + FrameEvent::Ready => match self.finish_ext_frame() { + Ok(true) => input_state.needs_redraw = true, + Ok(false) => { + warn!( + "Discarded ext-image-copy frozen capture because the active output changed" + ); + self.finish_stale_direct_capture(input_state); + } + Err(error) => { + warn!("Ext-image-copy frame failed: {error:#}"); + return self.fail_ext_capture(); + } + }, + FrameEvent::Failed { reason } => { + match reason { + WEnum::Value(FailureReason::BufferConstraints) => { + if let Err(error) = self.retry_ext_frame_after_constraints(qh) { + warn!( + "Failed to retry ext-image-copy with replacement constraints: {error:#}" + ); + return self.fail_ext_capture(); + } + return false; + } + WEnum::Value(FailureReason::Unknown) => { + warn!("Ext-image-copy frame failed: unknown compositor error"); + } + WEnum::Value(FailureReason::Stopped) => { + warn!("Ext-image-copy frame failed: capture session stopped"); + } + WEnum::Unknown(_) => { + warn!("Ext-image-copy frame failed: unknown failure reason"); + } + _ => { + warn!("Ext-image-copy frame failed: unsupported failure reason"); + } + } + return self.fail_ext_capture(); + } + FrameEvent::Transform { transform } => { + if let Some(capture) = self.ext_capture.as_mut() + && let Some(frame) = capture.frame.as_mut() + && let WEnum::Value(transform) = transform + { + frame.transform = Some(transform); + } + } + FrameEvent::Damage { .. } | FrameEvent::PresentationTime { .. } => {} + _ => {} + } + false + } + + fn retry_ext_frame_after_constraints(&mut self, qh: &QueueHandle) -> Result<()> + where + State: Dispatch + 'static, + { + let replacement = { + let capture = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing")?; + let frame = capture + .frame + .take() + .context("failed ext-image-copy frame missing")?; + frame.proxy.destroy(); + capture.constraints.take_replacement_for_retry() + }; + + let replacement = replacement?; + debug!("Retrying ext-image-copy with replacement buffer constraints"); + self.submit_ext_frame(qh, replacement) + } + + fn finish_ext_frame(&mut self) -> Result { + let frame = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing")?; + let frame = frame.frame.take().context("ext-image-copy frame missing")?; + let result: Result<_> = (|| { + let capture = self + .ext_capture + .as_mut() + .context("ext-image-copy capture session missing")?; + let canvas = frame + .buffer + .canvas(&mut capture.pool) + .context("Unable to map ext-image-copy buffer")?; + let image = copy_shm_argb( + canvas, + frame.width, + frame.height, + frame.stride, + frame.format, + false, + )?; + Ok((image, frame.transform)) + })(); + frame.proxy.destroy(); + let (image, output_transform) = result?; + let capture = self + .ext_capture + .take() + .context("ext-image-copy capture session missing after frame completion")?; + capture.destroy(); + let context = self + .direct_capture + .take() + .context("ext-image-copy direct capture context missing")?; + if context.backend != DirectCaptureBackend::ExtImageCopy { + anyhow::bail!("ext-image-copy completed with a mismatched direct backend context"); + } + if !context.output_matches(self.active_output_id) { + return Ok(false); + } + self.set_pending_output_image_with_transform( + image, + context.source_geometry, + output_transform, + ); + Ok(true) + } + + fn fail_ext_capture(&mut self) -> bool { + if let Some(capture) = self.ext_capture.take() { + capture.destroy(); + } + self.direct_capture = None; + true + } +} + +fn select_shm_format(formats: &[wl_shm::Format]) -> Option { + [wl_shm::Format::Argb8888, wl_shm::Format::Xrgb8888] + .into_iter() + .find(|preferred| formats.contains(preferred)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn shm_format_selection_prefers_argb_and_accepts_xrgb() { + let formats = [wl_shm::Format::Xrgb8888, wl_shm::Format::Argb8888]; + assert_eq!(select_shm_format(&formats), Some(wl_shm::Format::Argb8888)); + assert_eq!( + select_shm_format(&[wl_shm::Format::Xrgb8888]), + Some(wl_shm::Format::Xrgb8888) + ); + assert_eq!(select_shm_format(&[wl_shm::Format::Rgb565]), None); + } + + #[test] + fn replacement_constraint_batch_does_not_mutate_the_in_flight_layout() { + let mut tracker = ConstraintTracker::default(); + tracker.record_size(1920, 1080); + tracker.record_format(WEnum::Value(wl_shm::Format::Argb8888)); + let initial = tracker + .finish_batch(false) + .expect("initial constraints should be submitted"); + + tracker.record_size(1280, 720); + tracker.record_format(WEnum::Value(wl_shm::Format::Xrgb8888)); + assert!( + tracker.finish_batch(true).is_none(), + "replacement constraints wait while the original frame is pending" + ); + + assert_eq!(initial.size, Some((1920, 1080))); + assert_eq!(initial.formats, vec![wl_shm::Format::Argb8888]); + let replacement = tracker + .take_replacement_for_retry() + .expect("replacement constraints retained for retry"); + assert_eq!(replacement.size, Some((1280, 720))); + assert_eq!(replacement.formats, vec![wl_shm::Format::Xrgb8888]); + } + + #[test] + fn constraint_batches_do_not_accumulate_old_formats() { + let mut tracker = ConstraintTracker::default(); + tracker.record_size(100, 100); + tracker.record_format(WEnum::Value(wl_shm::Format::Argb8888)); + let _ = tracker.finish_batch(false); + + tracker.record_size(200, 200); + tracker.record_format(WEnum::Value(wl_shm::Format::Xrgb8888)); + let replacement = tracker + .finish_batch(false) + .expect("second batch should be independent"); + + assert_eq!(replacement.formats, vec![wl_shm::Format::Xrgb8888]); + } + + #[test] + fn constraint_retries_are_bounded_before_backend_fallback() { + let mut tracker = ConstraintTracker::default(); + for attempt in 0..MAX_CONSTRAINT_RETRIES { + tracker.record_size(100 + u32::from(attempt), 100); + tracker.record_format(WEnum::Value(wl_shm::Format::Argb8888)); + assert!(tracker.finish_batch(true).is_none()); + tracker + .take_replacement_for_retry() + .expect("retry remains within the budget"); + } + + tracker.record_size(200, 100); + tracker.record_format(WEnum::Value(wl_shm::Format::Argb8888)); + assert!(tracker.finish_batch(true).is_none()); + assert!(tracker.take_replacement_for_retry().is_err()); + } +} diff --git a/src/backend/wayland/frozen/image.rs b/src/backend/wayland/frozen/image.rs index e49f3245..440b646e 100644 --- a/src/backend/wayland/frozen/image.rs +++ b/src/backend/wayland/frozen/image.rs @@ -1,4 +1,5 @@ -use wayland_client::protocol::wl_output; +use anyhow::{Context, Result}; +use wayland_client::protocol::{wl_output, wl_shm}; /// CPU-side frozen image ready for Cairo rendering. pub struct FrozenImage { @@ -35,6 +36,78 @@ impl FrozenImage { } } +/// Copy a compositor-provided SHM buffer into tightly packed Cairo-compatible +/// ARGB data while validating every advertised dimension and row boundary. +pub(super) fn copy_shm_argb( + canvas: &[u8], + width: u32, + height: u32, + stride: i32, + format: wl_shm::Format, + y_invert: bool, +) -> Result { + if !matches!(format, wl_shm::Format::Argb8888 | wl_shm::Format::Xrgb8888) { + anyhow::bail!("Unsupported frozen capture SHM format: {format:?}"); + } + + let width = usize::try_from(width).context("Frozen capture width does not fit in memory")?; + let height = usize::try_from(height).context("Frozen capture height does not fit in memory")?; + let row_bytes = width + .checked_mul(4) + .context("Frozen capture row size overflow")?; + let stride = usize::try_from(stride).context("Frozen capture stride is negative")?; + if stride < row_bytes { + anyhow::bail!("Frozen capture stride is smaller than its pixel row"); + } + + let image_size = row_bytes + .checked_mul(height) + .context("Frozen capture image size overflow")?; + let mut data = vec![0; image_size]; + for source_row in 0..height { + let source_start = source_row + .checked_mul(stride) + .context("Frozen capture source row offset overflow")?; + let source_end = source_start + .checked_add(row_bytes) + .context("Frozen capture source row end overflow")?; + let target_row = if y_invert { + height - 1 - source_row + } else { + source_row + }; + let target_start = target_row + .checked_mul(row_bytes) + .context("Frozen capture target row offset overflow")?; + let target_end = target_start + .checked_add(row_bytes) + .context("Frozen capture target row end overflow")?; + let source = canvas + .get(source_start..source_end) + .context("Frozen capture buffer is shorter than advertised")?; + let target = data + .get_mut(target_start..target_end) + .context("Frozen capture image allocation is shorter than expected")?; + target.copy_from_slice(source); + } + + if format == wl_shm::Format::Xrgb8888 { + for pixel in data.chunks_exact_mut(4) { + pixel[3] = 0xff; + } + } + + let width = u32::try_from(width).context("Frozen capture width exceeds u32")?; + let height = u32::try_from(height).context("Frozen capture height exceeds u32")?; + let stride = i32::try_from(row_bytes).context("Frozen capture packed stride exceeds i32")?; + Ok(FrozenImage { + width, + height, + stride, + data, + }) +} + fn transform_argb( width: usize, height: usize, @@ -140,4 +213,32 @@ mod tests { assert_eq!((transformed.width, transformed.height), (3, 2)); assert_eq!(values(&transformed), vec![3, 2, 1, 6, 5, 4]); } + + #[test] + fn shm_copy_removes_padding_and_applies_y_invert() { + let image = copy_shm_argb( + &[ + 1, 0, 0, 255, 2, 0, 0, 255, 9, 9, 9, 9, // + 3, 0, 0, 255, 4, 0, 0, 255, 8, 8, 8, 8, + ], + 2, + 2, + 12, + wl_shm::Format::Argb8888, + true, + ) + .expect("valid padded SHM buffer"); + + assert_eq!(values(&image), vec![3, 4, 1, 2]); + assert_eq!(image.stride, 8); + } + + #[test] + fn shm_copy_makes_xrgb_pixels_opaque_and_rejects_short_buffers() { + let image = copy_shm_argb(&[1, 2, 3, 0], 1, 1, 4, wl_shm::Format::Xrgb8888, false) + .expect("valid XRGB buffer"); + assert_eq!(image.data, vec![1, 2, 3, 255]); + + assert!(copy_shm_argb(&[1, 2, 3], 1, 1, 4, wl_shm::Format::Argb8888, false,).is_err()); + } } diff --git a/src/backend/wayland/frozen/mod.rs b/src/backend/wayland/frozen/mod.rs index 1131f565..4f0980cb 100644 --- a/src/backend/wayland/frozen/mod.rs +++ b/src/backend/wayland/frozen/mod.rs @@ -1,9 +1,12 @@ mod capture; +mod ext_image_copy; mod image; mod portal; mod state; +pub(in crate::backend::wayland) use ext_image_copy::ExtImageCopyManagers; pub use image::FrozenImage; +pub(in crate::backend::wayland) use state::FrozenCaptureBackend; pub use state::FrozenState; type PortalCaptureResult = Result< @@ -12,5 +15,5 @@ type PortalCaptureResult = Result< Option, self::image::FrozenImage, ), - String, + crate::capture::types::CaptureError, >; diff --git a/src/backend/wayland/frozen/portal.rs b/src/backend/wayland/frozen/portal.rs index ef30d2da..a68a4434 100644 --- a/src/backend/wayland/frozen/portal.rs +++ b/src/backend/wayland/frozen/portal.rs @@ -9,12 +9,13 @@ use crate::backend::wayland::portal_capture::{ }; use crate::backend::wayland::portal_task::{PortalPoll, PortalTask}; use crate::capture::sources::frozen::decode_image_to_argb; +use crate::capture::types::CaptureError; use crate::input::InputState; use super::state::FrozenState; impl FrozenState { - pub(super) fn capture_via_portal( + pub(in crate::backend::wayland) fn capture_via_portal( &mut self, tokio_handle: &tokio::runtime::Handle, ) -> Result<()> { @@ -43,8 +44,8 @@ impl FrozenState { async { let bytes = capture_via_portal_fullscreen_bytes().await?; - let (data, width, height) = - decode_image_to_argb(&bytes).map_err(|e| format!("Decode failed: {}", e))?; + let (data, width, height) = decode_image_to_argb(&bytes) + .map_err(|error| CaptureError::ImageError(format!("Decode failed: {error}")))?; Ok(( target_output_id, @@ -105,7 +106,14 @@ impl FrozenState { self.finish_portal_task(); } - PortalPoll::Ready(Err(err)) | PortalPoll::Failed(err) => { + PortalPoll::Ready(Err(CaptureError::Cancelled(reason))) => { + log::info!("Portal frozen capture cancelled: {reason}"); + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + self.finish_portal_task(); + self.capture_done = true; + } + PortalPoll::Ready(Err(err)) => { warn!("Portal frozen capture failed: {err}"); input_state.push_toast( ToastPriority::Critical, @@ -116,6 +124,17 @@ impl FrozenState { self.finish_portal_task(); self.capture_done = true; } + PortalPoll::Failed(err) => { + warn!("Portal frozen capture task failed: {err}"); + input_state.push_toast( + ToastPriority::Critical, + "freeze", + Toast::error("Freeze could not capture the screen."), + ); + input_state.set_frozen_active(false); + self.finish_portal_task(); + self.capture_done = true; + } PortalPoll::Pending => {} PortalPoll::Disconnected => { warn!("Portal frozen capture channel disconnected"); @@ -206,7 +225,7 @@ mod tests { }) } else { PortalTask::spawn(&tokio::runtime::Handle::current(), wake.handle(), async { - Err("portal denied".to_string()) + Err(CaptureError::PermissionDenied) }) }); frozen.portal_in_progress = true; @@ -248,6 +267,30 @@ mod tests { } } + #[tokio::test] + async fn user_cancellation_restores_quietly_without_an_error_toast() { + let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); + let mut input = make_test_input_state(); + frozen.portal_task = Some(PortalTask::spawn( + &tokio::runtime::Handle::current(), + wake.handle(), + async { + Err(CaptureError::Cancelled( + "user closed the chooser".to_string(), + )) + }, + )); + frozen.portal_in_progress = true; + + poll_until_finished(&mut frozen, &mut input).await; + + assert!(!frozen.is_in_progress()); + assert!(frozen.take_capture_done()); + assert!(!input.frozen_active()); + assert!(input.ui_toast.is_none()); + } + #[tokio::test] async fn stale_output_is_discarded_without_mutating_current_frozen_state() { let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); diff --git a/src/backend/wayland/frozen/state.rs b/src/backend/wayland/frozen/state.rs index e4c62772..a338149f 100644 --- a/src/backend/wayland/frozen/state.rs +++ b/src/backend/wayland/frozen/state.rs @@ -1,4 +1,5 @@ use log::info; +use std::time::{Duration, Instant}; use wayland_client::protocol::wl_output; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; @@ -11,10 +12,12 @@ use crate::input::InputState; use super::PortalCaptureResult; use super::capture::CaptureSession; +use super::ext_image_copy::{ExtImageCopyManagers, ExtImageCopySession}; struct PendingFrozenImage { image: FrozenImage, source_geometry: Option, + output_transform: Option, needs_output_transform: bool, source: FrozenCaptureSource, } @@ -25,14 +28,84 @@ enum FrozenCaptureSource { Desktop, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(in crate::backend::wayland) enum FrozenCaptureBackend { + WlrScreencopy, + ExtImageCopy, + Portal, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum DirectCaptureBackend { + WlrScreencopy, + ExtImageCopy, +} + +impl DirectCaptureBackend { + fn capture_backend(self) -> FrozenCaptureBackend { + match self { + Self::WlrScreencopy => FrozenCaptureBackend::WlrScreencopy, + Self::ExtImageCopy => FrozenCaptureBackend::ExtImageCopy, + } + } +} + +pub(super) const DIRECT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(3); + +pub(super) struct DirectCaptureContext { + pub(super) backend: DirectCaptureBackend, + pub(super) target_output_id: u32, + pub(super) source_geometry: Option, + started_at: Instant, +} + +impl DirectCaptureContext { + pub(super) fn new( + backend: DirectCaptureBackend, + target_output_id: u32, + source_geometry: Option, + ) -> Self { + Self::new_at(backend, target_output_id, source_geometry, Instant::now()) + } + + fn new_at( + backend: DirectCaptureBackend, + target_output_id: u32, + source_geometry: Option, + started_at: Instant, + ) -> Self { + Self { + backend, + target_output_id, + source_geometry, + started_at, + } + } + + fn timeout(&self, now: Instant) -> Duration { + self.started_at + .checked_add(DIRECT_CAPTURE_TIMEOUT) + .map(|deadline| deadline.saturating_duration_since(now)) + .unwrap_or(Duration::ZERO) + } + + pub(super) fn output_matches(&self, current_output_id: Option) -> bool { + current_output_id == Some(self.target_output_id) + } +} + /// End-to-end controller for frozen mode capture and image storage. #[allow(clippy::type_complexity)] pub struct FrozenState { pub(super) manager: Option, + pub(super) ext_managers: Option, + pub(super) ext_capture: Option, + pub(super) portal_available: bool, pub(super) active_output: Option, pub(super) active_output_id: Option, pub(super) active_geometry: Option, pub(super) capture: Option, + pub(super) direct_capture: Option, pub(super) image: Option, image_target_dimensions: Option<(u32, u32)>, image_generation: u64, @@ -41,7 +114,7 @@ pub struct FrozenState { pub(super) portal_target_output_id: Option, pub(super) runtime_wake: Option, pub(super) preflight_pending: bool, - pub(super) preflight_use_fallback: bool, + pub(super) preflight_backend: Option, pub(super) capture_done: bool, pending_image: Option, } @@ -49,26 +122,42 @@ pub struct FrozenState { impl FrozenState { #[cfg(test)] pub fn new(manager: Option) -> Self { - Self::new_inner(manager, None) + Self::new_inner(manager, None, false, None) } + #[cfg(test)] pub(in crate::backend::wayland) fn new_with_runtime_wake( manager: Option, runtime_wake: RuntimeWakeHandle, ) -> Self { - Self::new_inner(manager, Some(runtime_wake)) + Self::new_inner(manager, None, true, Some(runtime_wake)) + } + + pub(in crate::backend::wayland) fn new_with_backends( + manager: Option, + ext_managers: Option, + portal_available: bool, + runtime_wake: RuntimeWakeHandle, + ) -> Self { + Self::new_inner(manager, ext_managers, portal_available, Some(runtime_wake)) } fn new_inner( manager: Option, + ext_managers: Option, + portal_available: bool, runtime_wake: Option, ) -> Self { Self { manager, + ext_managers, + ext_capture: None, + portal_available, active_output: None, active_output_id: None, active_geometry: None, capture: None, + direct_capture: None, image: None, image_target_dimensions: None, image_generation: 0, @@ -77,14 +166,25 @@ impl FrozenState { portal_target_output_id: None, runtime_wake, preflight_pending: false, - preflight_use_fallback: false, + preflight_backend: None, capture_done: false, pending_image: None, } } - pub fn manager_available(&self) -> bool { - self.manager.is_some() + pub(in crate::backend::wayland) fn preferred_backend(&self) -> Option { + select_capture_backend( + self.manager.is_some(), + self.ext_managers.is_some(), + self.portal_available, + ) + } + + pub(super) fn next_backend_after( + &self, + failed: FrozenCaptureBackend, + ) -> Option { + next_capture_backend_after(failed, self.ext_managers.is_some(), self.portal_available) } pub fn set_active_output(&mut self, output: Option, id: Option) { @@ -123,6 +223,22 @@ impl FrozenState { self.pending_image = Some(PendingFrozenImage { image, source_geometry, + output_transform: None, + needs_output_transform: true, + source: FrozenCaptureSource::ActiveOutput, + }); + } + + pub(super) fn set_pending_output_image_with_transform( + &mut self, + image: FrozenImage, + source_geometry: Option, + output_transform: Option, + ) { + self.pending_image = Some(PendingFrozenImage { + image, + source_geometry, + output_transform, needs_output_transform: true, source: FrozenCaptureSource::ActiveOutput, }); @@ -136,6 +252,7 @@ impl FrozenState { self.pending_image = Some(PendingFrozenImage { image, source_geometry, + output_transform: None, needs_output_transform: false, source: FrozenCaptureSource::Desktop, }); @@ -147,19 +264,21 @@ impl FrozenState { pub fn is_in_progress(&self) -> bool { self.capture.is_some() + || self.ext_capture.is_some() + || self.direct_capture.is_some() || self.portal_in_progress || self.preflight_pending || self.pending_image.is_some() } - pub fn take_preflight_pending(&mut self) -> Option { + pub(in crate::backend::wayland) fn take_preflight_pending( + &mut self, + ) -> Option { if !self.preflight_pending { return None; } - let use_fallback = self.preflight_use_fallback; self.preflight_pending = false; - self.preflight_use_fallback = false; - Some(use_fallback) + self.preflight_backend.take() } pub fn take_capture_done(&mut self) -> bool { @@ -168,6 +287,40 @@ impl FrozenState { done } + pub(in crate::backend::wayland) fn direct_capture_timeout( + &self, + now: Instant, + ) -> Option { + self.direct_capture + .as_ref() + .map(|capture| capture.timeout(now)) + } + + pub(in crate::backend::wayland) fn take_timed_out_direct_capture( + &mut self, + now: Instant, + ) -> Option { + let capture = self.direct_capture.as_ref()?; + if !capture.timeout(now).is_zero() { + return None; + } + let backend = capture.backend; + self.direct_capture = None; + match backend { + DirectCaptureBackend::WlrScreencopy => { + if let Some(capture) = self.capture.take() { + capture.frame.destroy(); + } + } + DirectCaptureBackend::ExtImageCopy => { + if let Some(capture) = self.ext_capture.take() { + capture.destroy(); + } + } + } + Some(backend.capture_backend()) + } + pub fn activate_pending_image( &mut self, phys_width: u32, @@ -180,12 +333,14 @@ impl FrozenState { let mut image = pending.image; if pending.needs_output_transform { - let output_transform = pending - .source_geometry - .as_ref() - .or(self.active_geometry.as_ref()) - .map(|geo| geo.transform) - .unwrap_or(wl_output::Transform::Normal); + let output_transform = pending.output_transform.unwrap_or_else(|| { + pending + .source_geometry + .as_ref() + .or(self.active_geometry.as_ref()) + .map(|geo| geo.transform) + .unwrap_or(wl_output::Transform::Normal) + }); image = image.with_output_transform(output_transform); } @@ -278,8 +433,12 @@ impl FrozenState { if let Some(capture) = self.capture.take() { capture.frame.destroy(); } + if let Some(capture) = self.ext_capture.take() { + capture.destroy(); + } + self.direct_capture = None; self.preflight_pending = false; - self.preflight_use_fallback = false; + self.preflight_backend = None; self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { task.cancel(); @@ -305,11 +464,115 @@ impl FrozenState { } } +fn select_capture_backend( + wlr_screencopy: bool, + ext_image_copy: bool, + portal: bool, +) -> Option { + if wlr_screencopy { + Some(FrozenCaptureBackend::WlrScreencopy) + } else if ext_image_copy { + Some(FrozenCaptureBackend::ExtImageCopy) + } else if portal { + Some(FrozenCaptureBackend::Portal) + } else { + None + } +} + +fn next_capture_backend_after( + failed: FrozenCaptureBackend, + ext_image_copy: bool, + portal: bool, +) -> Option { + match failed { + FrozenCaptureBackend::WlrScreencopy if ext_image_copy => { + Some(FrozenCaptureBackend::ExtImageCopy) + } + FrozenCaptureBackend::WlrScreencopy | FrozenCaptureBackend::ExtImageCopy if portal => { + Some(FrozenCaptureBackend::Portal) + } + FrozenCaptureBackend::WlrScreencopy + | FrozenCaptureBackend::ExtImageCopy + | FrozenCaptureBackend::Portal => None, + } +} + #[cfg(test)] mod tests { use super::*; use crate::input::state::test_support::make_test_input_state; + #[test] + fn capture_backend_priority_is_wlr_then_ext_then_portal() { + assert_eq!( + select_capture_backend(true, true, true), + Some(FrozenCaptureBackend::WlrScreencopy) + ); + assert_eq!( + select_capture_backend(false, true, true), + Some(FrozenCaptureBackend::ExtImageCopy) + ); + assert_eq!( + select_capture_backend(false, false, true), + Some(FrozenCaptureBackend::Portal) + ); + assert_eq!(select_capture_backend(false, false, false), None); + assert_eq!( + next_capture_backend_after(FrozenCaptureBackend::WlrScreencopy, true, true), + Some(FrozenCaptureBackend::ExtImageCopy) + ); + assert_eq!( + next_capture_backend_after(FrozenCaptureBackend::WlrScreencopy, false, true), + Some(FrozenCaptureBackend::Portal) + ); + assert_eq!( + next_capture_backend_after(FrozenCaptureBackend::ExtImageCopy, true, true), + Some(FrozenCaptureBackend::Portal) + ); + assert_eq!( + next_capture_backend_after(FrozenCaptureBackend::Portal, true, true), + None + ); + } + + #[test] + fn direct_capture_deadline_expires_and_keeps_its_backend_identity() { + let started_at = Instant::now(); + let mut state = FrozenState::new(None); + state.direct_capture = Some(DirectCaptureContext::new_at( + DirectCaptureBackend::ExtImageCopy, + 7, + None, + started_at, + )); + + assert_eq!( + state.direct_capture_timeout(started_at), + Some(DIRECT_CAPTURE_TIMEOUT) + ); + assert_eq!( + state.take_timed_out_direct_capture(started_at + DIRECT_CAPTURE_TIMEOUT), + Some(FrozenCaptureBackend::ExtImageCopy) + ); + assert!(state.direct_capture.is_none()); + assert_eq!(state.direct_capture_timeout(started_at), None); + } + + #[test] + fn direct_capture_context_rejects_a_different_or_missing_output() { + let capture = DirectCaptureContext::new_at( + DirectCaptureBackend::WlrScreencopy, + 7, + None, + Instant::now(), + ); + + assert!(capture.output_matches(Some(7))); + assert!(!capture.output_matches(Some(8))); + assert!(!capture.output_matches(None)); + } + #[test] fn active_output_capture_accepts_native_fractional_scale_dimensions() { let mut state = FrozenState::new(None); @@ -340,6 +603,29 @@ mod tests { assert!(!input_state.frozen_active()); } + #[test] + fn active_output_capture_uses_protocol_transform_without_output_geometry() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + state.set_pending_output_image_with_transform( + FrozenImage { + width: 2, + height: 1, + stride: 8, + data: vec![1, 0, 0, 255, 2, 0, 0, 255], + }, + None, + Some(wl_output::Transform::_90), + ); + + state + .activate_pending_image(1, 2, &mut input_state) + .expect("capture transform should orient the frozen image"); + + let image = state.image().expect("the frozen image should be active"); + assert_eq!((image.width, image.height), (1, 2)); + } + #[test] fn desktop_capture_still_requires_a_crop_covering_the_target() { let mut state = FrozenState::new(None); diff --git a/src/backend/wayland/handlers/ext_image_copy.rs b/src/backend/wayland/handlers/ext_image_copy.rs new file mode 100644 index 00000000..3014d11d --- /dev/null +++ b/src/backend/wayland/handlers/ext_image_copy.rs @@ -0,0 +1,92 @@ +// Dispatch handlers for ext-image-copy-capture objects used by frozen mode. +use log::debug; +use wayland_client::{Connection, Dispatch, QueueHandle}; +use wayland_protocols::ext::{ + image_capture_source::v1::client::{ + ext_image_capture_source_v1::{Event as SourceEvent, ExtImageCaptureSourceV1}, + ext_output_image_capture_source_manager_v1::{ + Event as OutputSourceManagerEvent, ExtOutputImageCaptureSourceManagerV1, + }, + }, + image_copy_capture::v1::client::{ + ext_image_copy_capture_frame_v1::{Event as FrameEvent, ExtImageCopyCaptureFrameV1}, + ext_image_copy_capture_manager_v1::{Event as ManagerEvent, ExtImageCopyCaptureManagerV1}, + ext_image_copy_capture_session_v1::{Event as SessionEvent, ExtImageCopyCaptureSessionV1}, + }, +}; + +use super::super::frozen::FrozenCaptureBackend; +use super::super::state::WaylandState; + +impl Dispatch for WaylandState { + fn event( + _state: &mut Self, + _proxy: &ExtImageCopyCaptureManagerV1, + event: ManagerEvent, + _data: &(), + _conn: &Connection, + _qh: &QueueHandle, + ) { + debug!("Ext-image-copy manager event ignored: {event:?}"); + } +} + +impl Dispatch for WaylandState { + fn event( + _state: &mut Self, + _proxy: &ExtOutputImageCaptureSourceManagerV1, + event: OutputSourceManagerEvent, + _data: &(), + _conn: &Connection, + _qh: &QueueHandle, + ) { + debug!("Ext output capture source manager event ignored: {event:?}"); + } +} + +impl Dispatch for WaylandState { + fn event( + _state: &mut Self, + _proxy: &ExtImageCaptureSourceV1, + event: SourceEvent, + _data: &(), + _conn: &Connection, + _qh: &QueueHandle, + ) { + debug!("Ext image capture source event ignored: {event:?}"); + } +} + +impl Dispatch for WaylandState { + fn event( + state: &mut Self, + _proxy: &ExtImageCopyCaptureSessionV1, + event: SessionEvent, + _data: &(), + _conn: &Connection, + qh: &QueueHandle, + ) { + let should_fallback = state.frozen.handle_ext_session_event(event, qh); + if should_fallback { + state.continue_frozen_capture_after_failure(FrozenCaptureBackend::ExtImageCopy, qh); + } + } +} + +impl Dispatch for WaylandState { + fn event( + state: &mut Self, + _proxy: &ExtImageCopyCaptureFrameV1, + event: FrameEvent, + _data: &(), + _conn: &Connection, + qh: &QueueHandle, + ) { + if state + .frozen + .handle_ext_frame_event(event, qh, &mut state.input_state) + { + state.continue_frozen_capture_after_failure(FrozenCaptureBackend::ExtImageCopy, qh); + } + } +} diff --git a/src/backend/wayland/handlers/mod.rs b/src/backend/wayland/handlers/mod.rs index 25dcca2e..fafdf7ac 100644 --- a/src/backend/wayland/handlers/mod.rs +++ b/src/backend/wayland/handlers/mod.rs @@ -25,6 +25,7 @@ delegate_xdg_window!(WaylandState); mod activation; mod buffer; mod compositor; +mod ext_image_copy; pub(in crate::backend::wayland) mod keyboard; mod layer; mod output; diff --git a/src/backend/wayland/handlers/screencopy.rs b/src/backend/wayland/handlers/screencopy.rs index 7397307d..ba4fd827 100644 --- a/src/backend/wayland/handlers/screencopy.rs +++ b/src/backend/wayland/handlers/screencopy.rs @@ -6,6 +6,7 @@ use wayland_protocols_wlr::screencopy::v1::client::{ zwlr_screencopy_manager_v1::{Event as ManagerEvent, ZwlrScreencopyManagerV1}, }; +use super::super::frozen::FrozenCaptureBackend; use super::super::state::WaylandState; impl Dispatch for WaylandState { @@ -28,14 +29,15 @@ impl Dispatch for WaylandState { event: FrameEvent, _data: &(), _conn: &Connection, - _qh: &QueueHandle, + qh: &QueueHandle, ) { if state.zoom.is_in_progress() { state.zoom.handle_frame_event(event, &mut state.input_state); - } else { - state - .frozen - .handle_frame_event(event, &mut state.input_state); + } else if state + .frozen + .handle_frame_event(event, &mut state.input_state) + { + state.continue_frozen_capture_after_failure(FrozenCaptureBackend::WlrScreencopy, qh); } } } diff --git a/src/backend/wayland/portal_capture.rs b/src/backend/wayland/portal_capture.rs index 81b30dbf..42ac6131 100644 --- a/src/backend/wayland/portal_capture.rs +++ b/src/backend/wayland/portal_capture.rs @@ -22,18 +22,18 @@ pub(crate) fn screenshot_portal_available(_runtime: &tokio::runtime::Runtime) -> } #[cfg(feature = "portal")] -pub(crate) async fn capture_via_portal_fullscreen_bytes() -> Result, String> { +pub(crate) async fn capture_via_portal_fullscreen_bytes() +-> Result, crate::capture::types::CaptureError> { use crate::capture::sources::portal::capture_via_portal_bytes; use crate::capture::types::CaptureType; - capture_via_portal_bytes(CaptureType::FullScreen) - .await - .map_err(|error| format!("Portal capture failed: {error}")) + capture_via_portal_bytes(CaptureType::FullScreen).await } #[cfg(not(feature = "portal"))] -pub(crate) async fn capture_via_portal_fullscreen_bytes() -> Result, String> { - Err("Portal capture is disabled (feature flag)".to_string()) +pub(crate) async fn capture_via_portal_fullscreen_bytes() +-> Result, crate::capture::types::CaptureError> { + Err(crate::capture::types::CaptureError::PortalUnavailable) } pub(crate) const fn portal_output_matches(target: Option, current: Option) -> bool { diff --git a/src/backend/wayland/state.rs b/src/backend/wayland/state.rs index 0552f901..79a60fee 100644 --- a/src/backend/wayland/state.rs +++ b/src/backend/wayland/state.rs @@ -78,7 +78,7 @@ use super::{ ClipboardOperationController, ClipboardOperationIdSource, ClipboardPasteCompletion, ClipboardPublishCompletion, }, - frozen::FrozenState, + frozen::{ExtImageCopyManagers, FrozenState}, overlay_passthrough::set_surface_clickthrough, session::SessionState, surface::SurfaceState, @@ -170,6 +170,8 @@ pub(in crate::backend::wayland) struct WaylandStateInit { pub main_surface_uses_overlay_layer: bool, pub pending_freeze_on_start: bool, pub screencopy_manager: Option, + pub ext_image_copy_managers: Option, + pub portal_freeze_supported: bool, pub text_input_manager: Option, #[cfg(feature = "tablet-input")] pub tablet_manager: Option, diff --git a/src/backend/wayland/state/capture.rs b/src/backend/wayland/state/capture.rs index 5f19e3d2..a5b88550 100644 --- a/src/backend/wayland/state/capture.rs +++ b/src/backend/wayland/state/capture.rs @@ -1,4 +1,5 @@ use super::*; +use crate::backend::wayland::frozen::FrozenCaptureBackend; use crate::capture::{CaptureRequest, CaptureRequestId, CaptureSubmitError}; use crate::input::state::{Toast, ToastPriority}; @@ -18,6 +19,20 @@ fn should_exit_after_capture(mode: ExitAfterCaptureMode, destination: CaptureDes } impl WaylandState { + pub(in crate::backend::wayland) fn continue_frozen_capture_after_failure( + &mut self, + failed_backend: FrozenCaptureBackend, + qh: &QueueHandle, + ) { + if let Err(error) = + self.frozen + .begin_fallback_capture(failed_backend, &self.shm, qh, &self.tokio_handle) + { + log::warn!("No frozen capture fallback succeeded after {failed_backend:?}: {error:#}"); + self.frozen.cancel(&mut self.input_state); + } + } + fn should_exit_after_capture(&self, destination: CaptureDestination) -> bool { should_exit_after_capture(self.exit_after_capture_mode, destination) } diff --git a/src/backend/wayland/state/capture/barrier.rs b/src/backend/wayland/state/capture/barrier.rs index bc6b5f78..9b10dd7a 100644 --- a/src/backend/wayland/state/capture/barrier.rs +++ b/src/backend/wayland/state/capture/barrier.rs @@ -319,17 +319,15 @@ impl WaylandState { match reason { OverlaySuppression::Frozen => { - let Some(use_fallback) = self.frozen.take_preflight_pending() else { + let Some(backend) = self.frozen.take_preflight_pending() else { log::warn!("Frozen capture barrier completed without a pending preflight"); self.cancel_overlay_capture_preflight(reason); return; }; - if let Err(err) = self.frozen.begin_preflight_capture( - use_fallback, - &self.shm, - qh, - &self.tokio_handle, - ) { + if let Err(err) = + self.frozen + .begin_preflight_capture(backend, &self.shm, qh, &self.tokio_handle) + { log::warn!("Frozen preflight capture failed: {err}"); self.frozen.cancel(&mut self.input_state); } diff --git a/src/backend/wayland/state/core/init.rs b/src/backend/wayland/state/core/init.rs index 61f2a018..7dd07fab 100644 --- a/src/backend/wayland/state/core/init.rs +++ b/src/backend/wayland/state/core/init.rs @@ -25,6 +25,8 @@ impl WaylandState { main_surface_uses_overlay_layer, pending_freeze_on_start, screencopy_manager, + ext_image_copy_managers, + portal_freeze_supported, text_input_manager, #[cfg(feature = "tablet-input")] tablet_manager, @@ -167,7 +169,12 @@ impl WaylandState { ui_animation_next_tick: None, ui_animation_interval, capture: CaptureState::new(capture_manager), - frozen: FrozenState::new_with_runtime_wake(screencopy_manager, runtime_wake.clone()), + frozen: FrozenState::new_with_backends( + screencopy_manager, + ext_image_copy_managers, + portal_freeze_supported, + runtime_wake.clone(), + ), zoom: ZoomState::new_with_runtime_wake(zoom_manager, runtime_wake.clone()), perf: perf::PerfMetrics::from_env(), exit_after_capture_mode, diff --git a/src/backend/wayland/zoom/mod.rs b/src/backend/wayland/zoom/mod.rs index 1bec1d31..ac649571 100644 --- a/src/backend/wayland/zoom/mod.rs +++ b/src/backend/wayland/zoom/mod.rs @@ -8,5 +8,7 @@ pub use state::ZoomState; const MIN_ZOOM_SCALE: f64 = 1.0; const MAX_ZOOM_SCALE: f64 = 8.0; -type PortalCaptureResult = - Result<(Option, crate::backend::wayland::frozen::FrozenImage), String>; +type PortalCaptureResult = Result< + (Option, crate::backend::wayland::frozen::FrozenImage), + crate::capture::types::CaptureError, +>; diff --git a/src/backend/wayland/zoom/portal.rs b/src/backend/wayland/zoom/portal.rs index 2944a863..f7c7e453 100644 --- a/src/backend/wayland/zoom/portal.rs +++ b/src/backend/wayland/zoom/portal.rs @@ -8,6 +8,7 @@ use crate::backend::wayland::portal_capture::{ }; use crate::backend::wayland::portal_task::{PortalPoll, PortalTask}; use crate::capture::sources::frozen::decode_image_to_argb; +use crate::capture::types::CaptureError; use crate::input::InputState; use super::state::ZoomState; @@ -41,8 +42,8 @@ impl ZoomState { async { let bytes = capture_via_portal_fullscreen_bytes().await?; - let (mut data, mut width, mut height) = - decode_image_to_argb(&bytes).map_err(|e| format!("Decode failed: {}", e))?; + let (mut data, mut width, mut height) = decode_image_to_argb(&bytes) + .map_err(|error| CaptureError::ImageError(format!("Decode failed: {error}")))?; if let Some(geo) = geo { let (phys_w, phys_h) = geo.physical_size(); @@ -136,10 +137,18 @@ impl ZoomState { input_state.needs_redraw = true; self.capture_done = true; } - PortalPoll::Ready(Err(err)) | PortalPoll::Failed(err) => { + PortalPoll::Ready(Err(CaptureError::Cancelled(reason))) => { + log::info!("Portal zoom capture cancelled: {reason}"); + self.finish_failed_portal_task(input_state); + } + PortalPoll::Ready(Err(err)) => { warn!("Portal zoom capture failed: {err}"); self.finish_failed_portal_task(input_state); } + PortalPoll::Failed(err) => { + warn!("Portal zoom capture task failed: {err}"); + self.finish_failed_portal_task(input_state); + } PortalPoll::Pending => {} PortalPoll::Disconnected => { warn!("Portal zoom capture channel disconnected"); @@ -230,7 +239,7 @@ mod tests { }) } else { PortalTask::spawn(&tokio::runtime::Handle::current(), wake.handle(), async { - Err("portal denied".to_string()) + Err(CaptureError::PermissionDenied) }) }); zoom.portal_in_progress = true; diff --git a/src/capture/portal.rs b/src/capture/portal.rs index 3118152d..d53aba3a 100644 --- a/src/capture/portal.rs +++ b/src/capture/portal.rs @@ -2,9 +2,15 @@ use super::types::{CaptureError, CaptureType}; use std::collections::HashMap; -use zbus::zvariant::OwnedValue; +use zbus::zvariant::{OwnedObjectPath, OwnedValue}; use zbus::{Connection, proxy}; +const PORTAL_DESTINATION: &str = "org.freedesktop.portal.Desktop"; +const PORTAL_REQUEST_PATH_PREFIX: &str = "/org/freedesktop/portal/desktop/request"; +const PORTAL_OPTION_HANDLE_TOKEN_KEY: &str = "handle_token"; +const PORTAL_HANDLE_RANDOM_BYTES: usize = 16; +const LOWERCASE_HEX: &[u8; 16] = b"0123456789abcdef"; + /// D-Bus proxy for the xdg-desktop-portal Screenshot interface. #[proxy( interface = "org.freedesktop.portal.Screenshot", @@ -12,6 +18,10 @@ use zbus::{Connection, proxy}; default_path = "/org/freedesktop/portal/desktop" )] trait Screenshot { + /// Maximum Screenshot interface version supported by the selected portal backend. + #[zbus(property, name = "version")] + fn version(&self) -> zbus::Result; + /// Take a screenshot. /// /// # Arguments @@ -87,6 +97,7 @@ pub async fn capture_via_portal(capture_type: CaptureType) -> Result return Ok(uri), + Err(err @ CaptureError::Cancelled(_)) => return Err(err), Err(err) => { log::warn!("Portal capture attempt '{}' failed: {}", attempt.label, err); last_error = Some(err); @@ -117,52 +128,129 @@ fn portal_attempts(capture_type: CaptureType) -> Vec { async fn capture_once( connection: &Connection, proxy: &ScreenshotProxy<'_>, - options: HashMap>, + mut options: HashMap>, ) -> Result { + let handle_token = next_handle_token()?; + let request_path = portal_request_path(connection, &handle_token)?; + options.insert( + PORTAL_OPTION_HANDLE_TOKEN_KEY.to_string(), + handle_token.into(), + ); log::debug!("Calling portal screenshot with options: {:?}", options); - // Call screenshot method - this returns a Request object path. - let request_path = proxy - .screenshot("", options) - .await - .map_err(map_portal_call_error)?; - - log::info!("Screenshot request created: {:?}", request_path); - - // Create a proxy for the Request object to receive Response signal. + // Portal backends may complete non-interactive screenshots before the + // Screenshot method reply reaches us. Subscribe at the predicted request + // path first so that fast Response signals cannot be lost. let request_proxy = RequestProxy::builder(connection) - .path(request_path) + .destination(PORTAL_DESTINATION) + .map_err(CaptureError::DBusError)? + .path(request_path.clone()) .map_err(CaptureError::DBusError)? .build() .await .map_err(CaptureError::DBusError)?; - - // Wait for the Response signal. let mut response_stream = request_proxy .receive_response() .await .map_err(CaptureError::DBusError)?; + let returned_path = proxy + .screenshot("", options) + .await + .map_err(map_portal_call_error)?; + + log::info!("Screenshot request created: {:?}", returned_path); log::debug!("Waiting for Response signal..."); - // Get the first (and only) response. - let response_signal = crate::zbus_stream::next(&mut response_stream) - .await - .ok_or_else(|| CaptureError::InvalidResponse("No Response signal received".to_string()))?; + // Most portals honor handle_token, which lets us install the signal match + // before calling Screenshot. Older implementations may return a different + // path; switch to that path as required by the Request compatibility + // contract instead of rejecting an otherwise valid request. + let response_signal = if returned_path == request_path { + crate::zbus_stream::next(&mut response_stream).await + } else { + log::warn!( + "Screenshot portal returned a different request path; updating Response subscription" + ); + let returned_request_proxy = RequestProxy::builder(connection) + .destination(PORTAL_DESTINATION) + .map_err(CaptureError::DBusError)? + .path(returned_path) + .map_err(CaptureError::DBusError)? + .build() + .await + .map_err(CaptureError::DBusError)?; + let mut returned_response_stream = returned_request_proxy + .receive_response() + .await + .map_err(CaptureError::DBusError)?; + crate::zbus_stream::next(&mut returned_response_stream).await + } + .ok_or_else(|| CaptureError::InvalidResponse("No Response signal received".to_string()))?; let args = response_signal.args().map_err(|e| { CaptureError::InvalidResponse(format!("Failed to parse response args: {}", e)) })?; log::debug!( - "Response signal received: code={}, results={:?}", + "Response signal received: code={}, result_keys={:?}", args.response, - args.results + args.results.keys().collect::>() ); parse_response(args.response, &args.results) } +fn next_handle_token() -> Result { + let mut random = [0_u8; PORTAL_HANDLE_RANDOM_BYTES]; + getrandom::fill(&mut random).map_err(|error| { + CaptureError::InvalidResponse(format!( + "Failed to generate a secure portal handle token: {error}" + )) + })?; + Ok(handle_token_from_random(&random)) +} + +fn handle_token_from_random(random: &[u8; PORTAL_HANDLE_RANDOM_BYTES]) -> String { + let mut token = String::with_capacity("wayscriber_".len() + random.len() * 2); + token.push_str("wayscriber_"); + for byte in random { + token.push(char::from(LOWERCASE_HEX[usize::from(byte >> 4)])); + token.push(char::from(LOWERCASE_HEX[usize::from(byte & 0x0f)])); + } + token +} + +fn portal_request_path( + connection: &Connection, + handle_token: &str, +) -> Result { + let unique_name = connection.unique_name().ok_or_else(|| { + CaptureError::InvalidResponse("Session bus connection has no unique D-Bus name".to_string()) + })?; + portal_request_path_for_unique_name(unique_name.as_str(), handle_token) +} + +fn portal_request_path_for_unique_name( + unique_name: &str, + handle_token: &str, +) -> Result { + if handle_token.is_empty() + || !handle_token + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'_') + { + return Err(CaptureError::InvalidResponse( + "Portal handle token is not a valid D-Bus object-path element".to_string(), + )); + } + let sender = unique_name.trim_start_matches(':').replace('.', "_"); + OwnedObjectPath::try_from(format!( + "{PORTAL_REQUEST_PATH_PREFIX}/{sender}/{handle_token}" + )) + .map_err(|error| CaptureError::InvalidResponse(format!("Invalid portal request path: {error}"))) +} + fn parse_response( response_code: u32, results: &HashMap, @@ -182,12 +270,14 @@ fn parse_response( CaptureError::InvalidResponse(format!("URI is not a string: {}", e)) })?; - log::info!("Screenshot captured successfully: {}", uri_str); + log::info!("Screenshot captured successfully"); Ok(uri_str.to_string()) } 1 => { - log::warn!("Screenshot cancelled by user"); - Err(CaptureError::PermissionDenied) + log::info!("Screenshot cancelled by user"); + Err(CaptureError::Cancelled( + "portal screenshot request was cancelled by the user".to_string(), + )) } code => { log::error!("Screenshot failed with code {}", code); @@ -200,11 +290,16 @@ fn parse_response( } fn map_portal_call_error(err: zbus::Error) -> CaptureError { - log::error!("Portal screenshot call failed: {}", err); let message = err.to_string(); - if message.contains("Cancelled") || message.contains("denied") { + let lowercase_message = message.to_ascii_lowercase(); + if lowercase_message.contains("cancelled") || lowercase_message.contains("canceled") { + log::info!("Portal screenshot call was cancelled"); + CaptureError::Cancelled("portal screenshot request was cancelled".to_string()) + } else if lowercase_message.contains("denied") { + log::warn!("Portal screenshot permission was denied"); CaptureError::PermissionDenied } else { + log::error!("Portal screenshot call failed: {err}"); CaptureError::DBusError(err) } } @@ -239,12 +334,13 @@ fn build_active_window_interactive_options() -> HashMap bool { match Connection::session().await { Ok(connection) => { - // Try to create the proxy. - ScreenshotProxy::new(&connection).await.is_ok() + let Ok(proxy) = ScreenshotProxy::new(&connection).await else { + return false; + }; + proxy.version().await.is_ok() } Err(_) => false, } @@ -302,4 +398,54 @@ mod tests { Some(&zbus::zvariant::Value::from(true)) ); } + + #[test] + fn portal_request_path_uses_the_dbus_unique_name_and_handle_token() { + assert_eq!( + portal_request_path_for_unique_name(":1.42", "wayscriber_7_9") + .expect("valid request path") + .as_str(), + "/org/freedesktop/portal/desktop/request/1_42/wayscriber_7_9" + ); + } + + #[test] + fn portal_request_path_rejects_an_invalid_handle_token() { + assert!( + portal_request_path_for_unique_name(":1.42", "invalid/token").is_err(), + "a handle token must remain one D-Bus object-path element" + ); + } + + #[test] + fn portal_handle_token_is_a_128_bit_random_object_path_element() { + let token = handle_token_from_random(&[ + 0x00, 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0xfe, 0xdc, 0xba, 0x98, 0x76, + 0x54, 0x32, + ]); + + assert_eq!(token, "wayscriber_000123456789abcdeffedcba98765432"); + assert!(portal_request_path_for_unique_name(":1.42", &token).is_ok()); + } + + #[test] + fn generated_portal_handle_tokens_have_independent_random_suffixes() { + let first = next_handle_token().expect("generate first secure token"); + let second = next_handle_token().expect("generate second secure token"); + + assert_ne!(first, second); + assert_eq!( + first.len(), + "wayscriber_".len() + PORTAL_HANDLE_RANDOM_BYTES * 2 + ); + assert_eq!(second.len(), first.len()); + assert!(portal_request_path_for_unique_name(":1.42", &first).is_ok()); + assert!(portal_request_path_for_unique_name(":1.42", &second).is_ok()); + } + + #[test] + fn portal_response_code_one_preserves_user_cancellation() { + let error = parse_response(1, &HashMap::new()).expect_err("response 1 must cancel"); + assert!(matches!(error, CaptureError::Cancelled(_))); + } } diff --git a/src/capture/sources/portal.rs b/src/capture/sources/portal.rs index 840d00ed..b9d6f8c9 100644 --- a/src/capture/sources/portal.rs +++ b/src/capture/sources/portal.rs @@ -8,7 +8,7 @@ use super::reader::read_image_from_uri; /// Capture using xdg-desktop-portal and return image bytes without blocking the Tokio runtime. pub async fn capture_via_portal_bytes(capture_type: CaptureType) -> Result, CaptureError> { let uri = portal::capture_via_portal(capture_type).await?; - log::info!("Portal returned URI: {}", uri); + log::info!("Portal returned a screenshot URI"); tokio::task::spawn_blocking(move || read_image_from_uri(&uri)) .await diff --git a/src/input/state/core/base/types.rs b/src/input/state/core/base/types.rs index 8a80f6f9..99d8214f 100644 --- a/src/input/state/core/base/types.rs +++ b/src/input/state/core/base/types.rs @@ -377,6 +377,7 @@ pub(crate) enum HistoryMode { pub struct CompositorCapabilities { pub layer_shell: bool, pub screencopy: bool, + pub image_copy_capture: bool, pub freeze_capture: bool, pub pointer_constraints: bool, pub desktop_environment: DesktopEnvironment, @@ -399,8 +400,12 @@ pub enum ShellMode { } impl CompositorCapabilities { + pub fn direct_capture_available(&self) -> bool { + self.screencopy || self.image_copy_capture + } + pub fn all_available(&self) -> bool { - self.layer_shell && self.screencopy && self.pointer_constraints + self.layer_shell && self.direct_capture_available() && self.pointer_constraints } pub fn limitations_summary(&self) -> Option { @@ -410,7 +415,7 @@ impl CompositorCapabilities { } if !self.freeze_capture { issues.push("Freeze unavailable"); - } else if !self.screencopy { + } else if !self.direct_capture_available() { issues.push("Freeze uses portal capture"); } if !self.pointer_constraints { diff --git a/src/input/state/core/base/types/tests.rs b/src/input/state/core/base/types/tests.rs index 3a09b86f..3b1e4f4c 100644 --- a/src/input/state/core/base/types/tests.rs +++ b/src/input/state/core/base/types/tests.rs @@ -6,6 +6,7 @@ fn compositor_capabilities_limitations_summary_returns_none_when_fully_available CompositorCapabilities { layer_shell: true, screencopy: true, + image_copy_capture: false, freeze_capture: true, pointer_constraints: true, desktop_environment: Default::default(), @@ -22,6 +23,7 @@ fn compositor_capabilities_limitations_summary_lists_missing_features_in_order() CompositorCapabilities { layer_shell: false, screencopy: true, + image_copy_capture: false, freeze_capture: true, pointer_constraints: false, desktop_environment: Default::default(), @@ -39,6 +41,7 @@ fn compositor_capabilities_reports_portal_freeze_without_hiding_limitations() { let caps = CompositorCapabilities { layer_shell: true, screencopy: false, + image_copy_capture: false, freeze_capture: true, pointer_constraints: true, desktop_environment: Default::default(), @@ -51,3 +54,20 @@ fn compositor_capabilities_reports_portal_freeze_without_hiding_limitations() { Some("Freeze uses portal capture".to_string()) ); } + +#[test] +fn compositor_capabilities_accepts_ext_image_copy_as_direct_capture() { + let caps = CompositorCapabilities { + layer_shell: true, + screencopy: false, + image_copy_capture: true, + freeze_capture: true, + pointer_constraints: true, + desktop_environment: Default::default(), + shell_mode: Default::default(), + }; + + assert!(caps.direct_capture_available()); + assert!(caps.all_available()); + assert_eq!(caps.limitations_summary(), None); +} diff --git a/tools/check-nixpkgs-recipe.py b/tools/check-nixpkgs-recipe.py index 9ced443c..07d83b17 100755 --- a/tools/check-nixpkgs-recipe.py +++ b/tools/check-nixpkgs-recipe.py @@ -40,6 +40,7 @@ "anyhow": (), "cairo-rs": ("cairo",), "flate2": (), + "getrandom": (), "glib": (), "gtk4": ("gtk4",), "gtk4-layer-shell": ("gtk4-layer-shell",), From 7c3dc6459f69c65ea6dafb6538b60d408049e936 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Mon, 3 Aug 2026 13:01:13 +0200 Subject: [PATCH 2/2] fix(freeze): harden capture lifecycle and fallback --- src/backend/wayland/frozen/capture.rs | 103 +++++++----- src/backend/wayland/frozen/ext_image_copy.rs | 138 +++++++-------- src/backend/wayland/frozen/image.rs | 67 ++++++++ src/backend/wayland/frozen/portal.rs | 60 ++++--- src/backend/wayland/frozen/state.rs | 166 ++++++++++--------- src/capture/portal.rs | 8 +- src/capture/sources/reader.rs | 20 +-- src/file_uri.rs | 25 ++- 8 files changed, 348 insertions(+), 239 deletions(-) diff --git a/src/backend/wayland/frozen/capture.rs b/src/backend/wayland/frozen/capture.rs index de240142..9e65c465 100644 --- a/src/backend/wayland/frozen/capture.rs +++ b/src/backend/wayland/frozen/capture.rs @@ -23,8 +23,8 @@ use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::Z use crate::input::InputState; -use super::image::copy_shm_argb; -use super::state::{DirectCaptureBackend, DirectCaptureContext, FrozenCaptureBackend, FrozenState}; +use super::image::{copy_shm_argb, validate_shm_buffer_layout}; +use super::state::{DirectCaptureAttempt, DirectCaptureContext, FrozenCaptureBackend, FrozenState}; /// Internal capture session tracking a single screencopy frame. pub(super) struct CaptureSession { @@ -81,12 +81,7 @@ impl CaptureSession { impl FrozenState { /// Start a screencopy capture for the active output. pub fn start_capture(&mut self) -> Result<()> { - if self.capture.is_some() - || self.ext_capture.is_some() - || self.direct_capture.is_some() - || self.portal_in_progress - || self.preflight_pending - { + if self.direct_capture.is_some() || self.portal_in_progress || self.preflight_pending { warn!("Frozen-mode capture already in progress; ignoring toggle"); return Ok(()); } @@ -239,18 +234,24 @@ impl FrozenState { let frame = manager.capture_output(0, &output, qh, ()); let mut capture = CaptureSession::new(frame); capture.pool = Some(pool); - self.capture = Some(capture); - self.direct_capture = Some(DirectCaptureContext::new( - DirectCaptureBackend::WlrScreencopy, - target_output_id, - source_geometry, - )); + self.direct_capture = Some(DirectCaptureAttempt::WlrScreencopy { + session: Box::new(capture), + context: DirectCaptureContext::new(target_output_id, source_geometry), + }); Ok(()) } /// Handle screencopy frame events. pub fn handle_frame_event(&mut self, event: FrameEvent, input_state: &mut InputState) -> bool { + if !matches!( + self.direct_capture.as_ref(), + Some(DirectCaptureAttempt::WlrScreencopy { .. }) + ) { + debug!("Ignoring screencopy frame event without an active WLR frozen capture"); + return false; + } + match event { FrameEvent::Buffer { format, @@ -274,7 +275,10 @@ impl FrozenState { } } FrameEvent::Flags { flags } => { - if let Some(capture) = self.capture.as_mut() { + if let Some(DirectCaptureAttempt::WlrScreencopy { + session: capture, .. + }) = self.direct_capture.as_mut() + { let raw_flags = match flags { WEnum::Value(v) => v.bits(), WEnum::Unknown(raw) => raw, @@ -292,7 +296,10 @@ impl FrozenState { } Err(err) => { warn!("Frozen capture ready handling failed: {}", err); - return self.fail_wlr_capture(); + // `on_ready` owns and destroys the completed WLR attempt, + // including error paths, so only the fallback decision + // remains here. + return true; } }, FrameEvent::Failed => { @@ -305,11 +312,19 @@ impl FrozenState { } fn fail_wlr_capture(&mut self) -> bool { - if let Some(capture) = self.capture.take() { - capture.frame.destroy(); + let Some(capture) = self.direct_capture.take() else { + return false; + }; + match capture { + DirectCaptureAttempt::WlrScreencopy { session, .. } => { + session.frame.destroy(); + true + } + capture @ DirectCaptureAttempt::ExtImageCopy { .. } => { + self.direct_capture = Some(capture); + false + } } - self.direct_capture = None; - true } fn on_buffer( @@ -319,10 +334,10 @@ impl FrozenState { height: u32, stride: u32, ) -> Result<()> { - let capture = self - .capture - .as_mut() - .context("No capture session present for buffer event")?; + let capture = match self.direct_capture.as_mut() { + Some(DirectCaptureAttempt::WlrScreencopy { session, .. }) => session, + _ => anyhow::bail!("No WLR capture session present for buffer event"), + }; let format = match format { WEnum::Value(fmt) => fmt, @@ -331,19 +346,19 @@ impl FrozenState { } }; + let layout = validate_shm_buffer_layout(width, height, stride)?; capture.width = width; capture.height = height; - capture.stride = stride as i32; + capture.stride = layout.stride; capture.format = Some(format); // Resize pool and create buffer let pool = capture.pool.as_mut().context("Capture pool missing")?; - let total_size = (capture.stride as usize) * (height as usize); - if total_size > pool.len() { - pool.resize(total_size)?; + if layout.total_size > pool.len() { + pool.resize(layout.total_size)?; } let (buffer, _) = pool - .create_buffer(width as i32, height as i32, capture.stride, format) + .create_buffer(layout.width, layout.height, layout.stride, format) .context("Failed to create capture buffer")?; capture.buffer = Some(buffer); capture.request_copy(); @@ -351,19 +366,26 @@ impl FrozenState { } fn on_buffer_done(&mut self) -> Result<()> { - let capture = self - .capture - .as_mut() - .context("No capture session present for buffer_done")?; + let capture = match self.direct_capture.as_mut() { + Some(DirectCaptureAttempt::WlrScreencopy { session, .. }) => session, + _ => anyhow::bail!("No WLR capture session present for buffer_done"), + }; capture.request_copy(); Ok(()) } fn on_ready(&mut self) -> Result { - let mut capture = self - .capture + let attempt = self + .direct_capture .take() - .context("No capture session present for ready event")?; + .context("No WLR capture attempt present for ready event")?; + let (mut capture, context) = match attempt { + DirectCaptureAttempt::WlrScreencopy { session, context } => (session, context), + attempt @ DirectCaptureAttempt::ExtImageCopy { .. } => { + self.direct_capture = Some(attempt); + anyhow::bail!("No WLR capture attempt present for ready event"); + } + }; let result = (|| { let pool = capture.pool.as_mut().context("Capture pool missing")?; let buffer = capture.buffer.as_ref().context("Capture buffer missing")?; @@ -382,18 +404,11 @@ impl FrozenState { })(); capture.frame.destroy(); let image = result?; - let context = self - .direct_capture - .take() - .context("WLR direct capture context missing")?; - if context.backend != DirectCaptureBackend::WlrScreencopy { - anyhow::bail!("WLR capture completed with a mismatched direct backend context"); - } if !context.output_matches(self.active_output_id) { return Ok(false); } - self.set_pending_output_image(image, context.source_geometry); + self.set_pending_output_image(image, context.target_output_id, context.source_geometry); Ok(true) } diff --git a/src/backend/wayland/frozen/ext_image_copy.rs b/src/backend/wayland/frozen/ext_image_copy.rs index 52512e06..b42b995c 100644 --- a/src/backend/wayland/frozen/ext_image_copy.rs +++ b/src/backend/wayland/frozen/ext_image_copy.rs @@ -24,8 +24,8 @@ use wayland_protocols::ext::{ use crate::input::InputState; -use super::image::copy_shm_argb; -use super::state::{DirectCaptureBackend, DirectCaptureContext, FrozenState}; +use super::image::{copy_shm_argb, validate_shm_buffer_layout}; +use super::state::{DirectCaptureAttempt, DirectCaptureContext, FrozenState}; const MAX_CONSTRAINT_RETRIES: u8 = 2; @@ -173,12 +173,10 @@ impl FrozenState { let session = managers .capture .create_session(&source, Options::empty(), qh, ()); - self.ext_capture = Some(ExtImageCopySession::new(source, session, pool)); - self.direct_capture = Some(DirectCaptureContext::new( - DirectCaptureBackend::ExtImageCopy, - target_output_id, - source_geometry, - )); + self.direct_capture = Some(DirectCaptureAttempt::ExtImageCopy { + session: Box::new(ExtImageCopySession::new(source, session, pool)), + context: DirectCaptureContext::new(target_output_id, source_geometry), + }); debug!("Requested ext-image-copy capture constraints for active output"); Ok(()) } @@ -191,19 +189,21 @@ impl FrozenState { where State: Dispatch + 'static, { + if !matches!( + self.direct_capture.as_ref(), + Some(DirectCaptureAttempt::ExtImageCopy { .. }) + ) { + debug!("Ignoring ext-image-copy session event without an active ext capture"); + return false; + } + let result = match event { SessionEvent::BufferSize { width, height } => { - let capture = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing"); + let capture = self.ext_capture_mut(); capture.map(|capture| capture.constraints.record_size(width, height)) } SessionEvent::ShmFormat { format } => { - let capture = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing"); + let capture = self.ext_capture_mut(); capture.map(|capture| capture.constraints.record_format(format)) } SessionEvent::Done => self.finish_constraint_batch(qh), @@ -226,10 +226,7 @@ impl FrozenState { State: Dispatch + 'static, { let constraints = { - let capture = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing")?; + let capture = self.ext_capture_mut()?; capture.constraints.finish_batch(capture.frame.is_some()) }; if let Some(constraints) = constraints { @@ -248,54 +245,36 @@ impl FrozenState { where State: Dispatch + 'static, { - let capture = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing")?; + let capture = self.ext_capture_mut()?; if capture.frame.is_some() { anyhow::bail!("ext-image-copy frame submitted while another frame is pending"); } let (width, height) = constraints .size .context("compositor omitted the ext-image-copy buffer size")?; - if width == 0 || height == 0 { - anyhow::bail!("compositor advertised an empty ext-image-copy buffer"); - } let format = select_shm_format(&constraints.formats) .context("compositor did not advertise an ARGB/XRGB shared-memory format")?; let stride = width .checked_mul(4) - .and_then(|value| i32::try_from(value).ok()) .context("ext-image-copy buffer stride overflow")?; - let total_size = usize::try_from(stride) - .ok() - .and_then(|stride| { - usize::try_from(height) - .ok() - .and_then(|height| stride.checked_mul(height)) - }) - .context("ext-image-copy buffer size overflow")?; - let buffer_width = i32::try_from(width) - .context("ext-image-copy buffer width exceeds the Wayland limit")?; - let buffer_height = i32::try_from(height) - .context("ext-image-copy buffer height exceeds the Wayland limit")?; - if total_size > capture.pool.len() { - capture.pool.resize(total_size)?; + let layout = validate_shm_buffer_layout(width, height, stride)?; + if layout.total_size > capture.pool.len() { + capture.pool.resize(layout.total_size)?; } let (buffer, _) = capture .pool - .create_buffer(buffer_width, buffer_height, stride, format) + .create_buffer(layout.width, layout.height, layout.stride, format) .context("Failed to create ext-image-copy buffer")?; let frame = capture.session.create_frame(qh, ()); frame.attach_buffer(buffer.wl_buffer()); - frame.damage_buffer(0, 0, buffer_width, buffer_height); + frame.damage_buffer(0, 0, layout.width, layout.height); frame.capture(); capture.frame = Some(ExtImageCopyFrame { proxy: frame, buffer, width, height, - stride, + stride: layout.stride, format, transform: None, }); @@ -311,6 +290,14 @@ impl FrozenState { where State: Dispatch + 'static, { + if !matches!( + self.direct_capture.as_ref(), + Some(DirectCaptureAttempt::ExtImageCopy { .. }) + ) { + debug!("Ignoring ext-image-copy frame event without an active ext capture"); + return false; + } + match event { FrameEvent::Ready => match self.finish_ext_frame() { Ok(true) => input_state.needs_redraw = true, @@ -352,7 +339,7 @@ impl FrozenState { return self.fail_ext_capture(); } FrameEvent::Transform { transform } => { - if let Some(capture) = self.ext_capture.as_mut() + if let Ok(capture) = self.ext_capture_mut() && let Some(frame) = capture.frame.as_mut() && let WEnum::Value(transform) = transform { @@ -370,10 +357,7 @@ impl FrozenState { State: Dispatch + 'static, { let replacement = { - let capture = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing")?; + let capture = self.ext_capture_mut()?; let frame = capture .frame .take() @@ -388,16 +372,10 @@ impl FrozenState { } fn finish_ext_frame(&mut self) -> Result { - let frame = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing")?; + let frame = self.ext_capture_mut()?; let frame = frame.frame.take().context("ext-image-copy frame missing")?; let result: Result<_> = (|| { - let capture = self - .ext_capture - .as_mut() - .context("ext-image-copy capture session missing")?; + let capture = self.ext_capture_mut()?; let canvas = frame .buffer .canvas(&mut capture.pool) @@ -414,23 +392,16 @@ impl FrozenState { })(); frame.proxy.destroy(); let (image, output_transform) = result?; - let capture = self - .ext_capture - .take() - .context("ext-image-copy capture session missing after frame completion")?; - capture.destroy(); - let context = self - .direct_capture - .take() - .context("ext-image-copy direct capture context missing")?; - if context.backend != DirectCaptureBackend::ExtImageCopy { - anyhow::bail!("ext-image-copy completed with a mismatched direct backend context"); - } + let (capture, context) = self + .take_ext_capture() + .context("ext-image-copy capture attempt missing after frame completion")?; + (*capture).destroy(); if !context.output_matches(self.active_output_id) { return Ok(false); } self.set_pending_output_image_with_transform( image, + context.target_output_id, context.source_geometry, output_transform, ); @@ -438,11 +409,30 @@ impl FrozenState { } fn fail_ext_capture(&mut self) -> bool { - if let Some(capture) = self.ext_capture.take() { - capture.destroy(); + if let Some((capture, _)) = self.take_ext_capture() { + (*capture).destroy(); + true + } else { + false + } + } + + fn ext_capture_mut(&mut self) -> Result<&mut ExtImageCopySession> { + match self.direct_capture.as_mut() { + Some(DirectCaptureAttempt::ExtImageCopy { session, .. }) => Ok(session.as_mut()), + _ => anyhow::bail!("ext-image-copy capture session missing"), + } + } + + fn take_ext_capture(&mut self) -> Option<(Box, DirectCaptureContext)> { + let attempt = self.direct_capture.take()?; + match attempt { + DirectCaptureAttempt::ExtImageCopy { session, context } => Some((session, context)), + attempt @ DirectCaptureAttempt::WlrScreencopy { .. } => { + self.direct_capture = Some(attempt); + None + } } - self.direct_capture = None; - true } } diff --git a/src/backend/wayland/frozen/image.rs b/src/backend/wayland/frozen/image.rs index 440b646e..574f13f0 100644 --- a/src/backend/wayland/frozen/image.rs +++ b/src/backend/wayland/frozen/image.rs @@ -1,6 +1,53 @@ use anyhow::{Context, Result}; use wayland_client::protocol::{wl_output, wl_shm}; +pub(super) struct ShmBufferLayout { + pub(super) width: i32, + pub(super) height: i32, + pub(super) stride: i32, + pub(super) total_size: usize, +} + +/// Validate compositor-owned dimensions before allocating or creating a +/// `wl_buffer` from them. +pub(super) fn validate_shm_buffer_layout( + width: u32, + height: u32, + stride: u32, +) -> Result { + if width == 0 || height == 0 { + anyhow::bail!("Frozen capture advertised an empty SHM buffer"); + } + + let buffer_width = + i32::try_from(width).context("Frozen capture width exceeds the Wayland limit")?; + let buffer_height = + i32::try_from(height).context("Frozen capture height exceeds the Wayland limit")?; + let buffer_stride = + i32::try_from(stride).context("Frozen capture stride exceeds the Wayland limit")?; + let row_bytes = width + .checked_mul(4) + .context("Frozen capture row size overflow")?; + if stride < row_bytes { + anyhow::bail!("Frozen capture stride is smaller than its pixel row"); + } + let total_size = usize::try_from(stride) + .ok() + .and_then(|stride| { + usize::try_from(height) + .ok() + .and_then(|height| stride.checked_mul(height)) + }) + .context("Frozen capture buffer size overflow")?; + + Ok(ShmBufferLayout { + width: buffer_width, + height: buffer_height, + stride: buffer_stride, + total_size, + }) +} + /// CPU-side frozen image ready for Cairo rendering. pub struct FrozenImage { pub width: u32, @@ -241,4 +288,24 @@ mod tests { assert!(copy_shm_argb(&[1, 2, 3], 1, 1, 4, wl_shm::Format::Argb8888, false,).is_err()); } + + #[test] + fn shm_layout_rejects_invalid_compositor_dimensions_before_allocation() { + assert!(validate_shm_buffer_layout(0, 1, 4).is_err()); + assert!(validate_shm_buffer_layout(1, 0, 4).is_err()); + assert!(validate_shm_buffer_layout(2, 1, 4).is_err()); + assert!(validate_shm_buffer_layout(u32::MAX, 1, u32::MAX).is_err()); + assert!(validate_shm_buffer_layout(1, 1, u32::MAX).is_err()); + } + + #[test] + fn shm_layout_accepts_padding_and_returns_checked_wayland_values() { + let layout = validate_shm_buffer_layout(2, 3, 12) + .expect("the test establishes positive in-range dimensions and sufficient stride"); + + assert_eq!(layout.width, 2); + assert_eq!(layout.height, 3); + assert_eq!(layout.stride, 12); + assert_eq!(layout.total_size, 36); + } } diff --git a/src/backend/wayland/frozen/portal.rs b/src/backend/wayland/frozen/portal.rs index a68a4434..dee4d17c 100644 --- a/src/backend/wayland/frozen/portal.rs +++ b/src/backend/wayland/frozen/portal.rs @@ -98,7 +98,7 @@ impl FrozenState { let output_matches = portal_output_matches(target_output, self.active_output_id); if output_matches { - self.set_pending_desktop_image(image, source_geometry); + self.set_pending_desktop_image(image, target_output, source_geometry); } else { warn!("Portal capture for inactive output discarded"); self.capture_done = true; @@ -173,20 +173,23 @@ mod tests { } } - async fn poll_until_finished(frozen: &mut FrozenState, input: &mut InputState) { + async fn poll_until_finished( + frozen: &mut FrozenState, + input: &mut InputState, + ) -> anyhow::Result<()> { for _ in 0..100 { frozen.poll_portal_capture(input, Instant::now()); if !frozen.portal_in_progress { - return; + return Ok(()); } tokio::task::yield_now().await; } - panic!("frozen portal task did not finish"); + anyhow::bail!("frozen portal task did not finish") } #[tokio::test] - async fn poll_portal_applies_image() { - let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + async fn poll_portal_applies_image() -> anyhow::Result<()> { + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); let mut input = make_test_input_state(); @@ -196,7 +199,7 @@ mod tests { async { Ok((None, None, image(0))) }, )); frozen.portal_in_progress = true; - poll_until_finished(&mut frozen, &mut input).await; + poll_until_finished(&mut frozen, &mut input).await?; assert!(!input.frozen_active()); assert!(frozen.has_pending_image()); @@ -211,12 +214,13 @@ mod tests { assert!(input.frozen_active()); assert!(frozen.image.is_some()); assert!(frozen.take_capture_done()); + Ok(()) } #[tokio::test] - async fn domain_error_and_task_panic_restore_the_frozen_lifecycle() { + async fn domain_error_and_task_panic_restore_the_frozen_lifecycle() -> anyhow::Result<()> { for panic_task in [false, true] { - let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); let mut input = make_test_input_state(); frozen.portal_task = Some(if panic_task { @@ -230,27 +234,31 @@ mod tests { }); frozen.portal_in_progress = true; - poll_until_finished(&mut frozen, &mut input).await; + poll_until_finished(&mut frozen, &mut input).await?; assert!(!frozen.is_in_progress()); assert!(frozen.portal_task.is_none()); assert!(frozen.take_capture_done()); assert!(!input.frozen_active()); } + Ok(()) } #[tokio::test] - async fn disconnect_and_deadline_expiry_restore_without_a_producer_result() { + async fn disconnect_and_deadline_expiry_restore_without_a_producer_result() -> anyhow::Result<()> + { let now = Instant::now(); for timed_out in [false, true] { - let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); let mut input = make_test_input_state(); frozen.portal_task = Some(if timed_out { PortalTask::spawn_at_for_test( &tokio::runtime::Handle::current(), wake.handle(), - now.checked_sub(PORTAL_CAPTURE_TIMEOUT).unwrap(), + now.checked_sub(PORTAL_CAPTURE_TIMEOUT).ok_or_else(|| { + anyhow::anyhow!("monotonic clock cannot represent the test deadline") + })?, std::future::pending(), ) } else { @@ -265,11 +273,12 @@ mod tests { assert!(frozen.take_capture_done()); assert!(!input.frozen_active()); } + Ok(()) } #[tokio::test] - async fn user_cancellation_restores_quietly_without_an_error_toast() { - let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + async fn user_cancellation_restores_quietly_without_an_error_toast() -> anyhow::Result<()> { + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); let mut input = make_test_input_state(); frozen.portal_task = Some(PortalTask::spawn( @@ -283,17 +292,19 @@ mod tests { )); frozen.portal_in_progress = true; - poll_until_finished(&mut frozen, &mut input).await; + poll_until_finished(&mut frozen, &mut input).await?; assert!(!frozen.is_in_progress()); assert!(frozen.take_capture_done()); assert!(!input.frozen_active()); assert!(input.ui_toast.is_none()); + Ok(()) } #[tokio::test] - async fn stale_output_is_discarded_without_mutating_current_frozen_state() { - let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + async fn stale_output_is_discarded_without_mutating_current_frozen_state() -> anyhow::Result<()> + { + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); let mut input = make_test_input_state(); input.set_frozen_active(true); @@ -305,16 +316,18 @@ mod tests { )); frozen.portal_in_progress = true; - poll_until_finished(&mut frozen, &mut input).await; + poll_until_finished(&mut frozen, &mut input).await?; assert!(input.frozen_active()); assert!(!frozen.has_pending_image()); assert!(frozen.take_capture_done()); + Ok(()) } #[tokio::test] - async fn supersession_is_ignored_and_explicit_cancel_owns_task_cancellation() { - let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + async fn supersession_is_ignored_and_explicit_cancel_owns_task_cancellation() + -> anyhow::Result<()> { + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); let mut input = make_test_input_state(); frozen.portal_task = Some(PortalTask::spawn( @@ -324,13 +337,12 @@ mod tests { )); frozen.portal_in_progress = true; - frozen - .capture_via_portal(&tokio::runtime::Handle::current()) - .unwrap(); + frozen.capture_via_portal(&tokio::runtime::Handle::current())?; assert!(frozen.portal_task.is_some()); frozen.cancel(&mut input); assert!(frozen.portal_task.is_none()); assert!(!frozen.portal_in_progress); assert!(frozen.take_capture_done()); + Ok(()) } } diff --git a/src/backend/wayland/frozen/state.rs b/src/backend/wayland/frozen/state.rs index a338149f..6f5f9651 100644 --- a/src/backend/wayland/frozen/state.rs +++ b/src/backend/wayland/frozen/state.rs @@ -16,6 +16,7 @@ use super::ext_image_copy::{ExtImageCopyManagers, ExtImageCopySession}; struct PendingFrozenImage { image: FrozenImage, + target_output_id: Option, source_geometry: Option, output_transform: Option, needs_output_transform: bool, @@ -35,17 +36,35 @@ pub(in crate::backend::wayland) enum FrozenCaptureBackend { Portal, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum DirectCaptureBackend { - WlrScreencopy, - ExtImageCopy, +pub(super) enum DirectCaptureAttempt { + WlrScreencopy { + session: Box, + context: DirectCaptureContext, + }, + ExtImageCopy { + session: Box, + context: DirectCaptureContext, + }, } -impl DirectCaptureBackend { - fn capture_backend(self) -> FrozenCaptureBackend { +impl DirectCaptureAttempt { + fn backend(&self) -> FrozenCaptureBackend { + match self { + Self::WlrScreencopy { .. } => FrozenCaptureBackend::WlrScreencopy, + Self::ExtImageCopy { .. } => FrozenCaptureBackend::ExtImageCopy, + } + } + + fn context(&self) -> &DirectCaptureContext { match self { - Self::WlrScreencopy => FrozenCaptureBackend::WlrScreencopy, - Self::ExtImageCopy => FrozenCaptureBackend::ExtImageCopy, + Self::WlrScreencopy { context, .. } | Self::ExtImageCopy { context, .. } => context, + } + } + + fn destroy(self) { + match self { + Self::WlrScreencopy { session, .. } => session.frame.destroy(), + Self::ExtImageCopy { session, .. } => (*session).destroy(), } } } @@ -53,29 +72,22 @@ impl DirectCaptureBackend { pub(super) const DIRECT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(3); pub(super) struct DirectCaptureContext { - pub(super) backend: DirectCaptureBackend, pub(super) target_output_id: u32, pub(super) source_geometry: Option, started_at: Instant, } impl DirectCaptureContext { - pub(super) fn new( - backend: DirectCaptureBackend, - target_output_id: u32, - source_geometry: Option, - ) -> Self { - Self::new_at(backend, target_output_id, source_geometry, Instant::now()) + pub(super) fn new(target_output_id: u32, source_geometry: Option) -> Self { + Self::new_at(target_output_id, source_geometry, Instant::now()) } fn new_at( - backend: DirectCaptureBackend, target_output_id: u32, source_geometry: Option, started_at: Instant, ) -> Self { Self { - backend, target_output_id, source_geometry, started_at, @@ -99,13 +111,11 @@ impl DirectCaptureContext { pub struct FrozenState { pub(super) manager: Option, pub(super) ext_managers: Option, - pub(super) ext_capture: Option, pub(super) portal_available: bool, pub(super) active_output: Option, pub(super) active_output_id: Option, pub(super) active_geometry: Option, - pub(super) capture: Option, - pub(super) direct_capture: Option, + pub(super) direct_capture: Option, pub(super) image: Option, image_target_dimensions: Option<(u32, u32)>, image_generation: u64, @@ -151,12 +161,10 @@ impl FrozenState { Self { manager, ext_managers, - ext_capture: None, portal_available, active_output: None, active_output_id: None, active_geometry: None, - capture: None, direct_capture: None, image: None, image_target_dimensions: None, @@ -218,10 +226,12 @@ impl FrozenState { pub fn set_pending_output_image( &mut self, image: FrozenImage, + target_output_id: u32, source_geometry: Option, ) { self.pending_image = Some(PendingFrozenImage { image, + target_output_id: Some(target_output_id), source_geometry, output_transform: None, needs_output_transform: true, @@ -232,11 +242,13 @@ impl FrozenState { pub(super) fn set_pending_output_image_with_transform( &mut self, image: FrozenImage, + target_output_id: u32, source_geometry: Option, output_transform: Option, ) { self.pending_image = Some(PendingFrozenImage { image, + target_output_id: Some(target_output_id), source_geometry, output_transform, needs_output_transform: true, @@ -247,10 +259,12 @@ impl FrozenState { pub fn set_pending_desktop_image( &mut self, image: FrozenImage, + target_output_id: Option, source_geometry: Option, ) { self.pending_image = Some(PendingFrozenImage { image, + target_output_id, source_geometry, output_transform: None, needs_output_transform: false, @@ -263,9 +277,7 @@ impl FrozenState { } pub fn is_in_progress(&self) -> bool { - self.capture.is_some() - || self.ext_capture.is_some() - || self.direct_capture.is_some() + self.direct_capture.is_some() || self.portal_in_progress || self.preflight_pending || self.pending_image.is_some() @@ -293,7 +305,7 @@ impl FrozenState { ) -> Option { self.direct_capture .as_ref() - .map(|capture| capture.timeout(now)) + .map(|capture| capture.context().timeout(now)) } pub(in crate::backend::wayland) fn take_timed_out_direct_capture( @@ -301,24 +313,13 @@ impl FrozenState { now: Instant, ) -> Option { let capture = self.direct_capture.as_ref()?; - if !capture.timeout(now).is_zero() { + if !capture.context().timeout(now).is_zero() { return None; } - let backend = capture.backend; - self.direct_capture = None; - match backend { - DirectCaptureBackend::WlrScreencopy => { - if let Some(capture) = self.capture.take() { - capture.frame.destroy(); - } - } - DirectCaptureBackend::ExtImageCopy => { - if let Some(capture) = self.ext_capture.take() { - capture.destroy(); - } - } - } - Some(backend.capture_backend()) + let backend = capture.backend(); + let capture = self.direct_capture.take()?; + capture.destroy(); + Some(backend) } pub fn activate_pending_image( @@ -330,6 +331,16 @@ impl FrozenState { let Some(pending) = self.pending_image.take() else { return Ok(false); }; + if !crate::backend::wayland::portal_capture::portal_output_matches( + pending.target_output_id, + self.active_output_id, + ) { + info!("Pending frozen capture discarded after the active output changed"); + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + return Ok(false); + } let mut image = pending.image; if pending.needs_output_transform { @@ -430,13 +441,9 @@ impl FrozenState { } pub fn cancel(&mut self, input_state: &mut InputState) { - if let Some(capture) = self.capture.take() { - capture.frame.destroy(); - } - if let Some(capture) = self.ext_capture.take() { + if let Some(capture) = self.direct_capture.take() { capture.destroy(); } - self.direct_capture = None; self.preflight_pending = false; self.preflight_backend = None; self.portal_in_progress = false; @@ -537,37 +544,15 @@ mod tests { } #[test] - fn direct_capture_deadline_expires_and_keeps_its_backend_identity() { + fn direct_capture_context_tracks_its_deadline_and_output_identity() { let started_at = Instant::now(); - let mut state = FrozenState::new(None); - state.direct_capture = Some(DirectCaptureContext::new_at( - DirectCaptureBackend::ExtImageCopy, - 7, - None, - started_at, - )); + let capture = DirectCaptureContext::new_at(7, None, started_at); + assert_eq!(capture.timeout(started_at), DIRECT_CAPTURE_TIMEOUT); assert_eq!( - state.direct_capture_timeout(started_at), - Some(DIRECT_CAPTURE_TIMEOUT) - ); - assert_eq!( - state.take_timed_out_direct_capture(started_at + DIRECT_CAPTURE_TIMEOUT), - Some(FrozenCaptureBackend::ExtImageCopy) - ); - assert!(state.direct_capture.is_none()); - assert_eq!(state.direct_capture_timeout(started_at), None); - } - - #[test] - fn direct_capture_context_rejects_a_different_or_missing_output() { - let capture = DirectCaptureContext::new_at( - DirectCaptureBackend::WlrScreencopy, - 7, - None, - Instant::now(), + capture.timeout(started_at + DIRECT_CAPTURE_TIMEOUT), + Duration::ZERO ); - assert!(capture.output_matches(Some(7))); assert!(!capture.output_matches(Some(8))); assert!(!capture.output_matches(None)); @@ -577,6 +562,7 @@ mod tests { fn active_output_capture_accepts_native_fractional_scale_dimensions() { let mut state = FrozenState::new(None); let mut input_state = make_test_input_state(); + state.set_active_output(None, Some(7)); state.set_pending_output_image( FrozenImage { width: 10, @@ -584,6 +570,7 @@ mod tests { stride: 40, data: vec![0; 10 * 10 * 4], }, + 7, None, ); @@ -607,6 +594,7 @@ mod tests { fn active_output_capture_uses_protocol_transform_without_output_geometry() { let mut state = FrozenState::new(None); let mut input_state = make_test_input_state(); + state.set_active_output(None, Some(7)); state.set_pending_output_image_with_transform( FrozenImage { width: 2, @@ -614,6 +602,7 @@ mod tests { stride: 8, data: vec![1, 0, 0, 255, 2, 0, 0, 255], }, + 7, None, Some(wl_output::Transform::_90), ); @@ -638,6 +627,7 @@ mod tests { data: vec![0; 4 * 3 * 4], }, None, + None, ); assert!( @@ -649,6 +639,34 @@ mod tests { assert!(!input_state.frozen_active()); } + #[test] + fn pending_capture_is_discarded_if_output_changes_before_activation() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + state.set_active_output(None, Some(7)); + state.set_pending_output_image( + FrozenImage { + width: 1, + height: 1, + stride: 4, + data: vec![0; 4], + }, + 7, + None, + ); + + state.set_active_output(None, Some(8)); + let activated = state + .activate_pending_image(1, 1, &mut input_state) + .expect("the stale-output path is a handled non-error outcome"); + + assert!(!activated); + assert!(state.image().is_none()); + assert!(!state.has_pending_image()); + assert!(state.take_capture_done()); + assert!(!input_state.frozen_active()); + } + #[test] fn cancel_clears_an_in_flight_portal_capture() { let mut state = FrozenState::new(None); diff --git a/src/capture/portal.rs b/src/capture/portal.rs index d53aba3a..2cfe6ba2 100644 --- a/src/capture/portal.rs +++ b/src/capture/portal.rs @@ -429,9 +429,10 @@ mod tests { } #[test] - fn generated_portal_handle_tokens_have_independent_random_suffixes() { - let first = next_handle_token().expect("generate first secure token"); - let second = next_handle_token().expect("generate second secure token"); + fn generated_portal_handle_tokens_have_independent_random_suffixes() -> Result<(), CaptureError> + { + let first = next_handle_token()?; + let second = next_handle_token()?; assert_ne!(first, second); assert_eq!( @@ -441,6 +442,7 @@ mod tests { assert_eq!(second.len(), first.len()); assert!(portal_request_path_for_unique_name(":1.42", &first).is_ok()); assert!(portal_request_path_for_unique_name(":1.42", &second).is_ok()); + Ok(()) } #[test] diff --git a/src/capture/sources/reader.rs b/src/capture/sources/reader.rs index 88a5ceaa..1046ce30 100644 --- a/src/capture/sources/reader.rs +++ b/src/capture/sources/reader.rs @@ -12,7 +12,7 @@ use std::{fs, thread, time::Duration}; pub fn read_image_from_uri(uri: &str) -> Result, CaptureError> { let path = decode_file_uri(uri)?; - log::debug!("Reading screenshot from: {}", path.display()); + log::debug!("Reading screenshot from the portal temporary file"); // Wait briefly for portal to flush the file to disk (some portals write asynchronously) const MAX_ATTEMPTS: usize = 60; // up to 3 seconds total @@ -27,16 +27,14 @@ pub fn read_image_from_uri(uri: &str) -> Result, CaptureError> { } Ok(_) => { log::trace!( - "Portal screenshot file {} still empty (attempt {}/{})", - path.display(), + "Portal screenshot file still empty (attempt {}/{})", attempt + 1, MAX_ATTEMPTS ); } Err(e) => { log::trace!( - "Portal screenshot file {} not ready yet (attempt {}/{}): {}", - path.display(), + "Portal screenshot file not ready yet (attempt {}/{}): {}", attempt + 1, MAX_ATTEMPTS, e @@ -46,9 +44,7 @@ pub fn read_image_from_uri(uri: &str) -> Result, CaptureError> { if attempt + 1 == MAX_ATTEMPTS { return Err(CaptureError::ImageError(format!( - "Portal screenshot file {} not ready after {} attempts", - path.display(), - MAX_ATTEMPTS + "Portal screenshot file was not ready after {MAX_ATTEMPTS} attempts" ))); } @@ -62,13 +58,9 @@ pub fn read_image_from_uri(uri: &str) -> Result, CaptureError> { // Clean up portal temp file to prevent accumulation if let Err(e) = fs::remove_file(&path) { - log::warn!( - "Failed to remove portal temp file {}: {}", - path.display(), - e - ); + log::warn!("Failed to remove portal screenshot temporary file: {e}"); } else { - log::debug!("Removed portal temp file: {}", path.display()); + log::debug!("Removed portal screenshot temporary file"); } Ok(data) diff --git a/src/file_uri.rs b/src/file_uri.rs index 502ec6ce..16094929 100644 --- a/src/file_uri.rs +++ b/src/file_uri.rs @@ -6,18 +6,18 @@ use std::os::unix::ffi::OsStringExt; pub(crate) fn decode_file_uri(uri: &str) -> Result { let raw = uri .strip_prefix("file://") - .ok_or_else(|| format!("Invalid file URI '{uri}'"))?; + .ok_or_else(|| "Invalid file URI scheme".to_string())?; let path_part = if raw.starts_with("localhost/") { &raw["localhost".len()..] } else if raw.starts_with('/') { raw } else { - return Err(format!("Unsupported file URI host in '{uri}'")); + return Err("Unsupported file URI host".to_string()); }; let decoded = percent_decode(path_part) - .map_err(|err| format!("Invalid percent-encoding in '{uri}': {err}"))?; + .map_err(|err| format!("Invalid percent-encoding in file URI: {err}"))?; #[cfg(unix)] { @@ -28,7 +28,7 @@ pub(crate) fn decode_file_uri(uri: &str) -> Result { #[cfg(not(unix))] { let path = String::from_utf8(decoded) - .map_err(|err| format!("Non-UTF8 path in URI '{uri}': {err}"))?; + .map_err(|err| format!("Non-UTF8 path in file URI: {err}"))?; Ok(PathBuf::from(path)) } } @@ -72,14 +72,27 @@ mod tests { #[test] fn decode_file_uri_rejects_non_file_schemes() { - let err = decode_file_uri("http://example.com/file.png").expect_err("expected error"); + let uri = "http://private.example/screenshot.png"; + let err = decode_file_uri(uri).expect_err("expected error"); assert!(err.contains("Invalid file URI")); + assert!(!err.contains(uri)); } #[test] fn decode_file_uri_rejects_unsupported_hosts() { - let err = decode_file_uri("file://example.com/path.png").expect_err("expected error"); + let uri = "file://private.example/screenshot.png"; + let err = decode_file_uri(uri).expect_err("expected error"); assert!(err.contains("Unsupported file URI host")); + assert!(!err.contains(uri)); + } + + #[test] + fn decode_file_uri_redacts_invalid_percent_encoded_locations() { + let uri = "file:///home/private-user/%ZZ.png"; + let err = decode_file_uri(uri).expect_err("expected error"); + assert!(err.contains("Invalid percent-encoding in file URI")); + assert!(!err.contains(uri)); + assert!(!err.contains("private-user")); } #[test]