From 8c3fb4311e447cb676627a4032df9d9ae5853dab Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Tue, 18 Aug 2026 21:31:17 +0200 Subject: [PATCH 1/2] fix capture geometry across output changes --- CONTRIBUTING.md | 19 ++ docs/SETUP.md | 2 +- src/backend/wayland/capture.rs | 35 +++ src/backend/wayland/frozen/capture.rs | 8 +- src/backend/wayland/frozen/ext_image_copy.rs | 8 +- src/backend/wayland/frozen/image.rs | 101 +++++-- src/backend/wayland/frozen/mod.rs | 1 + src/backend/wayland/frozen/portal.rs | 42 ++- src/backend/wayland/frozen/state.rs | 267 ++++++++++++++++-- src/backend/wayland/frozen_geometry.rs | 204 ++++++++++++- src/backend/wayland/handlers/compositor.rs | 22 +- src/backend/wayland/handlers/layer.rs | 26 +- src/backend/wayland/handlers/output.rs | 18 +- src/backend/wayland/handlers/xdg.rs | 23 +- src/backend/wayland/state/capture/backdrop.rs | 70 +++-- src/backend/wayland/state/capture/pdf.rs | 48 +++- src/backend/wayland/state/screen_image.rs | 4 +- src/backend/wayland/zoom/capture.rs | 250 ++++++++++++---- src/backend/wayland/zoom/portal.rs | 185 +++++++++--- src/backend/wayland/zoom/state.rs | 16 +- src/backend/wayland/zoom/view.rs | 42 ++- src/capture/desktop_backdrop.rs | 19 ++ src/capture/tests/desktop_backdrop.rs | 57 +++- src/capture/types.rs | 26 ++ 24 files changed, 1199 insertions(+), 294 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3a2e302c0..8992ebd38 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -129,6 +129,25 @@ For offline work, prefetch dependencies first: ./tools/fetch-all-deps.sh ``` +### Freeze, zoom, and capture manual matrix + +Geometry unit tests cover crop arithmetic and stale-output rejection, but they do not prove a +compositor's protocol behavior. For capture-related changes, exercise every unchecked cell below +on real hardware; this checklist is manual evidence, not CI coverage. + +| Compositor | 1x | Fractional scale | Mixed DPI | +|---|---|---|---| +| Hyprland | [ ] | [ ] | [ ] | +| Plasma/KWin | [ ] | [ ] | [ ] | +| Niri | [ ] | [ ] | [ ] | +| Sway | [ ] | [ ] | [ ] | +| GNOME | [ ] | [ ] | [ ] | + +For each cell, record the selected capture backend from the logs. Verify that Freeze, Zoom, a +screenshot, and a PDF desktop backdrop all use the active output's exact pixels. Repeat while +switching outputs and, where supported, changing scale or transform during capture: the result must +either stay exact or fail visibly, never shift, stretch, or reuse another output's image. + `./tools/code-health-report.sh` reports navigational maintainability metrics. Its CI artifact is observational, not a global file/function-size gate; use the report to find code worth understanding, not as a reason for mechanical splitting. diff --git a/docs/SETUP.md b/docs/SETUP.md index 2a2d01f37..b842ca0b9 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 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. +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. On mixed-DPI or multi-monitor layouts, Wayscriber accepts a portal image only when it can validate the active output's crop; otherwise Freeze fails instead of guessing at an origin or scale. 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/src/backend/wayland/capture.rs b/src/backend/wayland/capture.rs index 0e516fe7e..1db598eb7 100644 --- a/src/backend/wayland/capture.rs +++ b/src/backend/wayland/capture.rs @@ -40,6 +40,31 @@ pub(in crate::backend::wayland) struct PendingPdfExport { pub action: Action, pub operation: ImageOperationKind, pub save_config: FileSaveConfig, + pub layout_context: CaptureLayoutContext, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(in crate::backend::wayland) struct CaptureLayoutContext { + target_output_id: u32, + layout_generation: u64, +} + +impl CaptureLayoutContext { + pub(in crate::backend::wayland) fn new(target_output_id: u32, layout_generation: u64) -> Self { + Self { + target_output_id, + layout_generation, + } + } + + pub(in crate::backend::wayland) fn matches( + self, + active_output_id: Option, + layout_generation: u64, + ) -> bool { + active_output_id == Some(self.target_output_id) + && layout_generation == self.layout_generation + } } /// Tracks capture manager state and in-progress flag. @@ -222,4 +247,14 @@ mod tests { assert_eq!(state.accepted_id(), None); assert!(!state.is_in_progress()); } + + #[test] + fn capture_layout_context_rejects_output_or_geometry_generation_changes() { + let context = CaptureLayoutContext::new(7, 3); + + assert!(context.matches(Some(7), 3)); + assert!(!context.matches(Some(8), 3)); + assert!(!context.matches(Some(7), 4)); + assert!(!context.matches(None, 3)); + } } diff --git a/src/backend/wayland/frozen/capture.rs b/src/backend/wayland/frozen/capture.rs index 9e65c4651..a32815291 100644 --- a/src/backend/wayland/frozen/capture.rs +++ b/src/backend/wayland/frozen/capture.rs @@ -236,7 +236,11 @@ impl FrozenState { capture.pool = Some(pool); self.direct_capture = Some(DirectCaptureAttempt::WlrScreencopy { session: Box::new(capture), - context: DirectCaptureContext::new(target_output_id, source_geometry), + context: DirectCaptureContext::new( + target_output_id, + source_geometry, + self.output_layout_generation, + ), }); Ok(()) @@ -404,7 +408,7 @@ impl FrozenState { })(); capture.frame.destroy(); let image = result?; - if !context.output_matches(self.active_output_id) { + if !context.output_and_layout_match(self.active_output_id, self.output_layout_generation) { return Ok(false); } diff --git a/src/backend/wayland/frozen/ext_image_copy.rs b/src/backend/wayland/frozen/ext_image_copy.rs index b42b995c2..41212e4cc 100644 --- a/src/backend/wayland/frozen/ext_image_copy.rs +++ b/src/backend/wayland/frozen/ext_image_copy.rs @@ -175,7 +175,11 @@ impl FrozenState { .create_session(&source, Options::empty(), qh, ()); self.direct_capture = Some(DirectCaptureAttempt::ExtImageCopy { session: Box::new(ExtImageCopySession::new(source, session, pool)), - context: DirectCaptureContext::new(target_output_id, source_geometry), + context: DirectCaptureContext::new( + target_output_id, + source_geometry, + self.output_layout_generation, + ), }); debug!("Requested ext-image-copy capture constraints for active output"); Ok(()) @@ -396,7 +400,7 @@ impl FrozenState { .take_ext_capture() .context("ext-image-copy capture attempt missing after frame completion")?; (*capture).destroy(); - if !context.output_matches(self.active_output_id) { + if !context.output_and_layout_match(self.active_output_id, self.output_layout_generation) { return Ok(false); } self.set_pending_output_image_with_transform( diff --git a/src/backend/wayland/frozen/image.rs b/src/backend/wayland/frozen/image.rs index 574f13f0b..2698ca242 100644 --- a/src/backend/wayland/frozen/image.rs +++ b/src/backend/wayland/frozen/image.rs @@ -1,16 +1,16 @@ 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, +pub(in crate::backend::wayland) struct ShmBufferLayout { + pub(in crate::backend::wayland) width: i32, + pub(in crate::backend::wayland) height: i32, + pub(in crate::backend::wayland) stride: i32, + pub(in crate::backend::wayland) total_size: usize, } /// Validate compositor-owned dimensions before allocating or creating a /// `wl_buffer` from them. -pub(super) fn validate_shm_buffer_layout( +pub(in crate::backend::wayland) fn validate_shm_buffer_layout( width: u32, height: u32, stride: u32, @@ -62,30 +62,45 @@ impl FrozenImage { /// WLR screencopy buffers are returned in output framebuffer coordinates. /// Frozen/zoom rendering paints them onto a logical surface, so rotated or /// flipped outputs need the same transform applied to the captured pixels. - pub fn with_output_transform(mut self, transform: wl_output::Transform) -> Self { + pub fn with_output_transform(mut self, transform: wl_output::Transform) -> Result { + let width = + usize::try_from(self.width).context("Frozen image width does not fit memory")?; + let height = + usize::try_from(self.height).context("Frozen image height does not fit memory")?; + let row_bytes = width + .checked_mul(4) + .context("Frozen image row size overflow")?; + let expected_len = row_bytes + .checked_mul(height) + .context("Frozen image size overflow")?; + let expected_stride = + i32::try_from(row_bytes).context("Frozen image stride exceeds i32")?; + if self.stride != expected_stride || self.data.len() != expected_len { + anyhow::bail!("Frozen image buffer does not match its dimensions"); + } if transform == wl_output::Transform::Normal { - return self; + return Ok(self); } - if let Some((width, height, data)) = transform_argb( - self.width as usize, - self.height as usize, - &self.data, - transform, - ) { - self.width = width as u32; - self.height = height as u32; - self.stride = (width * 4) as i32; - self.data = data; - } + let (width, height, data) = transform_argb(width, height, &self.data, transform) + .context("Frozen image transform rejected its pixel buffer")?; + self.width = u32::try_from(width).context("Transformed frozen width exceeds u32")?; + self.height = u32::try_from(height).context("Transformed frozen height exceeds u32")?; + self.stride = i32::try_from( + width + .checked_mul(4) + .context("Transformed frozen stride overflow")?, + ) + .context("Transformed frozen stride exceeds i32")?; + self.data = data; - self + Ok(self) } } /// 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( +pub(in crate::backend::wayland) fn copy_shm_argb( canvas: &[u8], width: u32, height: u32, @@ -161,6 +176,19 @@ fn transform_argb( data: &[u8], transform: wl_output::Transform, ) -> Option<(usize, usize, Vec)> { + if !matches!( + transform, + wl_output::Transform::Normal + | wl_output::Transform::_90 + | wl_output::Transform::_180 + | wl_output::Transform::_270 + | wl_output::Transform::Flipped + | wl_output::Transform::Flipped90 + | wl_output::Transform::Flipped180 + | wl_output::Transform::Flipped270 + ) { + return None; + } if data.len() != width.checked_mul(height)?.checked_mul(4)? { return None; } @@ -233,10 +261,27 @@ mod tests { image.data.chunks_exact(4).map(|chunk| chunk[0]).collect() } + #[test] + fn output_transform_rejects_an_invalid_pixel_buffer() { + let malformed = FrozenImage { + width: 2, + height: 2, + stride: 8, + data: vec![0; 12], + }; + + assert!( + malformed + .with_output_transform(wl_output::Transform::_90) + .is_err() + ); + } + #[test] fn output_transform_270_rotates_into_logical_orientation() { - let transformed = - image(3, 2, &[1, 2, 3, 4, 5, 6]).with_output_transform(wl_output::Transform::_270); + let transformed = image(3, 2, &[1, 2, 3, 4, 5, 6]) + .with_output_transform(wl_output::Transform::_270) + .expect("valid transform"); assert_eq!((transformed.width, transformed.height), (2, 3)); assert_eq!(values(&transformed), vec![4, 1, 5, 2, 6, 3]); @@ -245,8 +290,9 @@ mod tests { #[test] fn output_transform_90_rotates_into_logical_orientation() { - let transformed = - image(3, 2, &[1, 2, 3, 4, 5, 6]).with_output_transform(wl_output::Transform::_90); + let transformed = image(3, 2, &[1, 2, 3, 4, 5, 6]) + .with_output_transform(wl_output::Transform::_90) + .expect("valid transform"); assert_eq!((transformed.width, transformed.height), (2, 3)); assert_eq!(values(&transformed), vec![3, 6, 2, 5, 1, 4]); @@ -254,8 +300,9 @@ mod tests { #[test] fn flipped_transform_mirrors_pixels() { - let transformed = - image(3, 2, &[1, 2, 3, 4, 5, 6]).with_output_transform(wl_output::Transform::Flipped); + let transformed = image(3, 2, &[1, 2, 3, 4, 5, 6]) + .with_output_transform(wl_output::Transform::Flipped) + .expect("valid transform"); assert_eq!((transformed.width, transformed.height), (3, 2)); assert_eq!(values(&transformed), vec![3, 2, 1, 6, 5, 4]); diff --git a/src/backend/wayland/frozen/mod.rs b/src/backend/wayland/frozen/mod.rs index f51803c2c..e7d3bd920 100644 --- a/src/backend/wayland/frozen/mod.rs +++ b/src/backend/wayland/frozen/mod.rs @@ -6,6 +6,7 @@ mod state; pub(in crate::backend::wayland) use ext_image_copy::ExtImageCopyManagers; pub use image::FrozenImage; +pub(in crate::backend::wayland) use image::{copy_shm_argb, validate_shm_buffer_layout}; pub(in crate::backend::wayland) use state::FrozenCaptureBackend; pub use state::FrozenState; diff --git a/src/backend/wayland/frozen/portal.rs b/src/backend/wayland/frozen/portal.rs index f5e6b3977..995221080 100644 --- a/src/backend/wayland/frozen/portal.rs +++ b/src/backend/wayland/frozen/portal.rs @@ -28,11 +28,18 @@ impl FrozenState { .runtime_wake .clone() .ok_or_else(|| anyhow::anyhow!("portal capture runtime wake is unavailable"))?; + let source_geometry = self.active_geometry.clone().ok_or_else(|| { + anyhow::anyhow!("active output geometry is unavailable for portal freeze capture") + })?; + source_geometry.verified_pixel_size().ok_or_else(|| { + anyhow::anyhow!("active output pixel size is unavailable for portal freeze capture") + })?; + let target_output_id = self.active_output_id.ok_or_else(|| { + anyhow::anyhow!("active output identity is unavailable for portal freeze capture") + })?; self.portal_in_progress = true; - self.portal_target_output_id = self.active_output_id; + self.portal_target_output_id = Some(target_output_id); - let source_geometry = self.active_geometry.clone(); - let target_output_id = self.active_output_id; let layout_generation = self.output_layout_generation; // Notify user that portal fallback is in progress crate::notification::send_notification_async( @@ -49,9 +56,9 @@ impl FrozenState { .map_err(|error| CaptureError::ImageError(format!("Decode failed: {error}")))?; Ok(( - target_output_id, + Some(target_output_id), layout_generation, - source_geometry, + Some(source_geometry), FrozenImage { width, height, @@ -190,7 +197,10 @@ mod tests { logical_height: 1, scale: 1, transform: wayland_client::protocol::wl_output::Transform::Normal, + overlay_buffer_size: (2, 1), + pixel_size: Some((2, 1)), screenshot_origin: Some(origin), + screenshot_size: None, } } @@ -208,6 +218,28 @@ mod tests { anyhow::bail!("frozen portal task did not finish") } + #[tokio::test] + async fn portal_start_requires_verifiable_geometry_and_output_identity() -> anyhow::Result<()> { + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; + let mut frozen = FrozenState::new_with_runtime_wake(None, wake.handle()); + + let error = frozen + .capture_via_portal(&tokio::runtime::Handle::current()) + .expect_err("missing geometry must fail closed"); + assert!(error.to_string().contains("geometry is unavailable")); + assert!(!frozen.portal_in_progress); + assert!(frozen.portal_task.is_none()); + + frozen.set_active_geometry(Some(crop_geometry((0, 0)))); + let error = frozen + .capture_via_portal(&tokio::runtime::Handle::current()) + .expect_err("missing output identity must fail closed"); + assert!(error.to_string().contains("identity is unavailable")); + assert!(!frozen.portal_in_progress); + assert!(frozen.portal_task.is_none()); + Ok(()) + } + #[tokio::test] async fn poll_portal_applies_image() -> anyhow::Result<()> { let wake = crate::backend::wayland::RuntimeWakeSource::new()?; diff --git a/src/backend/wayland/frozen/state.rs b/src/backend/wayland/frozen/state.rs index add54a1b5..04ce675ac 100644 --- a/src/backend/wayland/frozen/state.rs +++ b/src/backend/wayland/frozen/state.rs @@ -74,22 +74,34 @@ pub(super) const DIRECT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(3); pub(super) struct DirectCaptureContext { pub(super) target_output_id: u32, pub(super) source_geometry: Option, + layout_generation: u64, started_at: Instant, } impl DirectCaptureContext { - pub(super) fn new(target_output_id: u32, source_geometry: Option) -> Self { - Self::new_at(target_output_id, source_geometry, Instant::now()) + pub(super) fn new( + target_output_id: u32, + source_geometry: Option, + layout_generation: u64, + ) -> Self { + Self::new_at( + target_output_id, + source_geometry, + layout_generation, + Instant::now(), + ) } fn new_at( target_output_id: u32, source_geometry: Option, + layout_generation: u64, started_at: Instant, ) -> Self { Self { target_output_id, source_geometry, + layout_generation, started_at, } } @@ -101,8 +113,13 @@ impl DirectCaptureContext { .unwrap_or(Duration::ZERO) } - pub(super) fn output_matches(&self, current_output_id: Option) -> bool { + pub(super) fn output_and_layout_match( + &self, + current_output_id: Option, + current_layout_generation: u64, + ) -> bool { current_output_id == Some(self.target_output_id) + && current_layout_generation == self.layout_generation } } @@ -211,12 +228,8 @@ impl FrozenState { self.active_geometry = geometry; } - pub fn active_geometry(&self) -> Option<&OutputGeometry> { - self.active_geometry.as_ref() - } - - pub fn active_output_matches(&self, info_id: u32) -> bool { - self.active_output_id == Some(info_id) + pub(in crate::backend::wayland) fn output_layout_generation(&self) -> u64 { + self.output_layout_generation } pub fn image(&self) -> Option<&FrozenImage> { @@ -363,24 +376,72 @@ impl FrozenState { .map(|geo| geo.transform) .unwrap_or(wl_output::Transform::Normal) }); - image = image.with_output_transform(output_transform); + image = match image.with_output_transform(output_transform) { + Ok(image) => image, + Err(error) => { + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + return Err(format!("Freeze capture transform failed: {error}")); + } + }; } - if matches!(pending.source, FrozenCaptureSource::Desktop) - && (image.width != phys_width || image.height != phys_height) + if matches!(pending.source, FrozenCaptureSource::ActiveOutput) + && pending + .source_geometry + .as_ref() + .or(self.active_geometry.as_ref()) + .is_some_and(|geometry| { + !geometry.accepts_transformed_pixel_size(image.width, image.height) + }) { - let Some(cropped) = self.crop_pending_image( - image, - pending.source_geometry.as_ref(), - phys_width, - phys_height, - ) else { + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + return Err("Freeze capture dimensions do not match the active output".to_string()); + } + + if matches!(pending.source, FrozenCaptureSource::Desktop) { + let geometry = pending + .source_geometry + .as_ref() + .or(self.active_geometry.as_ref()); + let capture_dimensions = match geometry { + Some(geometry) => geometry.verified_pixel_size(), + None if image.width == phys_width && image.height == phys_height => { + Some((phys_width, phys_height)) + } + None => None, + }; + let Some((capture_width, capture_height)) = capture_dimensions else { self.capture_done = true; input_state.set_frozen_active(false); input_state.needs_redraw = true; return Err("Freeze failed after the display changed size".to_string()); }; - image = cropped; + + if let Some(geometry) = geometry { + let Some(cropped) = + self.crop_pending_image(image, Some(geometry), capture_width, capture_height) + else { + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + return Err("Freeze failed after the display changed size".to_string()); + }; + image = cropped; + } + } + + if !OutputGeometry::dimensions_have_compatible_aspect( + (image.width, image.height), + (phys_width, phys_height), + ) { + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + return Err("Freeze capture aspect does not match the overlay surface".to_string()); } self.image_target_dimensions = Some((phys_width, phys_height)); @@ -417,10 +478,11 @@ impl FrozenState { if width != target_width || height != target_height { return None; } + let stride = i32::try_from(target_width.checked_mul(4)?).ok()?; Some(FrozenImage { width: target_width, height: target_height, - stride: (target_width * 4) as i32, + stride, data, }) } @@ -555,16 +617,17 @@ mod tests { #[test] fn direct_capture_context_tracks_its_deadline_and_output_identity() { let started_at = Instant::now(); - let capture = DirectCaptureContext::new_at(7, None, started_at); + let capture = DirectCaptureContext::new_at(7, None, 3, started_at); assert_eq!(capture.timeout(started_at), DIRECT_CAPTURE_TIMEOUT); assert_eq!( 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)); + assert!(capture.output_and_layout_match(Some(7), 3)); + assert!(!capture.output_and_layout_match(Some(8), 3)); + assert!(!capture.output_and_layout_match(None, 3)); + assert!(!capture.output_and_layout_match(Some(7), 4)); } #[test] @@ -599,6 +662,40 @@ mod tests { assert!(!input_state.frozen_active()); } + #[test] + fn active_output_capture_rejects_known_pixel_size_mismatch() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + state.set_active_output(None, Some(7)); + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wl_output::Transform::Normal, + Some((5, 3)), + ) + .expect("known output geometry"); + state.set_pending_output_image( + FrozenImage { + width: 4, + height: 3, + stride: 16, + data: vec![0; 4 * 3 * 4], + }, + 7, + Some(geometry), + ); + + assert!( + state + .activate_pending_image(6, 4, &mut input_state) + .is_err() + ); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + } + #[test] fn active_output_capture_uses_protocol_transform_without_output_geometry() { let mut state = FrozenState::new(None); @@ -624,6 +721,57 @@ mod tests { assert_eq!((image.width, image.height), (1, 2)); } + #[test] + fn active_output_capture_fails_closed_when_transform_input_is_malformed() { + 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, + height: 2, + stride: 8, + data: vec![0; 12], + }, + 7, + None, + Some(wl_output::Transform::_90), + ); + + assert!( + state + .activate_pending_image(2, 2, &mut input_state) + .is_err() + ); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + assert!(state.take_capture_done()); + } + + #[test] + fn active_output_capture_rejects_stretching_into_a_different_viewport() { + 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: 320, + height: 180, + stride: 1280, + data: vec![0; 320 * 180 * 4], + }, + 7, + None, + ); + + let error = state + .activate_pending_image(320, 176, &mut input_state) + .expect_err("a full output cannot be stretched into a shorter viewport"); + assert!(error.contains("aspect does not match")); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + } + #[test] fn desktop_capture_still_requires_a_crop_covering_the_target() { let mut state = FrozenState::new(None); @@ -656,6 +804,7 @@ mod tests { scale: i32, screenshot_origin: Option<(u32, u32)>, ) -> OutputGeometry { + let pixel_scale = u32::try_from(scale).expect("test scale must be positive"); OutputGeometry { logical_x, logical_y, @@ -663,7 +812,10 @@ mod tests { logical_height, scale, transform: wl_output::Transform::Normal, + overlay_buffer_size: (logical_width * pixel_scale, logical_height * pixel_scale), + pixel_size: Some((logical_width * pixel_scale, logical_height * pixel_scale)), screenshot_origin, + screenshot_size: None, } } @@ -713,6 +865,73 @@ mod tests { ); } + #[test] + fn desktop_capture_keeps_fractional_output_pixels_for_a_larger_overlay_buffer() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + let mut geometry = desktop_geometry(0, 0, 3, 2, 2, Some((0, 0))); + geometry.pixel_size = Some((5, 3)); + state.set_pending_desktop_image( + FrozenImage { + width: 5, + height: 3, + stride: 20, + data: vec![7; 5 * 3 * 4], + }, + None, + Some(geometry), + ); + + state + .activate_pending_image(6, 4, &mut input_state) + .expect("native output pixels should render into the integer-scale overlay buffer"); + + let image = state.image().expect("the frozen image should be active"); + assert_eq!((image.width, image.height), (5, 3)); + state.handle_resize(6, 4, &mut input_state); + assert!(state.image().is_some()); + assert!(input_state.frozen_active()); + } + + #[test] + fn desktop_capture_rejects_a_portal_image_from_a_different_layout_size() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + let geometry = desktop_geometry(0, 0, 4, 1, 1, None).with_desktop_backdrop_geometry(Some( + crate::capture::DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 4, + logical_height: 1, + scale: 1, + physical_width: Some(4), + physical_height: Some(1), + crop_x: Some(6), + crop_y: Some(0), + screenshot_width: Some(10), + screenshot_height: Some(1), + }, + )); + state.set_pending_desktop_image( + FrozenImage { + width: 11, + height: 1, + stride: 44, + data: vec![0; 11 * 4], + }, + None, + Some(geometry), + ); + + assert!( + state + .activate_pending_image(4, 1, &mut input_state) + .is_err() + ); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + } + #[test] fn desktop_capture_crops_at_buffer_origin_when_screenshot_origin_is_unknown() { let mut state = FrozenState::new(None); diff --git a/src/backend/wayland/frozen_geometry.rs b/src/backend/wayland/frozen_geometry.rs index 6a6244ab9..920872454 100644 --- a/src/backend/wayland/frozen_geometry.rs +++ b/src/backend/wayland/frozen_geometry.rs @@ -1,5 +1,7 @@ use wayland_client::protocol::wl_output; +use crate::capture::DesktopBackdropGeometry; + /// Geometry and scale details for the active output, used for cropping fallback captures. #[derive(Clone, Debug, PartialEq, Eq)] pub struct OutputGeometry { @@ -11,9 +13,17 @@ pub struct OutputGeometry { pub logical_height: u32, pub scale: i32, pub transform: wl_output::Transform, + /// Integer-scale buffer dimensions of the overlay surface. These are + /// independent of the output's logical/native size on xdg-shell fallbacks. + pub(super) overlay_buffer_size: (u32, u32), + /// Physical pixels belonging to this output after applying its transform. + /// This may differ from `logical * scale` under fractional scaling. + pub(super) pixel_size: Option<(u32, u32)>, /// Origin of this output inside a full-desktop screenshot, walking every /// known output so mixed-DPI layouts are not `logical * this_output.scale`. pub screenshot_origin: Option<(u32, u32)>, + /// Expected full-desktop screenshot dimensions for the captured layout. + pub(super) screenshot_size: Option<(u32, u32)>, } impl OutputGeometry { @@ -23,12 +33,26 @@ impl OutputGeometry { fallback_size: (u32, u32), scale: i32, transform: wl_output::Transform, + pixel_size: Option<(u32, u32)>, ) -> Option { let (lx, ly) = logical_pos.unwrap_or((0, 0)); - let (lw, lh) = logical_size.unwrap_or((fallback_size.0 as i32, fallback_size.1 as i32)); + let fallback_width = i32::try_from(fallback_size.0).ok()?; + let fallback_height = i32::try_from(fallback_size.1).ok()?; + let (lw, lh) = logical_size.unwrap_or((fallback_width, fallback_height)); if lw <= 0 || lh <= 0 || scale <= 0 { return None; } + let buffer_scale = u32::try_from(scale).ok()?; + let overlay_buffer_size = ( + fallback_size.0.checked_mul(buffer_scale)?, + fallback_size.1.checked_mul(buffer_scale)?, + ); + if overlay_buffer_size.0 == 0 || overlay_buffer_size.1 == 0 { + return None; + } + if pixel_size.is_some_and(|(width, height)| width == 0 || height == 0) { + return None; + } Some(Self { logical_x: lx, logical_y: ly, @@ -36,7 +60,10 @@ impl OutputGeometry { logical_height: lh as u32, scale, transform, + overlay_buffer_size, + pixel_size, screenshot_origin: None, + screenshot_size: None, }) } } @@ -45,6 +72,7 @@ impl OutputGeometry { #[allow(clippy::items_after_test_module)] mod tests { use super::*; + use crate::capture::DesktopBackdropGeometry; #[test] fn update_from_uses_logical_and_scale() { @@ -54,6 +82,7 @@ mod tests { (800, 600), 2, wl_output::Transform::_270, + Some((3840, 2160)), ) .expect("geometry"); assert_eq!(geo.logical_x, 10); @@ -61,7 +90,8 @@ mod tests { assert_eq!(geo.logical_width, 1920); assert_eq!(geo.logical_height, 1080); assert_eq!(geo.transform, wl_output::Transform::_270); - assert_eq!(geo.physical_size(), (3840, 2160)); + assert_eq!(geo.verified_pixel_size(), Some((3840, 2160))); + assert_eq!(geo.buffer_size(), Some((1600, 1200))); assert_eq!(geo.physical_origin(), (20, 40)); assert_eq!(geo.portal_crop_origin(3840, 2160), Some((0, 0))); assert_eq!(geo.portal_crop_origin(8000, 2160), None); @@ -74,11 +104,104 @@ mod tests { #[test] fn update_from_uses_fallback_when_missing_logical_size() { - let geo = - OutputGeometry::update_from(None, None, (800, 600), 1, wl_output::Transform::Normal) - .expect("geometry"); + let geo = OutputGeometry::update_from( + None, + None, + (800, 600), + 1, + wl_output::Transform::Normal, + None, + ) + .expect("geometry"); assert_eq!(geo.logical_width, 800); assert_eq!(geo.logical_height, 600); + assert_eq!(geo.verified_pixel_size(), None); + assert_eq!(geo.portal_crop_origin(800, 600), None); + } + + #[test] + fn update_from_preserves_fractional_output_pixel_size() { + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((2048, 1152)), + (2048, 1152), + 2, + wl_output::Transform::Normal, + Some((3200, 1800)), + ) + .expect("fractional output geometry"); + + assert_eq!(geo.verified_pixel_size(), Some((3200, 1800))); + } + + #[test] + fn buffer_aspect_allows_rounding_but_rejects_a_different_viewport() { + assert!(OutputGeometry::dimensions_have_compatible_aspect( + (5, 3), + (6, 4) + )); + assert!(!OutputGeometry::dimensions_have_compatible_aspect( + (3200, 1800), + (3200, 1760) + )); + } + + #[test] + fn desktop_backdrop_geometry_supplies_the_portal_output_pixel_size() { + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wl_output::Transform::Normal, + None, + ) + .expect("base geometry") + .with_desktop_backdrop_geometry(Some(DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 3, + logical_height: 2, + scale: 2, + physical_width: Some(5), + physical_height: Some(3), + crop_x: Some(0), + crop_y: Some(0), + screenshot_width: Some(5), + screenshot_height: Some(3), + })); + + assert_eq!(geo.verified_pixel_size(), Some((5, 3))); + } + + #[test] + fn unavailable_desktop_layout_clears_stale_screenshot_bounds() { + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wl_output::Transform::Normal, + Some((5, 3)), + ) + .expect("base geometry") + .with_desktop_backdrop_geometry(Some(DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 3, + logical_height: 2, + scale: 2, + physical_width: Some(5), + physical_height: Some(3), + crop_x: Some(7), + crop_y: Some(0), + screenshot_width: Some(12), + screenshot_height: Some(3), + })) + .with_desktop_backdrop_geometry(None); + + assert_eq!(geo.verified_pixel_size(), Some((5, 3))); + assert_eq!(geo.portal_crop_origin(12, 3), None); } #[test] @@ -90,6 +213,7 @@ mod tests { (800, 600), 1, wl_output::Transform::Normal, + None, ) .is_none() ); @@ -100,23 +224,52 @@ mod tests { (800, 600), 1, wl_output::Transform::Normal, + None, ) .is_none() ); assert!( - OutputGeometry::update_from(None, None, (800, 600), 0, wl_output::Transform::Normal) - .is_none() + OutputGeometry::update_from( + None, + None, + (800, 600), + 0, + wl_output::Transform::Normal, + None, + ) + .is_none() ); } } impl OutputGeometry { - /// Returns physical pixel dimensions. - pub fn physical_size(&self) -> (u32, u32) { - ( - self.logical_width.saturating_mul(self.scale as u32), - self.logical_height.saturating_mul(self.scale as u32), - ) + /// Returns compositor-reported output pixels without guessing from an + /// integer buffer scale. + pub fn verified_pixel_size(&self) -> Option<(u32, u32)> { + self.pixel_size + } + + /// Returns whether scaling source pixels into a buffer preserves aspect, + /// allowing at most one-pixel rounding from fractional-scale dimensions. + pub fn dimensions_have_compatible_aspect(source: (u32, u32), target: (u32, u32)) -> bool { + if source.0 == 0 || source.1 == 0 || target.0 == 0 || target.1 == 0 { + return false; + } + let horizontal = u64::from(source.0) * u64::from(target.1); + let vertical = u64::from(source.1) * u64::from(target.0); + let rounding_tolerance = u64::from(source.0.max(source.1).max(target.0).max(target.1)); + horizontal.abs_diff(vertical) <= rounding_tolerance + } + + /// Returns the integer-scale buffer dimensions used by the overlay surface. + pub fn buffer_size(&self) -> Option<(u32, u32)> { + Some(self.overlay_buffer_size) + } + + /// Validate post-transform pixels when the compositor's physical mode is known. + pub fn accepts_transformed_pixel_size(&self, width: u32, height: u32) -> bool { + self.pixel_size + .is_none_or(|pixel_size| pixel_size == (width, height)) } /// Returns physical pixel origin of the logical position on this output. @@ -133,11 +286,26 @@ impl OutputGeometry { ) } - pub fn with_screenshot_origin(mut self, origin: Option<(u32, u32)>) -> Self { + #[cfg(test)] + fn with_screenshot_origin(mut self, origin: Option<(u32, u32)>) -> Self { self.screenshot_origin = origin; self } + pub fn with_desktop_backdrop_geometry( + mut self, + geometry: Option, + ) -> Self { + self.screenshot_origin = None; + self.screenshot_size = None; + if let Some(geometry) = geometry { + self.pixel_size = geometry.verified_physical_size(); + self.screenshot_origin = geometry.physical_origin(); + self.screenshot_size = geometry.screenshot_size(); + } + self + } + /// Crop origin inside a portal/desktop screenshot of `image_width` × /// `image_height`. /// @@ -145,10 +313,16 @@ impl OutputGeometry { /// physical size (a single-output capture). Otherwise the origin stays /// unknown so a multi-output shot is not cropped from the first monitor. pub fn portal_crop_origin(&self, image_width: u32, image_height: u32) -> Option<(u32, u32)> { + if self + .screenshot_size + .is_some_and(|size| size != (image_width, image_height)) + { + return None; + } if let Some(origin) = self.screenshot_origin { return Some(origin); } - let (phys_width, phys_height) = self.physical_size(); + let (phys_width, phys_height) = self.verified_pixel_size()?; (image_width == phys_width && image_height == phys_height).then_some((0, 0)) } } diff --git a/src/backend/wayland/handlers/compositor.rs b/src/backend/wayland/handlers/compositor.rs index ebd61c47b..9cea01d44 100644 --- a/src/backend/wayland/handlers/compositor.rs +++ b/src/backend/wayland/handlers/compositor.rs @@ -52,6 +52,7 @@ impl CompositorHandler for WaylandState { let scale = new_factor.max(1); debug!("Scale factor changed to {}", scale); self.surface.set_scale(scale); + self.refresh_freeze_zoom_geometry(); self.buffer_damage .mark_all_full(FullDamageReason::ScaleChanged); let (phys_w, phys_h) = self.surface.physical_dimensions(); @@ -79,6 +80,7 @@ impl CompositorHandler for WaylandState { } debug!("Transform changed"); + self.refresh_freeze_zoom_geometry(); } fn frame( @@ -132,26 +134,8 @@ impl CompositorHandler for WaylandState { .mark_all_full(FullDamageReason::OutputChanged); self.toolbar.maybe_update_scale(Some(output), scale); self.toolbar.mark_dirty(); - let (logical_w, logical_h) = info - .logical_size - .unwrap_or((self.surface.width() as i32, self.surface.height() as i32)); - let (logical_x, logical_y) = info.logical_position.unwrap_or((0, 0)); - self.set_freeze_zoom_geometry(Some( - crate::backend::wayland::frozen_geometry::OutputGeometry { - logical_x, - logical_y, - logical_width: logical_w.max(0) as u32, - logical_height: logical_h.max(0) as u32, - scale, - transform: info.transform, - screenshot_origin: None, - }, - )); - self.frozen - .set_active_output(Some(output.clone()), Some(info.id)); - self.zoom - .set_active_output(Some(output.clone()), Some(info.id)); } + self.refresh_freeze_zoom_geometry(); self.frozen.unfreeze(&mut self.input_state); self.zoom.deactivate(&mut self.input_state); diff --git a/src/backend/wayland/handlers/layer.rs b/src/backend/wayland/handlers/layer.rs index d3a23f340..f1e206bf7 100644 --- a/src/backend/wayland/handlers/layer.rs +++ b/src/backend/wayland/handlers/layer.rs @@ -71,29 +71,9 @@ impl LayerShellHandler for WaylandState { self.cancel_eyedropper_if_source_missing(); self.cancel_ocr_if_source_missing(); - // Refresh active geometry for portal fallback cropping using latest logical size/scale. - let output_transform = self - .surface - .current_output() - .as_ref() - .and_then(|output| self.output_state.info(output)) - .map(|info| info.transform) - .unwrap_or(wayland_client::protocol::wl_output::Transform::Normal); - let logical_position = self - .surface - .current_output() - .as_ref() - .and_then(|output| self.output_state.info(output)) - .and_then(|info| info.logical_position); - if let Some(geo) = crate::backend::wayland::frozen_geometry::OutputGeometry::update_from( - logical_position, - Some((self.surface.width() as i32, self.surface.height() as i32)), - (self.surface.width(), self.surface.height()), - self.surface.scale(), - output_transform, - ) { - self.set_freeze_zoom_geometry(Some(geo)); - } + // Refresh active geometry for capture validation using the latest + // configured surface size and compositor output metadata. + self.refresh_freeze_zoom_geometry(); } self.surface.set_configured(true); diff --git a/src/backend/wayland/handlers/output.rs b/src/backend/wayland/handlers/output.rs index 2d26c6ffe..4158eeeb8 100644 --- a/src/backend/wayland/handlers/output.rs +++ b/src/backend/wayland/handlers/output.rs @@ -31,22 +31,8 @@ impl OutputHandler for WaylandState { if self.surface.current_output().as_ref() == Some(&output) { self.refresh_active_output_label(); } - if let Some(info) = self.output_state.info(&output) - && (self.frozen.active_output_matches(info.id) - || self.zoom.active_output_matches(info.id)) - && let Some(geo) = crate::backend::wayland::frozen_geometry::OutputGeometry::update_from( - info.logical_position, - info.logical_size, - (self.surface.width(), self.surface.height()), - info.scale_factor.max(1), - info.transform, - ) - { - self.set_freeze_zoom_geometry(Some(geo)); - self.frozen - .set_active_output(Some(output.clone()), Some(info.id)); - self.zoom - .set_active_output(Some(output.clone()), Some(info.id)); + if self.surface.current_output().as_ref() == Some(&output) { + self.refresh_freeze_zoom_geometry(); return; } // Screenshot origin walks every output, so a non-active monitor that diff --git a/src/backend/wayland/handlers/xdg.rs b/src/backend/wayland/handlers/xdg.rs index d21b7eb5b..55fb4d0ce 100644 --- a/src/backend/wayland/handlers/xdg.rs +++ b/src/backend/wayland/handlers/xdg.rs @@ -123,28 +123,7 @@ impl WindowHandler for WaylandState { self.cancel_eyedropper_if_source_missing(); self.cancel_ocr_if_source_missing(); - let output_transform = self - .surface - .current_output() - .as_ref() - .and_then(|output| self.output_state.info(output)) - .map(|info| info.transform) - .unwrap_or(wayland_client::protocol::wl_output::Transform::Normal); - let logical_position = self - .surface - .current_output() - .as_ref() - .and_then(|output| self.output_state.info(output)) - .and_then(|info| info.logical_position); - if let Some(geo) = crate::backend::wayland::frozen_geometry::OutputGeometry::update_from( - logical_position, - Some((self.surface.width() as i32, self.surface.height() as i32)), - (self.surface.width(), self.surface.height()), - self.surface.scale(), - output_transform, - ) { - self.set_freeze_zoom_geometry(Some(geo)); - } + self.refresh_freeze_zoom_geometry(); if self.xdg_frozen_fullscreen_requested() && self.frozen.has_pending_image() { if self.xdg_frozen_fullscreen_pending_configure() && !configure.is_fullscreen() { warn!("xdg frozen fullscreen was not granted; activating freeze on current size"); diff --git a/src/backend/wayland/state/capture/backdrop.rs b/src/backend/wayland/state/capture/backdrop.rs index 1d646969f..78185c2bf 100644 --- a/src/backend/wayland/state/capture/backdrop.rs +++ b/src/backend/wayland/state/capture/backdrop.rs @@ -9,15 +9,8 @@ pub(super) fn desktop_backdrop_output_geometry_from_info( if logical_width <= 0 || logical_height <= 0 { return None; } - let (physical_width, physical_height) = current_or_preferred_mode_size(info) - .map(|(width, height)| transformed_output_size(width, height, info.transform)) - .or_else(|| { - let scale = u32::try_from(info.scale_factor.max(1)).ok()?; - Some(( - u32::try_from(logical_width).ok()?.checked_mul(scale)?, - u32::try_from(logical_height).ok()?.checked_mul(scale)?, - )) - })?; + let (physical_width, physical_height) = current_mode_size(info) + .map(|(width, height)| transformed_output_size(width, height, info.transform))?; if physical_width == 0 || physical_height == 0 { return None; } @@ -32,13 +25,10 @@ pub(super) fn desktop_backdrop_output_geometry_from_info( }) } -fn current_or_preferred_mode_size( - info: &smithay_client_toolkit::output::OutputInfo, -) -> Option<(u32, u32)> { +fn current_mode_size(info: &smithay_client_toolkit::output::OutputInfo) -> Option<(u32, u32)> { info.modes .iter() .find(|mode| mode.current) - .or_else(|| info.modes.iter().find(|mode| mode.preferred)) .and_then(|mode| { Some(( u32::try_from(mode.dimensions.0).ok()?, @@ -103,31 +93,61 @@ impl WaylandState { geometry: Option, exclude: Option<&wl_output::WlOutput>, ) { - let screenshot_origin = self - .desktop_backdrop_geometry_excluding(exclude) - .and_then(DesktopBackdropGeometry::physical_origin); - let geometry = geometry.map(|geo| geo.with_screenshot_origin(screenshot_origin)); + let backdrop_geometry = self.desktop_backdrop_geometry_excluding(exclude); + let geometry = geometry.map(|geo| geo.with_desktop_backdrop_geometry(backdrop_geometry)); self.frozen.set_active_geometry(geometry.clone()); self.zoom.set_active_geometry(geometry); } pub(in crate::backend::wayland) fn refresh_freeze_zoom_screenshot_origin(&mut self) { - self.refresh_freeze_zoom_screenshot_origin_excluding(None); + self.refresh_freeze_zoom_geometry_excluding(None); } pub(in crate::backend::wayland) fn refresh_freeze_zoom_screenshot_origin_excluding( &mut self, exclude: Option<&wl_output::WlOutput>, ) { - let Some(geometry) = self - .frozen - .active_geometry() - .cloned() - .or_else(|| self.zoom.active_geometry().cloned()) - else { + self.refresh_freeze_zoom_geometry_excluding(exclude); + } + + pub(in crate::backend::wayland) fn refresh_freeze_zoom_geometry(&mut self) { + self.refresh_freeze_zoom_geometry_excluding(None); + } + + fn refresh_freeze_zoom_geometry_excluding(&mut self, exclude: Option<&wl_output::WlOutput>) { + let Some(output) = self.surface.current_output() else { + self.set_freeze_zoom_geometry_excluding(None, exclude); + self.frozen.set_active_output(None, None); + self.zoom.set_active_output(None, None); return; }; - self.set_freeze_zoom_geometry_excluding(Some(geometry), exclude); + if exclude.is_some_and(|destroyed| destroyed == &output) { + self.set_freeze_zoom_geometry_excluding(None, exclude); + self.frozen.set_active_output(None, None); + self.zoom.set_active_output(None, None); + return; + } + let Some(info) = self.output_state.info(&output) else { + self.set_freeze_zoom_geometry_excluding(None, exclude); + self.frozen.set_active_output(None, None); + self.zoom.set_active_output(None, None); + return; + }; + + let pixel_size = current_mode_size(&info) + .map(|(width, height)| transformed_output_size(width, height, info.transform)); + let geometry = OutputGeometry::update_from( + info.logical_position, + info.logical_size, + (self.surface.width(), self.surface.height()), + self.surface.scale(), + info.transform, + pixel_size, + ); + self.set_freeze_zoom_geometry_excluding(geometry, exclude); + self.frozen + .set_active_output(Some(output.clone()), Some(info.id)); + self.zoom.set_active_output(Some(output), Some(info.id)); } } diff --git a/src/backend/wayland/state/capture/pdf.rs b/src/backend/wayland/state/capture/pdf.rs index f8b1111ed..894f1b894 100644 --- a/src/backend/wayland/state/capture/pdf.rs +++ b/src/backend/wayland/state/capture/pdf.rs @@ -1,4 +1,5 @@ use super::super::*; +use crate::backend::wayland::capture::CaptureLayoutContext; use crate::input::state::{Toast, ToastPriority}; impl WaylandState { @@ -33,7 +34,18 @@ impl WaylandState { let save_config = self.board_pdf_save_config(action); if self.should_capture_desktop_for_pdf_export(action) { - let request = self.desktop_backdrop_capture_request(operation); + let Some((request, layout_context)) = self.desktop_backdrop_capture_request(operation) + else { + let message = + "Board PDF export failed: active output geometry is unavailable".to_string(); + log::error!("{message}"); + self.input_state.push_toast( + ToastPriority::Critical, + "capture.pdf", + Toast::error(message), + ); + return; + }; if !self.enter_overlay_suppression(OverlaySuppression::DesktopBackdrop) { log::warn!( "Board PDF export action {:?} requested while overlay is suppressed; ignoring", @@ -54,6 +66,7 @@ impl WaylandState { action, operation, save_config, + layout_context, }); log::info!( "Queued {:?} desktop backdrop capture for PDF export; waiting for suppression frame", @@ -99,6 +112,25 @@ impl WaylandState { return; }; + let active_output_id = self + .surface + .current_output() + .and_then(|output| self.output_state.info(&output).map(|info| info.id)); + if !pending + .layout_context + .matches(active_output_id, self.frozen.output_layout_generation()) + { + let message = + "Board PDF export failed: output layout changed during desktop capture".to_string(); + log::error!("{message}"); + self.input_state.push_toast( + ToastPriority::Critical, + "capture.pdf", + Toast::error(message), + ); + return; + } + let snapshot = match self.board_pdf_export_snapshot_with_desktop_backdrop( pending.action, CanvasExportBackdropSnapshot::PersistedImage { @@ -193,13 +225,19 @@ impl WaylandState { fn desktop_backdrop_capture_request( &self, operation: ImageOperationKind, - ) -> DesktopBackdropCaptureRequest { - DesktopBackdropCaptureRequest { + ) -> Option<(DesktopBackdropCaptureRequest, CaptureLayoutContext)> { + let output = self.surface.current_output()?; + let output_id = self.output_state.info(&output)?.id; + let geometry = self.desktop_backdrop_geometry()?; + let layout_context = + CaptureLayoutContext::new(output_id, self.frozen.output_layout_generation()); + let request = DesktopBackdropCaptureRequest { logical_width: self.surface.width(), logical_height: self.surface.height(), scale: self.surface.scale(), - geometry: self.desktop_backdrop_geometry(), + geometry: Some(geometry), operation, - } + }; + Some((request, layout_context)) } } diff --git a/src/backend/wayland/state/screen_image.rs b/src/backend/wayland/state/screen_image.rs index 877c7a677..6138277cd 100644 --- a/src/backend/wayland/state/screen_image.rs +++ b/src/backend/wayland/state/screen_image.rs @@ -451,7 +451,9 @@ mod tests { for value in [1u8, 2, 3, 4, 5, 6] { data.extend_from_slice(&[value, 0, 0, 0xFF]); } - let rotated = image(3, 2, 12, data).with_output_transform(wl_output::Transform::_270); + let rotated = image(3, 2, 12, data) + .with_output_transform(wl_output::Transform::_270) + .expect("valid transform"); assert_eq!((rotated.width, rotated.height), (2, 3)); let crop = copy_image_rect( diff --git a/src/backend/wayland/zoom/capture.rs b/src/backend/wayland/zoom/capture.rs index 9e29069ff..d555af4a3 100644 --- a/src/backend/wayland/zoom/capture.rs +++ b/src/backend/wayland/zoom/capture.rs @@ -13,7 +13,8 @@ use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_frame_v1::{ }; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; -use crate::backend::wayland::frozen::FrozenImage; +use crate::backend::wayland::frozen::{FrozenImage, copy_shm_argb, validate_shm_buffer_layout}; +use crate::backend::wayland::frozen_geometry::OutputGeometry; use crate::input::InputState; use super::state::ZoomState; @@ -29,10 +30,11 @@ pub(super) struct CaptureSession { format: Option, y_invert: bool, copy_requested: bool, + context: CaptureContext, } impl CaptureSession { - fn new(frame: ZwlrScreencopyFrameV1) -> Self { + fn new(frame: ZwlrScreencopyFrameV1, context: CaptureContext) -> Self { Self { frame, pool: None, @@ -43,6 +45,7 @@ impl CaptureSession { format: None, y_invert: false, copy_requested: false, + context, } } @@ -70,6 +73,61 @@ impl CaptureSession { } } +struct CaptureContext { + target_output_id: u32, + source_geometry: Option, + layout_generation: u64, +} + +impl CaptureContext { + fn new( + target_output_id: u32, + source_geometry: Option, + layout_generation: u64, + ) -> Self { + Self { + target_output_id, + source_geometry, + layout_generation, + } + } + + fn output_and_layout_match( + &self, + active_output_id: Option, + layout_generation: u64, + ) -> bool { + active_output_id == Some(self.target_output_id) + && layout_generation == self.layout_generation + } +} + +fn finalize_capture_image(image: FrozenImage, context: &CaptureContext) -> Result { + let output_transform = context + .source_geometry + .as_ref() + .map(|geometry| geometry.transform) + .unwrap_or(wl_output::Transform::Normal); + let image = image.with_output_transform(output_transform)?; + if context + .source_geometry + .as_ref() + .is_some_and(|geometry| !geometry.accepts_transformed_pixel_size(image.width, image.height)) + { + anyhow::bail!("Zoom capture dimensions do not match the active output"); + } + let buffer_size = context + .source_geometry + .as_ref() + .and_then(OutputGeometry::buffer_size) + .context("Zoom capture has no overlay buffer geometry")?; + if !OutputGeometry::dimensions_have_compatible_aspect((image.width, image.height), buffer_size) + { + anyhow::bail!("Zoom capture aspect does not match the overlay surface"); + } + Ok(image) +} + impl ZoomState { /// Start a screencopy capture for the active output. pub fn start_capture( @@ -125,13 +183,25 @@ impl ZoomState { anyhow::bail!("No active output available for zoom capture"); } }; + let target_output_id = self + .active_output_id + .context("Active output has no stable identity for zoom capture")?; + let source_geometry = self + .active_geometry + .clone() + .context("Active output geometry is unavailable for zoom capture")?; + let context = CaptureContext::new( + target_output_id, + Some(source_geometry), + self.output_layout_generation, + ); info!( "capture.preflight component=zoom phase=screencopy-request output_id={:?}", self.active_output_id ); let frame = manager.capture_output(0, &output, qh, ()); - self.capture = Some(CaptureSession::new(frame)); + self.capture = Some(CaptureSession::new(frame, context)); if let Some(capture) = self.capture.as_mut() { capture.pool = Some(SlotPool::new(4, shm).context("Failed to create zoom pool")?); @@ -177,10 +247,18 @@ impl ZoomState { } } FrameEvent::Ready { .. } => { - if let Err(err) = self.on_ready() { - warn!("Zoom capture ready handling failed: {}", err); - self.cancel(input_state, false); - return; + match self.on_ready() { + Ok(true) => {} + Ok(false) => { + warn!("Zoom capture discarded after the output layout changed"); + self.cancel(input_state, false); + return; + } + Err(err) => { + warn!("Zoom capture ready handling failed: {}", err); + self.cancel(input_state, false); + return; + } } if self.pending_activation { @@ -228,18 +306,18 @@ impl ZoomState { } }; + 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); let pool = capture.pool.as_mut().context("Zoom 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 zoom buffer")?; capture.buffer = Some(buffer); capture.request_copy(); @@ -255,66 +333,124 @@ impl ZoomState { 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("Zoom pool missing")?; - let buffer = capture.buffer.as_ref().context("Zoom buffer missing")?; + let result = (|| { + let pool = capture.pool.as_mut().context("Zoom pool missing")?; + let buffer = capture.buffer.as_ref().context("Zoom buffer missing")?; + let canvas = buffer.canvas(pool).context("Unable to map zoom buffer")?; + let format = capture.format.context("Zoom format missing")?; + copy_shm_argb( + canvas, + capture.width, + capture.height, + capture.stride, + format, + capture.y_invert, + ) + })(); + capture.frame.destroy(); + let image = result?; + if !capture + .context + .output_and_layout_match(self.active_output_id, self.output_layout_generation) + { + return Ok(false); + } - let canvas = buffer.canvas(pool).context("Unable to map zoom buffer")?; + self.set_image(finalize_capture_image(image, &capture.context)?); - let pixel_width = (capture.width * 4) as usize; - let stride = capture.stride as usize; - if stride < pixel_width { - anyhow::bail!("Zoom stride smaller than expected pixel width"); - } + Ok(true) + } +} - let mut data = vec![0u8; (capture.width * capture.height * 4) as usize]; +#[cfg(test)] +mod tests { + use super::*; - 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); - } + #[test] + fn capture_context_rejects_output_or_layout_changes() { + let context = CaptureContext::new(7, None, 3); - if matches!(capture.format, Some(wl_shm::Format::Xrgb8888)) { - for chunk in data.chunks_exact_mut(4) { - chunk[3] = 0xFF; - } - } + assert!(context.output_and_layout_match(Some(7), 3)); + assert!(!context.output_and_layout_match(Some(8), 3)); + assert!(!context.output_and_layout_match(Some(7), 4)); + } - capture.frame.destroy(); + #[test] + fn finalized_capture_rejects_known_pixel_size_mismatch() { + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wl_output::Transform::Normal, + Some((5, 3)), + ) + .expect("known output geometry"); + let context = CaptureContext::new(7, Some(geometry), 3); + let image = crate::backend::wayland::frozen::FrozenImage { + width: 4, + height: 3, + stride: 16, + data: vec![0; 4 * 3 * 4], + }; - let output_transform = self - .active_geometry - .as_ref() - .map(|geo| geo.transform) - .unwrap_or(wl_output::Transform::Normal); - - self.set_image( - FrozenImage { - width: capture.width, - height: capture.height, - stride: (capture.width * 4) as i32, - data, - } - .with_output_transform(output_transform), - ); + assert!(finalize_capture_image(image, &context).is_err()); + } - Ok(()) + #[test] + fn finalized_capture_applies_transform_before_size_validation() { + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((2, 3)), + (2, 3), + 1, + wl_output::Transform::_270, + Some((2, 3)), + ) + .expect("rotated output geometry"); + let context = CaptureContext::new(7, Some(geometry), 3); + let image = crate::backend::wayland::frozen::FrozenImage { + width: 3, + height: 2, + stride: 12, + data: vec![0; 3 * 2 * 4], + }; + + let image = finalize_capture_image(image, &context).expect("transformed image"); + assert_eq!((image.width, image.height), (2, 3)); } -} -#[cfg(test)] -mod tests { - use super::*; + #[test] + fn finalized_capture_rejects_stretching_into_a_different_viewport() { + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((3200, 1800)), + (3200, 1760), + 1, + wl_output::Transform::Normal, + Some((3200, 1800)), + ) + .expect("known output geometry"); + let context = CaptureContext::new(7, Some(geometry), 3); + let image = FrozenImage { + width: 3200, + height: 1800, + stride: 12_800, + data: vec![0; 3200 * 1800 * 4], + }; + + let error = match finalize_capture_image(image, &context) { + Err(error) => error, + Ok(_) => panic!("a full output cannot be stretched into a shorter viewport"), + }; + assert!(error.to_string().contains("aspect does not match")); + } #[tokio::test] async fn portal_capture_waits_for_suppression_preflight() { diff --git a/src/backend/wayland/zoom/portal.rs b/src/backend/wayland/zoom/portal.rs index 047ef2d7b..90081ff37 100644 --- a/src/backend/wayland/zoom/portal.rs +++ b/src/backend/wayland/zoom/portal.rs @@ -3,6 +3,7 @@ use log::warn; use std::time::{Duration, Instant}; use crate::backend::wayland::frozen::FrozenImage; +use crate::backend::wayland::frozen_geometry::OutputGeometry; use crate::backend::wayland::portal_capture::{ capture_via_portal_fullscreen_bytes, crop_argb, portal_output_matches, }; @@ -27,11 +28,18 @@ impl ZoomState { .runtime_wake .clone() .ok_or_else(|| anyhow::anyhow!("portal capture runtime wake is unavailable"))?; + let geo = self.active_geometry.clone().ok_or_else(|| { + anyhow::anyhow!("active output geometry is unavailable for portal zoom capture") + })?; + geo.verified_pixel_size().ok_or_else(|| { + anyhow::anyhow!("active output pixel size is unavailable for portal zoom capture") + })?; + let target_output_id = self.active_output_id.ok_or_else(|| { + anyhow::anyhow!("active output identity is unavailable for portal zoom capture") + })?; self.portal_in_progress = true; - self.portal_target_output_id = self.active_output_id; + self.portal_target_output_id = Some(target_output_id); - let geo = self.active_geometry.clone(); - let target_output_id = self.active_output_id; let layout_generation = self.output_layout_generation; crate::notification::send_notification_async( tokio_handle, @@ -43,43 +51,11 @@ impl ZoomState { async { let bytes = capture_via_portal_fullscreen_bytes().await?; - let (mut data, mut width, mut height) = decode_image_to_argb(&bytes) + let (data, width, height) = decode_image_to_argb(&bytes) .map_err(|error| CaptureError::ImageError(format!("Decode failed: {error}")))?; + let image = crop_portal_image(data, width, height, &geo)?; - if let Some(geo) = geo { - let (phys_w, phys_h) = geo.physical_size(); - let Some((origin_x, origin_y)) = geo.portal_crop_origin(width, height) else { - return Err(CaptureError::ImageError( - "Zoom capture does not contain the active output".to_string(), - )); - }; - let Some((cropped_w, cropped_h, cropped)) = - crop_argb(&data, width, height, origin_x, origin_y, phys_w, phys_h) - else { - return Err(CaptureError::ImageError( - "Zoom capture does not contain the active output".to_string(), - )); - }; - if cropped_w != phys_w || cropped_h != phys_h { - return Err(CaptureError::ImageError( - "Zoom capture does not contain the active output".to_string(), - )); - } - data = cropped; - width = cropped_w; - height = cropped_h; - } - - Ok(( - target_output_id, - layout_generation, - FrozenImage { - width, - height, - stride: (width * 4) as i32, - data, - }, - )) + Ok((Some(target_output_id), layout_generation, image)) } .await })); @@ -187,6 +163,61 @@ impl ZoomState { } } +fn crop_portal_image( + data: Vec, + width: u32, + height: u32, + geometry: &OutputGeometry, +) -> Result { + let expected_len = u64::from(width) + .checked_mul(u64::from(height)) + .and_then(|pixels| pixels.checked_mul(4)) + .and_then(|bytes| usize::try_from(bytes).ok()) + .ok_or_else(|| CaptureError::ImageError("Zoom capture is too large".to_string()))?; + if data.len() != expected_len { + return Err(CaptureError::ImageError( + "Zoom capture buffer length does not match its dimensions".to_string(), + )); + } + + let (phys_w, phys_h) = geometry.verified_pixel_size().ok_or_else(|| { + CaptureError::ImageError("Zoom capture output dimensions are invalid".to_string()) + })?; + let buffer_size = geometry.buffer_size().ok_or_else(|| { + CaptureError::ImageError("Zoom capture overlay dimensions are invalid".to_string()) + })?; + if !OutputGeometry::dimensions_have_compatible_aspect((phys_w, phys_h), buffer_size) { + return Err(CaptureError::ImageError( + "Zoom capture aspect does not match the overlay surface".to_string(), + )); + } + let (origin_x, origin_y) = geometry.portal_crop_origin(width, height).ok_or_else(|| { + CaptureError::ImageError("Zoom capture does not match the active output layout".to_string()) + })?; + let (cropped_w, cropped_h, cropped) = + crop_argb(&data, width, height, origin_x, origin_y, phys_w, phys_h).ok_or_else(|| { + CaptureError::ImageError("Zoom capture does not contain the active output".to_string()) + })?; + if cropped_w != phys_w || cropped_h != phys_h { + return Err(CaptureError::ImageError( + "Zoom capture does not contain the active output".to_string(), + )); + } + let stride = i32::try_from( + cropped_w + .checked_mul(4) + .ok_or_else(|| CaptureError::ImageError("Zoom capture stride overflow".to_string()))?, + ) + .map_err(|_| CaptureError::ImageError("Zoom capture stride is too large".to_string()))?; + + Ok(FrozenImage { + width: cropped_w, + height: cropped_h, + stride, + data: cropped, + }) +} + #[cfg(test)] mod tests { use super::*; @@ -212,10 +243,88 @@ mod tests { logical_height: 1, scale: 1, transform: wayland_client::protocol::wl_output::Transform::Normal, + overlay_buffer_size: (2, 1), + pixel_size: Some((2, 1)), screenshot_origin: Some(origin), + screenshot_size: None, } } + #[tokio::test] + async fn portal_start_requires_verifiable_geometry_and_output_identity() -> anyhow::Result<()> { + let wake = crate::backend::wayland::RuntimeWakeSource::new()?; + let mut zoom = ZoomState::new_with_runtime_wake(None, wake.handle()); + + let error = zoom + .capture_via_portal(&tokio::runtime::Handle::current()) + .expect_err("missing geometry must fail closed"); + assert!(error.to_string().contains("geometry is unavailable")); + assert!(!zoom.portal_in_progress); + assert!(zoom.portal_task.is_none()); + + zoom.set_active_geometry(Some(crop_geometry((0, 0)))); + let error = zoom + .capture_via_portal(&tokio::runtime::Handle::current()) + .expect_err("missing output identity must fail closed"); + assert!(error.to_string().contains("identity is unavailable")); + assert!(!zoom.portal_in_progress); + assert!(zoom.portal_task.is_none()); + Ok(()) + } + + #[test] + fn portal_crop_uses_fractional_output_pixels_not_the_overlay_buffer_size() { + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wayland_client::protocol::wl_output::Transform::Normal, + Some((5, 3)), + ) + .expect("fractional geometry") + .with_desktop_backdrop_geometry(Some(crate::capture::DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 3, + logical_height: 2, + scale: 2, + physical_width: Some(5), + physical_height: Some(3), + crop_x: Some(0), + crop_y: Some(0), + screenshot_width: Some(5), + screenshot_height: Some(3), + })); + + let image = crop_portal_image(vec![7; 5 * 3 * 4], 5, 3, &geometry) + .expect("fractional output screenshot"); + + assert_eq!((image.width, image.height), (5, 3)); + assert_eq!(image.stride, 20); + } + + #[test] + fn portal_crop_rejects_a_different_screenshot_layout() { + let geometry = crop_geometry((0, 0)).with_desktop_backdrop_geometry(Some( + crate::capture::DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 2, + logical_height: 1, + scale: 1, + physical_width: Some(2), + physical_height: Some(1), + crop_x: Some(0), + crop_y: Some(0), + screenshot_width: Some(2), + screenshot_height: Some(1), + }, + )); + + assert!(crop_portal_image(vec![0; 3 * 4], 3, 1, &geometry).is_err()); + } + async fn poll_until_finished(zoom: &mut ZoomState, input: &mut InputState) { for _ in 0..100 { zoom.poll_portal_capture(input, Instant::now()); diff --git a/src/backend/wayland/zoom/state.rs b/src/backend/wayland/zoom/state.rs index 092fcf434..dc3d4e4df 100644 --- a/src/backend/wayland/zoom/state.rs +++ b/src/backend/wayland/zoom/state.rs @@ -21,6 +21,7 @@ pub struct ZoomState { pub(super) output_layout_generation: u64, pub(super) capture: Option, pub(super) image: Option, + pub(super) image_target_dimensions: Option<(u32, u32)>, image_generation: u64, pub(super) portal_task: Option>, pub(super) portal_in_progress: bool, @@ -63,6 +64,7 @@ impl ZoomState { output_layout_generation: 0, capture: None, image: None, + image_target_dimensions: None, image_generation: 0, portal_task: None, portal_in_progress: false, @@ -97,14 +99,6 @@ impl ZoomState { self.active_geometry = geometry; } - pub fn active_geometry(&self) -> Option<&OutputGeometry> { - self.active_geometry.as_ref() - } - - pub fn active_output_matches(&self, info_id: u32) -> bool { - self.active_output_id == Some(info_id) - } - pub fn image(&self) -> Option<&FrozenImage> { self.image.as_ref() } @@ -114,12 +108,18 @@ impl ZoomState { } pub fn set_image(&mut self, image: FrozenImage) { + self.image_target_dimensions = self + .active_geometry + .as_ref() + .and_then(OutputGeometry::buffer_size) + .or(Some((image.width, image.height))); self.image = Some(image); self.bump_image_generation(); } pub fn clear_image(&mut self) -> bool { let had_image = self.image.take().is_some(); + self.image_target_dimensions = None; if had_image { self.bump_image_generation(); } diff --git a/src/backend/wayland/zoom/view.rs b/src/backend/wayland/zoom/view.rs index a9384e4af..9ebaf63ed 100644 --- a/src/backend/wayland/zoom/view.rs +++ b/src/backend/wayland/zoom/view.rs @@ -77,11 +77,49 @@ impl ZoomState { phys_height: u32, input_state: &mut InputState, ) { - if let Some(img) = &self.image - && (img.width != phys_width || img.height != phys_height) + if let Some(target_dimensions) = self.image_target_dimensions + && target_dimensions != (phys_width, phys_height) { info!("Surface resized; clearing zoom image"); self.deactivate(input_state); } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::backend::wayland::frozen::FrozenImage; + use crate::backend::wayland::frozen_geometry::OutputGeometry; + use crate::input::state::test_support::make_test_input_state; + use wayland_client::protocol::wl_output; + + #[test] + fn fractional_output_image_tracks_the_overlay_buffer_size() { + let mut zoom = ZoomState::new(None); + let mut input = make_test_input_state(); + zoom.set_active_geometry(OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wl_output::Transform::Normal, + Some((5, 3)), + )); + zoom.set_image(FrozenImage { + width: 5, + height: 3, + stride: 20, + data: vec![0; 5 * 3 * 4], + }); + zoom.activate_without_capture(); + + zoom.handle_resize(6, 4, &mut input); + assert!(zoom.image().is_some()); + assert!(zoom.active); + + zoom.handle_resize(7, 4, &mut input); + assert!(zoom.image().is_none()); + assert!(!zoom.active); + } +} diff --git a/src/capture/desktop_backdrop.rs b/src/capture/desktop_backdrop.rs index 9e50da85a..b4f39548a 100644 --- a/src/capture/desktop_backdrop.rs +++ b/src/capture/desktop_backdrop.rs @@ -6,6 +6,7 @@ use crate::capture::{ dependencies::CaptureDependencies, types::{ CaptureError, CaptureType, DesktopBackdropCaptureRequest, DesktopBackdropCaptureResult, + DesktopBackdropGeometry, }, }; use crate::image_decode::{decode_rgba, format_from_mime_or_bytes}; @@ -75,6 +76,17 @@ pub(crate) fn desktop_backdrop_from_argb( let (logical_width, logical_height, expected_width, expected_height) = expected_backdrop_dimensions(request)?; + if let Some(expected_layout_size) = request + .geometry + .and_then(DesktopBackdropGeometry::screenshot_size) + && (width, height) != expected_layout_size + { + return Err(CaptureError::ImageError(format!( + "Desktop backdrop capture {width}x{height} does not match output layout {}x{}", + expected_layout_size.0, expected_layout_size.1 + ))); + } + if width == expected_width && height == expected_height { return desktop_backdrop_result(data, width, height, logical_width, logical_height); } @@ -126,6 +138,13 @@ fn expected_backdrop_dimensions( } if let Some(geometry) = request.geometry { + if (request.logical_width, request.logical_height) + != (geometry.logical_width, geometry.logical_height) + { + return Err(CaptureError::ImageError( + "Desktop backdrop surface does not match active output geometry".to_string(), + )); + } let (width, height) = geometry.physical_size().ok_or_else(|| { CaptureError::ImageError("Active output dimensions are too large".to_string()) })?; diff --git a/src/capture/tests/desktop_backdrop.rs b/src/capture/tests/desktop_backdrop.rs index 4a1b21e29..0edd1cf35 100644 --- a/src/capture/tests/desktop_backdrop.rs +++ b/src/capture/tests/desktop_backdrop.rs @@ -45,6 +45,8 @@ fn desktop_backdrop_accepts_fractional_scale_output_mode_size() { physical_height: Some(3), crop_x: Some(0), crop_y: Some(0), + screenshot_width: None, + screenshot_height: None, }; let result = @@ -56,6 +58,29 @@ fn desktop_backdrop_accepts_fractional_scale_output_mode_size() { assert_eq!(result.logical_to_image_scale_y, 1.5); } +#[test] +fn desktop_backdrop_rejects_a_surface_that_is_not_the_active_output() { + let geometry = DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 4, + logical_height: 2, + scale: 1, + physical_width: Some(4), + physical_height: Some(2), + crop_x: Some(0), + crop_y: Some(0), + screenshot_width: Some(4), + screenshot_height: Some(2), + }; + + let error = + desktop_backdrop_from_argb(vec![0; 4 * 2 * 4], 4, 2, &request(3, 2, 1, Some(geometry))) + .expect_err("a partial-output surface cannot map the full output backdrop exactly"); + + assert!(error.to_string().contains("does not match active output")); +} + #[test] fn desktop_backdrop_crops_multi_output_capture_to_active_output() { let mut data = Vec::new(); @@ -72,6 +97,8 @@ fn desktop_backdrop_crops_multi_output_capture_to_active_output() { physical_height: None, crop_x: Some(2), crop_y: Some(0), + screenshot_width: None, + screenshot_height: None, }; let result = @@ -95,6 +122,8 @@ fn desktop_backdrop_crop_failure_returns_error() { physical_height: None, crop_x: Some(2), crop_y: Some(0), + screenshot_width: None, + screenshot_height: None, }; let err = desktop_backdrop_from_argb(data, 3, 2, &request(2, 2, 1, Some(geometry))) @@ -120,12 +149,36 @@ fn desktop_backdrop_crops_mixed_scale_output_using_screenshot_origin() { desktop_backdrop_from_argb(data, 10, 1, &request(4, 1, 1, Some(geometry))).expect("crop"); assert_eq!(geometry.physical_origin(), Some((6, 0))); + assert_eq!(geometry.screenshot_size(), Some((10, 1))); assert_eq!( result.data.as_ref(), &[6, 6, 6, 255, 7, 7, 7, 255, 8, 8, 8, 255, 9, 9, 9, 255] ); } +#[test] +fn desktop_backdrop_rejects_layout_bounds_that_exceed_supported_coordinates() { + let output = output(i32::MAX - 1, 0, 4, 1, 4, 1); + + assert!(DesktopBackdropGeometry::from_outputs(output, &[output], 1).is_none()); +} + +#[test] +fn desktop_backdrop_rejects_a_screenshot_whose_layout_size_does_not_match() { + let outputs = [output(-4, 0, 4, 1, 6, 1), output(0, 0, 4, 1, 4, 1)]; + let geometry = + DesktopBackdropGeometry::from_outputs(outputs[1], &outputs, 1).expect("desktop geometry"); + let data = vec![0u8; 11 * 4]; + + let err = desktop_backdrop_from_argb(data, 11, 1, &request(4, 1, 1, Some(geometry))) + .expect_err("a wider image cannot use the crop origin from a different layout"); + + assert!( + err.to_string().contains("does not match output layout"), + "unexpected error: {err}" + ); +} + #[test] fn desktop_backdrop_single_output_at_nonzero_logical_origin_crops_at_zero() { let outputs = [output(10, 20, 4, 2, 8, 4)]; @@ -162,12 +215,12 @@ fn desktop_backdrop_crops_rotated_output_using_transformed_size() { let geometry = DesktopBackdropGeometry::from_outputs(outputs[0], &outputs, 1).expect("rotated output"); let mut data = Vec::new(); - for pixel in 0u8..12 { + for pixel in 0u8..24 { data.extend_from_slice(&[pixel, pixel, pixel, 255]); } let result = - desktop_backdrop_from_argb(data, 6, 2, &request(2, 4, 1, Some(geometry))).expect("crop"); + desktop_backdrop_from_argb(data, 6, 4, &request(2, 4, 1, Some(geometry))).expect("crop"); assert_eq!(geometry.physical_size(), Some((4, 2))); assert_eq!(geometry.physical_origin(), Some((0, 0))); diff --git a/src/capture/types.rs b/src/capture/types.rs index 240572a3f..6c4f39e7d 100644 --- a/src/capture/types.rs +++ b/src/capture/types.rs @@ -198,6 +198,8 @@ pub struct DesktopBackdropGeometry { pub physical_height: Option, pub crop_x: Option, pub crop_y: Option, + pub screenshot_width: Option, + pub screenshot_height: Option, } impl DesktopBackdropGeometry { @@ -224,6 +226,8 @@ impl DesktopBackdropGeometry { Axis::Vertical, outputs, )?), + screenshot_width: Some(physical_axis_size(Axis::Horizontal, outputs)?), + screenshot_height: Some(physical_axis_size(Axis::Vertical, outputs)?), }) } @@ -242,9 +246,21 @@ impl DesktopBackdropGeometry { )) } + pub fn verified_physical_size(self) -> Option<(u32, u32)> { + let width = self.physical_width?; + let height = self.physical_height?; + (width > 0 && height > 0).then_some((width, height)) + } + pub fn physical_origin(self) -> Option<(u32, u32)> { Some((self.crop_x?, self.crop_y?)) } + + pub fn screenshot_size(self) -> Option<(u32, u32)> { + let width = self.screenshot_width?; + let height = self.screenshot_height?; + (width > 0 && height > 0).then_some((width, height)) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -322,6 +338,16 @@ fn physical_axis_origin( Some(physical_origin.round() as u32) } +fn physical_axis_size(axis: Axis, outputs: &[DesktopBackdropOutputGeometry]) -> Option { + let max_end = outputs + .iter() + .map(|output| axis_span(*output, axis).map(|span| span.logical_end)) + .collect::>>()? + .into_iter() + .max()?; + physical_axis_origin(i32::try_from(max_end).ok()?, axis, outputs) +} + #[derive(Debug, Clone, Copy)] struct AxisSpan { logical_start: i64, From 971a10eaf4fe42ad54299e162fec598212adda06 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Wed, 19 Aug 2026 15:27:15 +0200 Subject: [PATCH 2/2] fix stale freeze and zoom captures Require verified output geometry and reject captures when output identity or layout changes, preventing shifted or reused pixels across direct, portal, and PDF backdrop paths. --- CHANGELOG.md | 19 - CONTRIBUTING.md | 13 +- docs/SETUP.md | 2 +- docs/codebase-overview.md | 2 +- .../wayland/backend/event_loop/capture.rs | 5 +- src/backend/wayland/capture.rs | 4 + src/backend/wayland/frozen/capture.rs | 30 +- src/backend/wayland/frozen/ext_image_copy.rs | 22 +- src/backend/wayland/frozen/portal.rs | 22 +- src/backend/wayland/frozen/state.rs | 488 +++++++++++++----- src/backend/wayland/frozen_geometry.rs | 216 +++++++- src/backend/wayland/handlers/output.rs | 10 +- src/backend/wayland/portal_capture.rs | 21 +- src/backend/wayland/state/capture.rs | 3 +- src/backend/wayland/state/capture/backdrop.rs | 39 +- src/backend/wayland/state/capture/barrier.rs | 10 + src/backend/wayland/state/core/accessors.rs | 11 +- src/backend/wayland/zoom/capture.rs | 108 ++-- src/backend/wayland/zoom/portal.rs | 90 +++- src/backend/wayland/zoom/state.rs | 79 ++- src/capture/desktop_backdrop.rs | 74 ++- src/capture/tests/desktop_backdrop.rs | 81 ++- src/capture/tests/manager.rs | 17 +- src/capture/types.rs | 18 - 24 files changed, 1006 insertions(+), 378 deletions(-) delete mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index e92bcc9a8..000000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,19 +0,0 @@ -# Changelog - -## Unreleased - -### Added - -- About dialog: a "Report a problem" row. It copies your diagnostics to the clipboard and opens `https://wayscriber.com/report`, which hands them straight into a prefilled bug form. Nothing is sent automatically, and the diagnostics travel in the URL fragment, so they never reach a server log. -- Automatic guidance toasts now provide explicit "Got it" and "Tip settings…" controls. The General UI setting can disable automatic tips without disabling manual tours or real warnings. - -### Breaking (Rust source) - -- The unified top toolbar is now the only layout. Panel-era typed config fields and order groups are removed from the serde model (`side_*` placement/pin/pane keys, `show_settings_section` / mode overrides, and `ui.toolbar.items.order.{side_sections,actions,pages,boards,presets,tool_options,sessions}`). Authored values at those exact paths remain in `config.toml` as retired settings and no longer affect the overlay. Matching keys under `runtime-ui.toml`'s recognized `item_order` map are pruned on rewrite. -- Public Rust types that described the side palette / panel-only toolbar order groups are gone. Downstream crates that constructed those fields must drop them; the in-repo configurator already matches this shape. - -### Fixed - -- Stylus pressure no longer overrides the selected Marker/Textmarker or Step Marker size. Pressure-to-thickness mapping remains limited to pressure-sensitive freehand Pen strokes. -- Automatic tips and skipped-default shortcut notices now remember acknowledgement and taught-feature use instead of returning every active launch. Persistence failures are surfaced instead of causing repeat loops. -- First-run onboarding no longer requires the radial-menu flick-to-commit exercise. Saved sessions paused on that retired step continue at the reference step. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8992ebd38..4afe334cb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -143,10 +143,15 @@ on real hardware; this checklist is manual evidence, not CI coverage. | Sway | [ ] | [ ] | [ ] | | GNOME | [ ] | [ ] | [ ] | -For each cell, record the selected capture backend from the logs. Verify that Freeze, Zoom, a -screenshot, and a PDF desktop backdrop all use the active output's exact pixels. Repeat while -switching outputs and, where supported, changing scale or transform during capture: the result must -either stay exact or fail visibly, never shift, stretch, or reuse another output's image. +For each cell, record the selected capture backend from the logs. Verify that Freeze, Zoom, +and a PDF desktop backdrop all use the active output's exact pixels. User full-screen +screenshots remain whole-desktop captures. Repeat while switching outputs and, where supported, +changing scale or transform during capture: the result must either stay exact or fail visibly, +never shift, stretch, or reuse another output's image. + +These captures now fail when the active output does not advertise a current `wl_output` mode. +Plugging or unplugging any monitor also cancels in-flight Freeze and Zoom captures on unrelated +outputs, because output count is part of the layout identity used to reject stale frames. `./tools/code-health-report.sh` reports navigational maintainability metrics. Its CI artifact is observational, not a global file/function-size gate; use the report to find code worth understanding, diff --git a/docs/SETUP.md b/docs/SETUP.md index b842ca0b9..b90fdeafa 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 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. On mixed-DPI or multi-monitor layouts, Wayscriber accepts a portal image only when it can validate the active output's crop; otherwise Freeze fails instead of guessing at an origin or scale. +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. On mixed-DPI or multi-monitor layouts, Wayscriber accepts a portal image only when it can validate the active output's crop; otherwise Freeze fails instead of guessing at an origin or scale. Freeze and Zoom also require the active output to advertise a current mode. Connecting or disconnecting a monitor cancels any in-flight Freeze or Zoom capture, including on screens that did not change. 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 3ac017ec1..2ad5a19b9 100644 --- a/docs/codebase-overview.md +++ b/docs/codebase-overview.md @@ -76,7 +76,7 @@ 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. +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. Direct capture and portal crop both require compositor-reported output pixels; a missing current mode fails instead of guessing from the overlay buffer. --- diff --git a/src/backend/wayland/backend/event_loop/capture.rs b/src/backend/wayland/backend/event_loop/capture.rs index 1a4528d43..a35fb4383 100644 --- a/src/backend/wayland/backend/event_loop/capture.rs +++ b/src/backend/wayland/backend/event_loop/capture.rs @@ -16,7 +16,10 @@ pub(super) fn poll_portal_captures(state: &mut WaylandState, now: Instant) { .frozen .poll_portal_capture(&mut state.input_state, now); handle_pending_frozen_image(state, now); - state.zoom.poll_portal_capture(&mut state.input_state, now); + let live_output_count = state.live_output_count(); + state + .zoom + .poll_portal_capture(&mut state.input_state, now, live_output_count); // Portal completion can make the capture controller idle before dispatch. // Release its overlay suppression now so the normal blocking dispatch does // not wait forever for a wake that has already been consumed. diff --git a/src/backend/wayland/capture.rs b/src/backend/wayland/capture.rs index 1db598eb7..1c35df29f 100644 --- a/src/backend/wayland/capture.rs +++ b/src/backend/wayland/capture.rs @@ -57,6 +57,10 @@ impl CaptureLayoutContext { } } + pub(in crate::backend::wayland) fn target_output_id(self) -> u32 { + self.target_output_id + } + pub(in crate::backend::wayland) fn matches( self, active_output_id: Option, diff --git a/src/backend/wayland/frozen/capture.rs b/src/backend/wayland/frozen/capture.rs index a32815291..616b64c20 100644 --- a/src/backend/wayland/frozen/capture.rs +++ b/src/backend/wayland/frozen/capture.rs @@ -21,6 +21,8 @@ use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_frame_v1::{ }; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; +use crate::backend::wayland::capture::CaptureLayoutContext; +use crate::backend::wayland::frozen_geometry::require_verified_capture_source; use crate::input::InputState; use super::image::{copy_shm_argb, validate_shm_buffer_layout}; @@ -91,6 +93,7 @@ impl FrozenState { self.preferred_backend() .context("no frozen capture backend is available")?, ); + self.snapshot_preflight_layout(); self.preflight_pending = true; Ok(()) } @@ -155,6 +158,8 @@ impl FrozenState { + Dispatch + 'static, { + self.ensure_preflight_layout_current() + .map_err(anyhow::Error::msg)?; let mut backend = Some(first_backend); let mut last_error = None; @@ -224,10 +229,12 @@ 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 (source_geometry, target_output_id) = require_verified_capture_source( + self.active_geometry.clone(), + self.active_output_id, + "frozen capture", + ) + .map_err(anyhow::Error::msg)?; let pool = SlotPool::new(4, shm).context("Failed to create frozen capture pool")?; debug!("Requesting screencopy frame for active output"); @@ -237,9 +244,8 @@ impl FrozenState { self.direct_capture = Some(DirectCaptureAttempt::WlrScreencopy { session: Box::new(capture), context: DirectCaptureContext::new( - target_output_id, + CaptureLayoutContext::new(target_output_id, self.output_layout_generation), source_geometry, - self.output_layout_generation, ), }); @@ -408,11 +414,18 @@ impl FrozenState { })(); capture.frame.destroy(); let image = result?; - if !context.output_and_layout_match(self.active_output_id, self.output_layout_generation) { + if !context + .layout + .matches(self.active_output_id, self.output_layout_generation) + { return Ok(false); } - self.set_pending_output_image(image, context.target_output_id, context.source_geometry); + self.set_pending_output_image( + image, + context.layout.target_output_id(), + context.source_geometry, + ); Ok(true) } @@ -421,5 +434,6 @@ impl FrozenState { self.capture_done = true; input_state.set_frozen_active(false); input_state.needs_redraw = true; + Self::push_stale_layout_toast(input_state); } } diff --git a/src/backend/wayland/frozen/ext_image_copy.rs b/src/backend/wayland/frozen/ext_image_copy.rs index 41212e4cc..48f08d3e5 100644 --- a/src/backend/wayland/frozen/ext_image_copy.rs +++ b/src/backend/wayland/frozen/ext_image_copy.rs @@ -22,6 +22,8 @@ use wayland_protocols::ext::{ }, }; +use crate::backend::wayland::capture::CaptureLayoutContext; +use crate::backend::wayland::frozen_geometry::require_verified_capture_source; use crate::input::InputState; use super::image::{copy_shm_argb, validate_shm_buffer_layout}; @@ -160,10 +162,12 @@ impl FrozenState { .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(); + let (source_geometry, target_output_id) = require_verified_capture_source( + self.active_geometry.clone(), + self.active_output_id, + "ext-image-copy capture", + ) + .map_err(anyhow::Error::msg)?; // Allocate the only fallible local resource before creating protocol // objects so an allocation failure cannot leave a live source/session. @@ -176,9 +180,8 @@ impl FrozenState { self.direct_capture = Some(DirectCaptureAttempt::ExtImageCopy { session: Box::new(ExtImageCopySession::new(source, session, pool)), context: DirectCaptureContext::new( - target_output_id, + CaptureLayoutContext::new(target_output_id, self.output_layout_generation), source_geometry, - self.output_layout_generation, ), }); debug!("Requested ext-image-copy capture constraints for active output"); @@ -400,12 +403,15 @@ impl FrozenState { .take_ext_capture() .context("ext-image-copy capture attempt missing after frame completion")?; (*capture).destroy(); - if !context.output_and_layout_match(self.active_output_id, self.output_layout_generation) { + if !context + .layout + .matches(self.active_output_id, self.output_layout_generation) + { return Ok(false); } self.set_pending_output_image_with_transform( image, - context.target_output_id, + context.layout.target_output_id(), context.source_geometry, output_transform, ); diff --git a/src/backend/wayland/frozen/portal.rs b/src/backend/wayland/frozen/portal.rs index 995221080..5025230a2 100644 --- a/src/backend/wayland/frozen/portal.rs +++ b/src/backend/wayland/frozen/portal.rs @@ -4,6 +4,7 @@ use log::warn; use std::time::{Duration, Instant}; use crate::backend::wayland::frozen::FrozenImage; +use crate::backend::wayland::frozen_geometry::require_verified_capture_source; use crate::backend::wayland::portal_capture::{ capture_via_portal_fullscreen_bytes, portal_output_matches, }; @@ -28,15 +29,12 @@ impl FrozenState { .runtime_wake .clone() .ok_or_else(|| anyhow::anyhow!("portal capture runtime wake is unavailable"))?; - let source_geometry = self.active_geometry.clone().ok_or_else(|| { - anyhow::anyhow!("active output geometry is unavailable for portal freeze capture") - })?; - source_geometry.verified_pixel_size().ok_or_else(|| { - anyhow::anyhow!("active output pixel size is unavailable for portal freeze capture") - })?; - let target_output_id = self.active_output_id.ok_or_else(|| { - anyhow::anyhow!("active output identity is unavailable for portal freeze capture") - })?; + let (source_geometry, target_output_id) = require_verified_capture_source( + self.active_geometry.clone(), + self.active_output_id, + "portal freeze capture", + ) + .map_err(anyhow::Error::msg)?; self.portal_in_progress = true; self.portal_target_output_id = Some(target_output_id); @@ -115,6 +113,7 @@ impl FrozenState { } else { warn!("Portal capture for inactive output discarded"); } + Self::push_stale_layout_toast(input_state); self.capture_done = true; } @@ -201,6 +200,7 @@ mod tests { pixel_size: Some((2, 1)), screenshot_origin: Some(origin), screenshot_size: None, + known_output_count: None, } } @@ -249,7 +249,7 @@ mod tests { frozen.portal_task = Some(PortalTask::spawn( &tokio::runtime::Handle::current(), wake.handle(), - async { Ok((None, 0, None, image(0))) }, + async { Ok((None, 0, Some(crop_geometry((0, 0))), image(0))) }, )); frozen.portal_in_progress = true; poll_until_finished(&mut frozen, &mut input).await?; @@ -374,6 +374,7 @@ mod tests { assert!(input.frozen_active()); assert!(!frozen.has_pending_image()); assert!(frozen.take_capture_done()); + assert!(input.ui_toast.is_some()); Ok(()) } @@ -396,6 +397,7 @@ mod tests { assert!(!frozen.has_pending_image()); assert!(frozen.take_capture_done()); + assert!(input.ui_toast.is_some()); Ok(()) } diff --git a/src/backend/wayland/frozen/state.rs b/src/backend/wayland/frozen/state.rs index 04ce675ac..986af6c61 100644 --- a/src/backend/wayland/frozen/state.rs +++ b/src/backend/wayland/frozen/state.rs @@ -4,11 +4,13 @@ use wayland_client::protocol::wl_output; use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::ZwlrScreencopyManagerV1; use crate::backend::wayland::RuntimeWakeHandle; +use crate::backend::wayland::capture::CaptureLayoutContext; use crate::backend::wayland::frozen::FrozenImage; use crate::backend::wayland::frozen_geometry::OutputGeometry; -use crate::backend::wayland::portal_capture::crop_argb; +use crate::backend::wayland::portal_capture::{crop_argb, layout_token_matches}; use crate::backend::wayland::portal_task::PortalTask; use crate::input::InputState; +use crate::input::state::{Toast, ToastPriority}; use super::PortalCaptureResult; use super::capture::CaptureSession; @@ -17,9 +19,9 @@ use super::ext_image_copy::{ExtImageCopyManagers, ExtImageCopySession}; struct PendingFrozenImage { image: FrozenImage, target_output_id: Option, + layout_generation: u64, source_geometry: Option, output_transform: Option, - needs_output_transform: bool, source: FrozenCaptureSource, } @@ -72,36 +74,24 @@ impl DirectCaptureAttempt { pub(super) const DIRECT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(3); pub(super) struct DirectCaptureContext { - pub(super) target_output_id: u32, - pub(super) source_geometry: Option, - layout_generation: u64, + pub(super) layout: CaptureLayoutContext, + pub(super) source_geometry: OutputGeometry, started_at: Instant, } impl DirectCaptureContext { - pub(super) fn new( - target_output_id: u32, - source_geometry: Option, - layout_generation: u64, - ) -> Self { - Self::new_at( - target_output_id, - source_geometry, - layout_generation, - Instant::now(), - ) + pub(super) fn new(layout: CaptureLayoutContext, source_geometry: OutputGeometry) -> Self { + Self::new_at(layout, source_geometry, Instant::now()) } fn new_at( - target_output_id: u32, - source_geometry: Option, - layout_generation: u64, + layout: CaptureLayoutContext, + source_geometry: OutputGeometry, started_at: Instant, ) -> Self { Self { - target_output_id, + layout, source_geometry, - layout_generation, started_at, } } @@ -112,15 +102,6 @@ impl DirectCaptureContext { .map(|deadline| deadline.saturating_duration_since(now)) .unwrap_or(Duration::ZERO) } - - pub(super) fn output_and_layout_match( - &self, - current_output_id: Option, - current_layout_generation: u64, - ) -> bool { - current_output_id == Some(self.target_output_id) - && current_layout_generation == self.layout_generation - } } /// End-to-end controller for frozen mode capture and image storage. @@ -145,6 +126,8 @@ pub struct FrozenState { pub(super) runtime_wake: Option, pub(super) preflight_pending: bool, pub(super) preflight_backend: Option, + preflight_output_id: Option, + preflight_layout_generation: Option, pub(super) capture_done: bool, pending_image: Option, } @@ -196,6 +179,8 @@ impl FrozenState { runtime_wake, preflight_pending: false, preflight_backend: None, + preflight_output_id: None, + preflight_layout_generation: None, capture_done: false, pending_image: None, } @@ -251,14 +236,14 @@ impl FrozenState { &mut self, image: FrozenImage, target_output_id: u32, - source_geometry: Option, + source_geometry: OutputGeometry, ) { self.pending_image = Some(PendingFrozenImage { image, target_output_id: Some(target_output_id), - source_geometry, + layout_generation: self.output_layout_generation, + source_geometry: Some(source_geometry), output_transform: None, - needs_output_transform: true, source: FrozenCaptureSource::ActiveOutput, }); } @@ -267,15 +252,15 @@ impl FrozenState { &mut self, image: FrozenImage, target_output_id: u32, - source_geometry: Option, + source_geometry: OutputGeometry, output_transform: Option, ) { self.pending_image = Some(PendingFrozenImage { image, target_output_id: Some(target_output_id), - source_geometry, + layout_generation: self.output_layout_generation, + source_geometry: Some(source_geometry), output_transform, - needs_output_transform: true, source: FrozenCaptureSource::ActiveOutput, }); } @@ -289,9 +274,9 @@ impl FrozenState { self.pending_image = Some(PendingFrozenImage { image, target_output_id, + layout_generation: self.output_layout_generation, source_geometry, output_transform: None, - needs_output_transform: false, source: FrozenCaptureSource::Desktop, }); } @@ -317,6 +302,52 @@ impl FrozenState { self.preflight_backend.take() } + pub(super) fn snapshot_preflight_layout(&mut self) { + self.preflight_output_id = self.active_output_id; + self.preflight_layout_generation = Some(self.output_layout_generation); + } + + pub(super) fn ensure_preflight_layout_current(&self) -> Result<(), String> { + let Some(generation) = self.preflight_layout_generation else { + return Ok(()); + }; + if layout_token_matches( + self.preflight_output_id, + generation, + self.active_output_id, + self.output_layout_generation, + ) { + Ok(()) + } else { + Err("Freeze failed after the display layout changed".to_string()) + } + } + + #[cfg(test)] + pub fn preflight_layout_is_current(&self) -> bool { + self.ensure_preflight_layout_current().is_ok() + } + + pub(super) fn push_stale_layout_toast(input_state: &mut InputState) { + input_state.push_toast( + ToastPriority::Critical, + "freeze", + Toast::error("Freeze failed after the display layout changed"), + ); + } + + pub(in crate::backend::wayland) fn finish_failed_fallback_capture( + &mut self, + input_state: &mut InputState, + ) { + let message = self + .ensure_preflight_layout_current() + .err() + .unwrap_or_else(|| "Freeze could not capture the screen.".to_string()); + input_state.push_toast(ToastPriority::Critical, "freeze", Toast::error(message)); + self.cancel(input_state); + } + pub fn take_capture_done(&mut self) -> bool { let done = self.capture_done; self.capture_done = false; @@ -346,102 +377,100 @@ impl FrozenState { Some(backend) } + #[cfg(test)] pub fn activate_pending_image( &mut self, phys_width: u32, phys_height: u32, input_state: &mut InputState, + ) -> Result { + self.activate_pending_image_with_live_outputs(phys_width, phys_height, input_state, None) + } + + pub fn activate_pending_image_with_live_outputs( + &mut self, + phys_width: u32, + phys_height: u32, + input_state: &mut InputState, + live_output_count: Option, ) -> Result { let Some(pending) = self.pending_image.take() else { return Ok(false); }; - if !crate::backend::wayland::portal_capture::portal_output_matches( + if !layout_token_matches( pending.target_output_id, + pending.layout_generation, self.active_output_id, + self.output_layout_generation, ) { - 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); + return self.reject_pending_image( + input_state, + "Freeze failed after the display layout changed", + ); } let mut image = pending.image; - if pending.needs_output_transform { - 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) - }); + if matches!(pending.source, FrozenCaptureSource::ActiveOutput) { + let Some(geometry) = pending.source_geometry.as_ref() else { + return self + .reject_pending_image(input_state, "Freeze capture geometry is unavailable"); + }; + let output_transform = pending.output_transform.unwrap_or(geometry.transform); image = match image.with_output_transform(output_transform) { Ok(image) => image, Err(error) => { - self.capture_done = true; - input_state.set_frozen_active(false); - input_state.needs_redraw = true; - return Err(format!("Freeze capture transform failed: {error}")); + return self.reject_pending_image( + input_state, + format!("Freeze capture transform failed: {error}"), + ); } }; - } - - if matches!(pending.source, FrozenCaptureSource::ActiveOutput) - && pending - .source_geometry - .as_ref() - .or(self.active_geometry.as_ref()) - .is_some_and(|geometry| { - !geometry.accepts_transformed_pixel_size(image.width, image.height) - }) - { - self.capture_done = true; - input_state.set_frozen_active(false); - input_state.needs_redraw = true; - return Err("Freeze capture dimensions do not match the active output".to_string()); + if !geometry.accepts_transformed_pixel_size(image.width, image.height) { + return self.reject_pending_image( + input_state, + "Freeze capture dimensions do not match the active output", + ); + } } if matches!(pending.source, FrozenCaptureSource::Desktop) { - let geometry = pending + let Some(geometry) = pending .source_geometry .as_ref() - .or(self.active_geometry.as_ref()); - let capture_dimensions = match geometry { - Some(geometry) => geometry.verified_pixel_size(), - None if image.width == phys_width && image.height == phys_height => { - Some((phys_width, phys_height)) - } - None => None, + .or(self.active_geometry.as_ref()) + .cloned() + .and_then(|geometry| geometry.with_revalidated_output_count(live_output_count)) + else { + return self.reject_pending_image( + input_state, + "Freeze failed after the output layout changed", + ); }; - let Some((capture_width, capture_height)) = capture_dimensions else { - self.capture_done = true; - input_state.set_frozen_active(false); - input_state.needs_redraw = true; - return Err("Freeze failed after the display changed size".to_string()); + let Some((capture_width, capture_height)) = geometry.verified_pixel_size() else { + return self.reject_pending_image( + input_state, + "Freeze failed after the display changed size", + ); }; - - if let Some(geometry) = geometry { - let Some(cropped) = - self.crop_pending_image(image, Some(geometry), capture_width, capture_height) - else { - self.capture_done = true; - input_state.set_frozen_active(false); - input_state.needs_redraw = true; - return Err("Freeze failed after the display changed size".to_string()); - }; - image = cropped; - } + let Some(cropped) = + self.crop_pending_image(image, &geometry, capture_width, capture_height) + else { + return self.reject_pending_image( + input_state, + "Freeze failed after the display changed size", + ); + }; + image = cropped; } if !OutputGeometry::dimensions_have_compatible_aspect( (image.width, image.height), (phys_width, phys_height), ) { - self.capture_done = true; - input_state.set_frozen_active(false); - input_state.needs_redraw = true; - return Err("Freeze capture aspect does not match the overlay surface".to_string()); + return self.reject_pending_image( + input_state, + "Freeze capture aspect does not match the overlay surface", + ); } self.image_target_dimensions = Some((phys_width, phys_height)); @@ -454,17 +483,27 @@ impl FrozenState { Ok(true) } + fn reject_pending_image( + &mut self, + input_state: &mut InputState, + error: impl Into, + ) -> Result { + self.capture_done = true; + input_state.set_frozen_active(false); + input_state.needs_redraw = true; + Err(error.into()) + } + fn crop_pending_image( &self, image: FrozenImage, - source_geometry: Option<&OutputGeometry>, + geometry: &OutputGeometry, target_width: u32, target_height: u32, ) -> Option { if target_width == 0 || target_height == 0 { return None; } - let geometry = source_geometry.or(self.active_geometry.as_ref())?; let (origin_x, origin_y) = geometry.portal_crop_origin(image.width, image.height)?; let (width, height, data) = crop_argb( &image.data, @@ -517,6 +556,8 @@ impl FrozenState { } self.preflight_pending = false; self.preflight_backend = None; + self.preflight_output_id = None; + self.preflight_layout_generation = None; self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { task.cancel(); @@ -581,6 +622,26 @@ mod tests { use super::*; use crate::input::state::test_support::make_test_input_state; + fn verified_output_geometry( + overlay_logical: (u32, u32), + scale: i32, + transform: wl_output::Transform, + pixel_size: (u32, u32), + ) -> OutputGeometry { + OutputGeometry::update_from( + Some((0, 0)), + Some(( + i32::try_from(overlay_logical.0).expect("test width"), + i32::try_from(overlay_logical.1).expect("test height"), + )), + overlay_logical, + scale, + transform, + Some(pixel_size), + ) + .expect("verified test output geometry") + } + #[test] fn capture_backend_priority_is_wlr_then_ext_then_portal() { assert_eq!( @@ -617,17 +678,27 @@ mod tests { #[test] fn direct_capture_context_tracks_its_deadline_and_output_identity() { let started_at = Instant::now(); - let capture = DirectCaptureContext::new_at(7, None, 3, started_at); + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((1, 1)), + (1, 1), + 1, + wl_output::Transform::Normal, + Some((1, 1)), + ) + .expect("geometry"); + let capture = + DirectCaptureContext::new_at(CaptureLayoutContext::new(7, 3), geometry, started_at); assert_eq!(capture.timeout(started_at), DIRECT_CAPTURE_TIMEOUT); assert_eq!( capture.timeout(started_at + DIRECT_CAPTURE_TIMEOUT), Duration::ZERO ); - assert!(capture.output_and_layout_match(Some(7), 3)); - assert!(!capture.output_and_layout_match(Some(8), 3)); - assert!(!capture.output_and_layout_match(None, 3)); - assert!(!capture.output_and_layout_match(Some(7), 4)); + assert!(capture.layout.matches(Some(7), 3)); + assert!(!capture.layout.matches(Some(8), 3)); + assert!(!capture.layout.matches(None, 3)); + assert!(!capture.layout.matches(Some(7), 4)); } #[test] @@ -643,7 +714,7 @@ mod tests { data: vec![0; 10 * 10 * 4], }, 7, - None, + verified_output_geometry((6, 6), 2, wl_output::Transform::Normal, (10, 10)), ); state @@ -684,7 +755,7 @@ mod tests { data: vec![0; 4 * 3 * 4], }, 7, - Some(geometry), + geometry, ); assert!( @@ -697,7 +768,41 @@ mod tests { } #[test] - fn active_output_capture_uses_protocol_transform_without_output_geometry() { + fn active_output_capture_rejects_unknown_pixel_size() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + state.set_active_output(None, Some(7)); + let geometry = OutputGeometry::update_from( + Some((0, 0)), + Some((3, 2)), + (3, 2), + 2, + wl_output::Transform::Normal, + None, + ) + .expect("geometry without mode pixels"); + state.set_pending_output_image( + FrozenImage { + width: 6, + height: 4, + stride: 24, + data: vec![0; 6 * 4 * 4], + }, + 7, + geometry, + ); + + assert!( + state + .activate_pending_image(6, 4, &mut input_state) + .is_err() + ); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + } + + #[test] + fn active_output_capture_prefers_protocol_transform_over_geometry() { let mut state = FrozenState::new(None); let mut input_state = make_test_input_state(); state.set_active_output(None, Some(7)); @@ -709,7 +814,7 @@ mod tests { data: vec![1, 0, 0, 255, 2, 0, 0, 255], }, 7, - None, + verified_output_geometry((1, 2), 1, wl_output::Transform::Normal, (1, 2)), Some(wl_output::Transform::_90), ); @@ -734,7 +839,7 @@ mod tests { data: vec![0; 12], }, 7, - None, + verified_output_geometry((2, 2), 1, wl_output::Transform::Normal, (2, 2)), Some(wl_output::Transform::_90), ); @@ -761,7 +866,7 @@ mod tests { data: vec![0; 320 * 180 * 4], }, 7, - None, + verified_output_geometry((320, 176), 1, wl_output::Transform::Normal, (320, 180)), ); let error = state @@ -816,6 +921,7 @@ mod tests { pixel_size: Some((logical_width * pixel_scale, logical_height * pixel_scale)), screenshot_origin, screenshot_size: None, + known_output_count: None, } } @@ -903,7 +1009,6 @@ mod tests { logical_y: 0, logical_width: 4, logical_height: 1, - scale: 1, physical_width: Some(4), physical_height: Some(1), crop_x: Some(6), @@ -933,12 +1038,12 @@ mod tests { } #[test] - fn desktop_capture_crops_at_buffer_origin_when_screenshot_origin_is_unknown() { + fn desktop_capture_fails_closed_when_screenshot_origin_is_unknown() { let mut state = FrozenState::new(None); let mut input_state = make_test_input_state(); let geometry = desktop_geometry(10, 20, 2, 1, 2, None); assert_eq!(geometry.physical_origin(), (20, 40)); - assert_eq!(geometry.portal_crop_origin(4, 2), Some((0, 0))); + assert_eq!(geometry.portal_crop_origin(4, 2), None); assert_eq!(geometry.portal_crop_origin(10, 2), None); state.set_pending_desktop_image( @@ -952,13 +1057,68 @@ mod tests { Some(geometry), ); + assert!( + state + .activate_pending_image(4, 2, &mut input_state) + .is_err() + ); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + } + + #[test] + fn desktop_capture_crops_at_buffer_origin_for_a_proven_single_output() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + let geometry = desktop_geometry(10, 20, 2, 1, 2, None).with_known_output_count(Some(1)); + assert_eq!(geometry.portal_crop_origin(4, 2), Some((0, 0))); + + state.set_pending_desktop_image( + FrozenImage { + width: 4, + height: 2, + stride: 16, + data: vec![7; 4 * 2 * 4], + }, + None, + Some(geometry), + ); + state - .activate_pending_image(4, 2, &mut input_state) - .expect("a single-output desktop shot should crop from the buffer origin"); + .activate_pending_image_with_live_outputs(4, 2, &mut input_state, Some(1)) + .expect( + "a single-output desktop shot without zxdg layout still crops at the buffer origin", + ); assert!(state.image().is_some()); assert!(input_state.frozen_active()); } + #[test] + fn desktop_capture_rejects_a_stale_single_output_snapshot_when_live_count_grows() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + let geometry = desktop_geometry(10, 20, 2, 1, 2, None).with_known_output_count(Some(1)); + + state.set_pending_desktop_image( + FrozenImage { + width: 4, + height: 2, + stride: 16, + data: vec![7; 4 * 2 * 4], + }, + None, + Some(geometry), + ); + + assert!( + state + .activate_pending_image_with_live_outputs(4, 2, &mut input_state, Some(2)) + .is_err() + ); + assert!(state.image().is_none()); + assert!(!input_state.frozen_active()); + } + #[test] fn desktop_capture_fails_when_screenshot_origin_is_unknown_and_image_is_larger() { let mut state = FrozenState::new(None); @@ -986,7 +1146,7 @@ mod tests { } #[test] - fn pending_capture_is_discarded_if_output_changes_before_activation() { + fn pending_capture_is_rejected_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)); @@ -998,21 +1158,103 @@ mod tests { data: vec![0; 4], }, 7, - None, + verified_output_geometry((1, 1), 1, wl_output::Transform::Normal, (1, 1)), ); state.set_active_output(None, Some(8)); - let activated = state + let error = state .activate_pending_image(1, 1, &mut input_state) - .expect("the stale-output path is a handled non-error outcome"); + .expect_err("stale output identity must fail closed"); - assert!(!activated); + assert!(error.contains("display layout changed")); assert!(state.image().is_none()); assert!(!state.has_pending_image()); assert!(state.take_capture_done()); assert!(!input_state.frozen_active()); } + #[test] + fn pending_capture_is_rejected_if_layout_changes_before_activation() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + let first = verified_output_geometry((1, 1), 1, wl_output::Transform::Normal, (1, 1)); + let second = verified_output_geometry((2, 2), 1, wl_output::Transform::Normal, (2, 2)); + state.set_active_output(None, Some(7)); + state.set_active_geometry(Some(first.clone())); + state.set_pending_output_image( + FrozenImage { + width: 1, + height: 1, + stride: 4, + data: vec![0; 4], + }, + 7, + first, + ); + + state.set_active_geometry(Some(second)); + let error = state + .activate_pending_image(1, 1, &mut input_state) + .expect_err("stale layout token must fail closed after delayed activation"); + + assert!(error.contains("display layout changed")); + assert!(state.image().is_none()); + assert!(!state.has_pending_image()); + assert!(!input_state.frozen_active()); + } + + #[test] + fn preflight_layout_snapshot_goes_stale_when_geometry_changes() { + let mut state = FrozenState::new(None); + state.snapshot_preflight_layout(); + assert!(state.preflight_layout_is_current()); + state.set_active_geometry(Some(verified_output_geometry( + (1, 1), + 1, + wl_output::Transform::Normal, + (1, 1), + ))); + assert!(!state.preflight_layout_is_current()); + } + + #[test] + fn exhausted_fallback_toasts_layout_change_when_preflight_is_stale() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + state.snapshot_preflight_layout(); + state.set_active_geometry(Some(verified_output_geometry( + (1, 1), + 1, + wl_output::Transform::Normal, + (1, 1), + ))); + + state.finish_failed_fallback_capture(&mut input_state); + + let toast = input_state + .ui_toast + .as_ref() + .expect("visible stale rejection"); + assert!(toast.message.contains("display layout changed")); + assert!(state.take_capture_done()); + } + + #[test] + fn exhausted_fallback_toasts_a_generic_failure_when_layout_is_current() { + let mut state = FrozenState::new(None); + let mut input_state = make_test_input_state(); + state.snapshot_preflight_layout(); + + state.finish_failed_fallback_capture(&mut input_state); + + let toast = input_state + .ui_toast + .as_ref() + .expect("visible capture failure"); + assert_eq!(toast.message, "Freeze could not capture the screen."); + assert!(state.take_capture_done()); + } + #[test] fn cancel_clears_an_in_flight_portal_capture() { let mut state = FrozenState::new(None); diff --git a/src/backend/wayland/frozen_geometry.rs b/src/backend/wayland/frozen_geometry.rs index 920872454..e9244dd08 100644 --- a/src/backend/wayland/frozen_geometry.rs +++ b/src/backend/wayland/frozen_geometry.rs @@ -24,6 +24,9 @@ pub struct OutputGeometry { pub screenshot_origin: Option<(u32, u32)>, /// Expected full-desktop screenshot dimensions for the captured layout. pub(super) screenshot_size: Option<(u32, u32)>, + /// Number of live `wl_output` objects when this snapshot was taken. + /// `None` means topology was not recorded (tests / incomplete refresh). + pub(super) known_output_count: Option, } impl OutputGeometry { @@ -64,6 +67,7 @@ impl OutputGeometry { pixel_size, screenshot_origin: None, screenshot_size: None, + known_output_count: None, }) } } @@ -91,9 +95,27 @@ mod tests { assert_eq!(geo.logical_height, 1080); assert_eq!(geo.transform, wl_output::Transform::_270); assert_eq!(geo.verified_pixel_size(), Some((3840, 2160))); - assert_eq!(geo.buffer_size(), Some((1600, 1200))); + assert_eq!(geo.buffer_size(), (1600, 1200)); assert_eq!(geo.physical_origin(), (20, 40)); - assert_eq!(geo.portal_crop_origin(3840, 2160), Some((0, 0))); + assert_eq!(geo.portal_crop_origin(3840, 2160), None); + assert_eq!( + geo.clone() + .with_known_output_count(Some(1)) + .portal_crop_origin(3840, 2160), + Some((0, 0)) + ); + assert_eq!( + geo.clone() + .with_known_output_count(Some(2)) + .portal_crop_origin(3840, 2160), + None + ); + assert_eq!( + geo.clone() + .with_screenshot_origin(Some((0, 0))) + .portal_crop_origin(3840, 2160), + Some((0, 0)) + ); assert_eq!(geo.portal_crop_origin(8000, 2160), None); assert_eq!( geo.with_screenshot_origin(Some((6, 0))) @@ -162,7 +184,6 @@ mod tests { logical_y: 0, logical_width: 3, logical_height: 2, - scale: 2, physical_width: Some(5), physical_height: Some(3), crop_x: Some(0), @@ -190,7 +211,6 @@ mod tests { logical_y: 0, logical_width: 3, logical_height: 2, - scale: 2, physical_width: Some(5), physical_height: Some(3), crop_x: Some(7), @@ -202,6 +222,125 @@ mod tests { assert_eq!(geo.verified_pixel_size(), Some((5, 3))); assert_eq!(geo.portal_crop_origin(12, 3), None); + assert_eq!(geo.portal_crop_origin(5, 3), None); + assert_eq!( + geo.with_known_output_count(Some(1)) + .portal_crop_origin(5, 3), + Some((0, 0)) + ); + } + + #[test] + fn portal_crop_origin_requires_layout_origin_even_when_the_image_matches_this_output() { + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((1920, 1080)), + (1920, 1080), + 1, + wl_output::Transform::Normal, + Some((1920, 1080)), + ) + .expect("matching output geometry"); + + assert_eq!(geo.portal_crop_origin(1920, 1080), None); + } + + #[test] + fn portal_crop_origin_infers_buffer_origin_only_for_a_proven_single_output() { + let geo = OutputGeometry::update_from( + Some((10, 20)), + Some((1920, 1080)), + (1920, 1080), + 1, + wl_output::Transform::Normal, + Some((1920, 1080)), + ) + .expect("single output without zxdg layout"); + + assert_eq!( + geo.clone() + .with_known_output_count(Some(1)) + .portal_crop_origin(1920, 1080), + Some((0, 0)) + ); + assert_eq!( + geo.clone() + .with_known_output_count(Some(1)) + .portal_crop_origin(3840, 1080), + None + ); + assert_eq!( + geo.with_known_output_count(Some(2)) + .portal_crop_origin(1920, 1080), + None + ); + } + + #[test] + fn revalidated_output_count_rejects_stale_single_output_snapshots() { + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((1920, 1080)), + (1920, 1080), + 1, + wl_output::Transform::Normal, + Some((1920, 1080)), + ) + .expect("geometry") + .with_known_output_count(Some(1)); + + assert!(geo.clone().with_revalidated_output_count(Some(2)).is_none()); + let live = geo + .clone() + .with_revalidated_output_count(Some(1)) + .expect("matching live topology"); + assert_eq!(live.portal_crop_origin(1920, 1080), Some((0, 0))); + assert_eq!( + geo.with_revalidated_output_count(None) + .expect("tests without a live count keep the snapshot") + .portal_crop_origin(1920, 1080), + Some((0, 0)) + ); + } + + #[test] + fn require_verified_capture_source_fails_closed_without_geometry_pixels_or_identity() { + assert_eq!( + require_verified_capture_source(None, Some(1), "test capture").unwrap_err(), + "active output geometry is unavailable for test capture" + ); + + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((800, 600)), + (800, 600), + 1, + wl_output::Transform::Normal, + None, + ) + .expect("geometry without mode pixels"); + assert_eq!( + require_verified_capture_source(Some(geo), Some(1), "test capture").unwrap_err(), + "active output pixel size is unavailable for test capture" + ); + + let geo = OutputGeometry::update_from( + Some((0, 0)), + Some((800, 600)), + (800, 600), + 1, + wl_output::Transform::Normal, + Some((800, 600)), + ) + .expect("geometry with mode pixels"); + assert_eq!( + require_verified_capture_source(Some(geo.clone()), None, "test capture").unwrap_err(), + "active output identity is unavailable for test capture" + ); + let (verified, output_id) = + require_verified_capture_source(Some(geo), Some(7), "test capture").expect("verified"); + assert_eq!(verified.verified_pixel_size(), Some((800, 600))); + assert_eq!(output_id, 7); } #[test] @@ -262,22 +401,24 @@ impl OutputGeometry { } /// Returns the integer-scale buffer dimensions used by the overlay surface. - pub fn buffer_size(&self) -> Option<(u32, u32)> { - Some(self.overlay_buffer_size) + pub fn buffer_size(&self) -> (u32, u32) { + self.overlay_buffer_size } - /// Validate post-transform pixels when the compositor's physical mode is known. + /// Validate post-transform pixels against the compositor's physical mode. + /// + /// Unknown mode size is a mismatch: guessing from the overlay buffer would + /// accept another output with the same aspect. pub fn accepts_transformed_pixel_size(&self, width: u32, height: u32) -> bool { - self.pixel_size - .is_none_or(|pixel_size| pixel_size == (width, height)) + self.pixel_size == Some((width, height)) } /// Returns physical pixel origin of the logical position on this output. /// /// Portal desktop screenshots must use [`Self::portal_crop_origin`] instead: /// mixed-scale layouts are not `logical * this output's scale`, and an - /// unknown origin must not be treated as `(0, 0)` unless the capture is - /// already this output's physical size. + /// unknown origin must not be treated as `(0, 0)` unless topology proves + /// there is a single output whose pixels match the capture. #[allow(dead_code)] // used by geometry tests; portal crop uses screenshot_origin pub fn physical_origin(&self) -> (i32, i32) { ( @@ -306,12 +447,36 @@ impl OutputGeometry { self } + pub fn with_known_output_count(mut self, count: Option) -> Self { + self.known_output_count = count; + self + } + + /// Apply the live `wl_output` count at capture accept time. + /// + /// SCTK can insert a new output into `OutputState` before `new_output` + /// runs, so a snapshot of `Some(1)` must not survive that window. + pub fn with_revalidated_output_count(self, live_output_count: Option) -> Option { + if self.output_count_conflicts_with_live(live_output_count) { + return None; + } + let known_output_count = self.known_output_count; + Some(self.with_known_output_count(live_output_count.or(known_output_count))) + } + + pub fn output_count_conflicts_with_live(&self, live_output_count: Option) -> bool { + matches!( + (self.known_output_count, live_output_count), + (Some(known), Some(live)) if known != live + ) + } + /// Crop origin inside a portal/desktop screenshot of `image_width` × /// `image_height`. /// - /// Unknown origin is `(0, 0)` only when those dimensions are this output's - /// physical size (a single-output capture). Otherwise the origin stays - /// unknown so a multi-output shot is not cropped from the first monitor. + /// Prefer the walked layout origin. If optional zxdg_output metadata is + /// missing, `(0, 0)` is inferred only when topology proves there is a + /// single output and the image is that output's physical size. pub fn portal_crop_origin(&self, image_width: u32, image_height: u32) -> Option<(u32, u32)> { if self .screenshot_size @@ -323,6 +488,27 @@ impl OutputGeometry { return Some(origin); } let (phys_width, phys_height) = self.verified_pixel_size()?; - (image_width == phys_width && image_height == phys_height).then_some((0, 0)) + (self.known_output_count == Some(1) + && image_width == phys_width + && image_height == phys_height) + .then_some((0, 0)) + } +} + +/// Require compositor-reported output pixels and a stable output identity. +pub fn require_verified_capture_source( + geometry: Option, + output_id: Option, + what: &str, +) -> Result<(OutputGeometry, u32), String> { + let geometry = + geometry.ok_or_else(|| format!("active output geometry is unavailable for {what}"))?; + if geometry.verified_pixel_size().is_none() { + return Err(format!( + "active output pixel size is unavailable for {what}" + )); } + let output_id = + output_id.ok_or_else(|| format!("active output identity is unavailable for {what}"))?; + Ok((geometry, output_id)) } diff --git a/src/backend/wayland/handlers/output.rs b/src/backend/wayland/handlers/output.rs index 4158eeeb8..df5ada9e4 100644 --- a/src/backend/wayland/handlers/output.rs +++ b/src/backend/wayland/handlers/output.rs @@ -18,7 +18,7 @@ impl OutputHandler for WaylandState { ) { debug!("New output detected"); self.refresh_active_output_label(); - self.refresh_freeze_zoom_screenshot_origin(); + self.refresh_freeze_zoom_geometry(); } fn update_output( @@ -31,14 +31,10 @@ impl OutputHandler for WaylandState { if self.surface.current_output().as_ref() == Some(&output) { self.refresh_active_output_label(); } - if self.surface.current_output().as_ref() == Some(&output) { - self.refresh_freeze_zoom_geometry(); - return; - } // Screenshot origin walks every output, so a non-active monitor that // is added, moved, scaled, or given logical geometry still has to // refresh the active crop. - self.refresh_freeze_zoom_screenshot_origin(); + self.refresh_freeze_zoom_geometry(); } fn output_destroyed( @@ -56,6 +52,6 @@ impl OutputHandler for WaylandState { // SCTK 0.20 calls this before removing the output from OutputState, so // a walk of current outputs would still include it. Exclude it here; // there is no later callback after the removal. - self.refresh_freeze_zoom_screenshot_origin_excluding(Some(&output)); + self.refresh_freeze_zoom_geometry_excluding(Some(&output)); } } diff --git a/src/backend/wayland/portal_capture.rs b/src/backend/wayland/portal_capture.rs index 8e9ca74a5..9bdb207cb 100644 --- a/src/backend/wayland/portal_capture.rs +++ b/src/backend/wayland/portal_capture.rs @@ -44,6 +44,16 @@ pub(crate) const fn portal_output_matches(target: Option, current: Option, + captured_generation: u64, + active_output: Option, + active_generation: u64, +) -> bool { + portal_output_matches(captured_output, active_output) + && captured_generation == active_generation +} + pub(crate) fn crop_argb( data: &[u8], width: u32, @@ -86,7 +96,7 @@ pub(crate) fn crop_argb( #[cfg(test)] mod tests { - use super::{crop_argb, portal_output_matches}; + use super::{crop_argb, layout_token_matches, portal_output_matches}; #[test] fn crop_argb_respects_bounds() { @@ -121,4 +131,13 @@ mod tests { assert!(!portal_output_matches(None, Some(1))); assert!(!portal_output_matches(Some(1), None)); } + + #[test] + fn layout_token_matches_requires_output_and_generation() { + assert!(layout_token_matches(Some(7), 3, Some(7), 3)); + assert!(!layout_token_matches(Some(7), 3, Some(8), 3)); + assert!(!layout_token_matches(Some(7), 3, Some(7), 4)); + assert!(!layout_token_matches(None, 3, Some(7), 3)); + assert!(layout_token_matches(None, 3, None, 3)); + } } diff --git a/src/backend/wayland/state/capture.rs b/src/backend/wayland/state/capture.rs index dc6bcad9d..b76fc25b7 100644 --- a/src/backend/wayland/state/capture.rs +++ b/src/backend/wayland/state/capture.rs @@ -29,7 +29,8 @@ impl WaylandState { .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); + self.frozen + .finish_failed_fallback_capture(&mut self.input_state); } } diff --git a/src/backend/wayland/state/capture/backdrop.rs b/src/backend/wayland/state/capture/backdrop.rs index 78185c2bf..bad3c263a 100644 --- a/src/backend/wayland/state/capture/backdrop.rs +++ b/src/backend/wayland/state/capture/backdrop.rs @@ -78,7 +78,26 @@ impl WaylandState { outputs.push(desktop_backdrop_output_geometry_from_info(&info)?); } - DesktopBackdropGeometry::from_outputs(active, &outputs, active_info.scale_factor.max(1)) + DesktopBackdropGeometry::from_outputs(active, &outputs) + } + + /// Count advertised `wl_output` objects, including ones whose `new_output` + /// callback has not run yet. SCTK can insert the proxy before metadata + /// completes; Freeze/Zoom must not treat that window as a proven single + /// output. + pub(in crate::backend::wayland) fn live_output_count(&self) -> Option { + self.known_output_count_excluding(None) + } + + fn known_output_count_excluding(&self, exclude: Option<&wl_output::WlOutput>) -> Option { + let mut count = 0u32; + for candidate in self.output_state.outputs() { + if exclude.is_some_and(|destroyed| destroyed == &candidate) { + continue; + } + count = count.checked_add(1)?; + } + Some(count) } pub(in crate::backend::wayland) fn set_freeze_zoom_geometry( @@ -94,27 +113,23 @@ impl WaylandState { exclude: Option<&wl_output::WlOutput>, ) { let backdrop_geometry = self.desktop_backdrop_geometry_excluding(exclude); - let geometry = geometry.map(|geo| geo.with_desktop_backdrop_geometry(backdrop_geometry)); + let known_output_count = self.known_output_count_excluding(exclude); + let geometry = geometry.map(|geo| { + geo.with_desktop_backdrop_geometry(backdrop_geometry) + .with_known_output_count(known_output_count) + }); self.frozen.set_active_geometry(geometry.clone()); self.zoom.set_active_geometry(geometry); } - pub(in crate::backend::wayland) fn refresh_freeze_zoom_screenshot_origin(&mut self) { + pub(in crate::backend::wayland) fn refresh_freeze_zoom_geometry(&mut self) { self.refresh_freeze_zoom_geometry_excluding(None); } - pub(in crate::backend::wayland) fn refresh_freeze_zoom_screenshot_origin_excluding( + pub(in crate::backend::wayland) fn refresh_freeze_zoom_geometry_excluding( &mut self, exclude: Option<&wl_output::WlOutput>, ) { - self.refresh_freeze_zoom_geometry_excluding(exclude); - } - - pub(in crate::backend::wayland) fn refresh_freeze_zoom_geometry(&mut self) { - self.refresh_freeze_zoom_geometry_excluding(None); - } - - fn refresh_freeze_zoom_geometry_excluding(&mut self, exclude: Option<&wl_output::WlOutput>) { let Some(output) = self.surface.current_output() else { self.set_freeze_zoom_geometry_excluding(None, exclude); self.frozen.set_active_output(None, None); diff --git a/src/backend/wayland/state/capture/barrier.rs b/src/backend/wayland/state/capture/barrier.rs index 9b10dd7a7..dd9cd133b 100644 --- a/src/backend/wayland/state/capture/barrier.rs +++ b/src/backend/wayland/state/capture/barrier.rs @@ -329,6 +329,11 @@ impl WaylandState { .begin_preflight_capture(backend, &self.shm, qh, &self.tokio_handle) { log::warn!("Frozen preflight capture failed: {err}"); + self.input_state.push_toast( + ToastPriority::Critical, + "freeze", + Toast::error(err.to_string()), + ); self.frozen.cancel(&mut self.input_state); } } @@ -345,6 +350,11 @@ impl WaylandState { &self.tokio_handle, ) { log::warn!("Zoom preflight capture failed: {err}"); + self.input_state.push_toast( + ToastPriority::Critical, + "zoom", + Toast::error(err.to_string()), + ); self.zoom.cancel(&mut self.input_state, false); } } diff --git a/src/backend/wayland/state/core/accessors.rs b/src/backend/wayland/state/core/accessors.rs index edf915a45..ff82d7f03 100644 --- a/src/backend/wayland/state/core/accessors.rs +++ b/src/backend/wayland/state/core/accessors.rs @@ -357,10 +357,13 @@ impl WaylandState { ) { let was_xdg_frozen_fullscreen = self.xdg_frozen_fullscreen_requested(); let (phys_width, phys_height) = self.surface.physical_dimensions(); - match self - .frozen - .activate_pending_image(phys_width, phys_height, &mut self.input_state) - { + let live_output_count = self.live_output_count(); + match self.frozen.activate_pending_image_with_live_outputs( + phys_width, + phys_height, + &mut self.input_state, + live_output_count, + ) { Ok(true) => { if was_xdg_frozen_fullscreen { self.data.xdg_frozen_fullscreen_state = diff --git a/src/backend/wayland/zoom/capture.rs b/src/backend/wayland/zoom/capture.rs index d555af4a3..0464d3ae5 100644 --- a/src/backend/wayland/zoom/capture.rs +++ b/src/backend/wayland/zoom/capture.rs @@ -4,17 +4,15 @@ use smithay_client_toolkit::shm::{ Shm, slot::{Buffer, SlotPool}, }; -use wayland_client::{ - Dispatch, QueueHandle, WEnum, - protocol::{wl_output, wl_shm}, -}; +use wayland_client::{Dispatch, QueueHandle, WEnum, protocol::wl_shm}; 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::capture::CaptureLayoutContext; use crate::backend::wayland::frozen::{FrozenImage, copy_shm_argb, validate_shm_buffer_layout}; -use crate::backend::wayland::frozen_geometry::OutputGeometry; +use crate::backend::wayland::frozen_geometry::{OutputGeometry, require_verified_capture_source}; use crate::input::InputState; use super::state::ZoomState; @@ -74,53 +72,28 @@ impl CaptureSession { } struct CaptureContext { - target_output_id: u32, - source_geometry: Option, - layout_generation: u64, + layout: CaptureLayoutContext, + source_geometry: OutputGeometry, } impl CaptureContext { - fn new( - target_output_id: u32, - source_geometry: Option, - layout_generation: u64, - ) -> Self { + fn new(target_output_id: u32, source_geometry: OutputGeometry, layout_generation: u64) -> Self { Self { - target_output_id, + layout: CaptureLayoutContext::new(target_output_id, layout_generation), source_geometry, - layout_generation, } } - - fn output_and_layout_match( - &self, - active_output_id: Option, - layout_generation: u64, - ) -> bool { - active_output_id == Some(self.target_output_id) - && layout_generation == self.layout_generation - } } fn finalize_capture_image(image: FrozenImage, context: &CaptureContext) -> Result { - let output_transform = context - .source_geometry - .as_ref() - .map(|geometry| geometry.transform) - .unwrap_or(wl_output::Transform::Normal); - let image = image.with_output_transform(output_transform)?; - if context + let image = image.with_output_transform(context.source_geometry.transform)?; + if !context .source_geometry - .as_ref() - .is_some_and(|geometry| !geometry.accepts_transformed_pixel_size(image.width, image.height)) + .accepts_transformed_pixel_size(image.width, image.height) { anyhow::bail!("Zoom capture dimensions do not match the active output"); } - let buffer_size = context - .source_geometry - .as_ref() - .and_then(OutputGeometry::buffer_size) - .context("Zoom capture has no overlay buffer geometry")?; + let buffer_size = context.source_geometry.buffer_size(); if !OutputGeometry::dimensions_have_compatible_aspect((image.width, image.height), buffer_size) { anyhow::bail!("Zoom capture aspect does not match the overlay surface"); @@ -142,6 +115,7 @@ impl ZoomState { self.capture_done = false; self.preflight_use_fallback = use_fallback || self.manager.is_none(); + self.snapshot_preflight_layout(); self.preflight_pending = true; Ok(()) } @@ -157,6 +131,8 @@ impl ZoomState { State: Dispatch + Dispatch + 'static, { + self.ensure_preflight_layout_current() + .map_err(anyhow::Error::msg)?; if use_fallback || self.manager.is_none() { info!("capture.preflight component=zoom phase=portal-start suppression_ready=true"); self.capture_via_portal(tokio_handle) @@ -183,16 +159,15 @@ impl ZoomState { anyhow::bail!("No active output available for zoom capture"); } }; - let target_output_id = self - .active_output_id - .context("Active output has no stable identity for zoom capture")?; - let source_geometry = self - .active_geometry - .clone() - .context("Active output geometry is unavailable for zoom capture")?; + let (source_geometry, target_output_id) = require_verified_capture_source( + self.active_geometry.clone(), + self.active_output_id, + "zoom capture", + ) + .map_err(anyhow::Error::msg)?; let context = CaptureContext::new( target_output_id, - Some(source_geometry), + source_geometry, self.output_layout_generation, ); @@ -251,7 +226,7 @@ impl ZoomState { Ok(true) => {} Ok(false) => { warn!("Zoom capture discarded after the output layout changed"); - self.cancel(input_state, false); + self.finish_stale_direct_capture(input_state); return; } Err(err) => { @@ -357,7 +332,8 @@ impl ZoomState { let image = result?; if !capture .context - .output_and_layout_match(self.active_output_id, self.output_layout_generation) + .layout + .matches(self.active_output_id, self.output_layout_generation) { return Ok(false); } @@ -371,15 +347,7 @@ impl ZoomState { #[cfg(test)] mod tests { use super::*; - - #[test] - fn capture_context_rejects_output_or_layout_changes() { - let context = CaptureContext::new(7, None, 3); - - assert!(context.output_and_layout_match(Some(7), 3)); - assert!(!context.output_and_layout_match(Some(8), 3)); - assert!(!context.output_and_layout_match(Some(7), 4)); - } + use wayland_client::protocol::wl_output; #[test] fn finalized_capture_rejects_known_pixel_size_mismatch() { @@ -392,7 +360,7 @@ mod tests { Some((5, 3)), ) .expect("known output geometry"); - let context = CaptureContext::new(7, Some(geometry), 3); + let context = CaptureContext::new(7, geometry, 3); let image = crate::backend::wayland::frozen::FrozenImage { width: 4, height: 3, @@ -414,7 +382,7 @@ mod tests { Some((2, 3)), ) .expect("rotated output geometry"); - let context = CaptureContext::new(7, Some(geometry), 3); + let context = CaptureContext::new(7, geometry, 3); let image = crate::backend::wayland::frozen::FrozenImage { width: 3, height: 2, @@ -437,7 +405,7 @@ mod tests { Some((3200, 1800)), ) .expect("known output geometry"); - let context = CaptureContext::new(7, Some(geometry), 3); + let context = CaptureContext::new(7, geometry, 3); let image = FrozenImage { width: 3200, height: 1800, @@ -465,4 +433,24 @@ mod tests { assert!(!state.portal_in_progress); assert_eq!(state.take_preflight_pending(), Some(true)); } + + #[test] + fn preflight_layout_snapshot_goes_stale_when_geometry_changes() { + let wake = crate::backend::wayland::RuntimeWakeSource::new().expect("runtime wake"); + let mut state = ZoomState::new_with_runtime_wake(None, wake.handle()); + state.snapshot_preflight_layout(); + assert!(state.preflight_layout_is_current()); + state.set_active_geometry(Some( + OutputGeometry::update_from( + Some((0, 0)), + Some((1, 1)), + (1, 1), + 1, + wl_output::Transform::Normal, + Some((1, 1)), + ) + .expect("geometry"), + )); + assert!(!state.preflight_layout_is_current()); + } } diff --git a/src/backend/wayland/zoom/portal.rs b/src/backend/wayland/zoom/portal.rs index 90081ff37..a483ded29 100644 --- a/src/backend/wayland/zoom/portal.rs +++ b/src/backend/wayland/zoom/portal.rs @@ -3,7 +3,7 @@ use log::warn; use std::time::{Duration, Instant}; use crate::backend::wayland::frozen::FrozenImage; -use crate::backend::wayland::frozen_geometry::OutputGeometry; +use crate::backend::wayland::frozen_geometry::{OutputGeometry, require_verified_capture_source}; use crate::backend::wayland::portal_capture::{ capture_via_portal_fullscreen_bytes, crop_argb, portal_output_matches, }; @@ -28,15 +28,12 @@ impl ZoomState { .runtime_wake .clone() .ok_or_else(|| anyhow::anyhow!("portal capture runtime wake is unavailable"))?; - let geo = self.active_geometry.clone().ok_or_else(|| { - anyhow::anyhow!("active output geometry is unavailable for portal zoom capture") - })?; - geo.verified_pixel_size().ok_or_else(|| { - anyhow::anyhow!("active output pixel size is unavailable for portal zoom capture") - })?; - let target_output_id = self.active_output_id.ok_or_else(|| { - anyhow::anyhow!("active output identity is unavailable for portal zoom capture") - })?; + let (geo, target_output_id) = require_verified_capture_source( + self.active_geometry.clone(), + self.active_output_id, + "portal zoom capture", + ) + .map_err(anyhow::Error::msg)?; self.portal_in_progress = true; self.portal_target_output_id = Some(target_output_id); @@ -63,7 +60,12 @@ impl ZoomState { Ok(()) } - pub fn poll_portal_capture(&mut self, input_state: &mut InputState, now: Instant) { + pub fn poll_portal_capture( + &mut self, + input_state: &mut InputState, + now: Instant, + live_output_count: Option, + ) { if !self.portal_in_progress { return; } @@ -96,6 +98,23 @@ impl ZoomState { let layout_matches = layout_generation == self.output_layout_generation; if output_matches && layout_matches { + // Crop used the spawn-time geometry moved into the task. + // A processed topology change updates `known_output_count`, + // so `OutputGeometry`'s equality bumps + // `output_layout_generation` and `layout_matches` drops the + // result. This live-count check covers the SCTK window + // where a new `wl_output` is already in `OutputState` + // before `new_output` refreshes `active_geometry`. Freeze + // instead revalidates the pending snapshot, because it + // crops on the Wayland thread at activate. + if self.active_geometry.as_ref().is_some_and(|geometry| { + geometry.output_count_conflicts_with_live(live_output_count) + }) { + warn!("Portal zoom capture discarded after output topology changed"); + Self::push_stale_layout_toast(input_state); + self.finish_failed_portal_task(input_state); + return; + } self.set_image(image); } else { if !layout_matches { @@ -103,6 +122,7 @@ impl ZoomState { } else { warn!("Portal zoom capture for inactive output discarded"); } + Self::push_stale_layout_toast(input_state); self.finish_failed_portal_task(input_state); return; } @@ -183,9 +203,7 @@ fn crop_portal_image( let (phys_w, phys_h) = geometry.verified_pixel_size().ok_or_else(|| { CaptureError::ImageError("Zoom capture output dimensions are invalid".to_string()) })?; - let buffer_size = geometry.buffer_size().ok_or_else(|| { - CaptureError::ImageError("Zoom capture overlay dimensions are invalid".to_string()) - })?; + let buffer_size = geometry.buffer_size(); if !OutputGeometry::dimensions_have_compatible_aspect((phys_w, phys_h), buffer_size) { return Err(CaptureError::ImageError( "Zoom capture aspect does not match the overlay surface".to_string(), @@ -247,6 +265,7 @@ mod tests { pixel_size: Some((2, 1)), screenshot_origin: Some(origin), screenshot_size: None, + known_output_count: None, } } @@ -288,7 +307,6 @@ mod tests { logical_y: 0, logical_width: 3, logical_height: 2, - scale: 2, physical_width: Some(5), physical_height: Some(3), crop_x: Some(0), @@ -312,7 +330,6 @@ mod tests { logical_y: 0, logical_width: 2, logical_height: 1, - scale: 1, physical_width: Some(2), physical_height: Some(1), crop_x: Some(0), @@ -326,8 +343,16 @@ mod tests { } async fn poll_until_finished(zoom: &mut ZoomState, input: &mut InputState) { + poll_until_finished_with_live_outputs(zoom, input, None).await; + } + + async fn poll_until_finished_with_live_outputs( + zoom: &mut ZoomState, + input: &mut InputState, + live_output_count: Option, + ) { for _ in 0..100 { - zoom.poll_portal_capture(input, Instant::now()); + zoom.poll_portal_capture(input, Instant::now(), live_output_count); if !zoom.portal_in_progress { return; } @@ -405,7 +430,7 @@ mod tests { }); zoom.portal_in_progress = true; - zoom.poll_portal_capture(&mut input, now); + zoom.poll_portal_capture(&mut input, now, None); assert!(!zoom.is_in_progress()); assert!(!zoom.active); @@ -438,6 +463,7 @@ mod tests { assert_eq!(zoom.image_generation(), generation); assert_eq!(zoom.image().unwrap().data, vec![4; 8]); assert!(zoom.take_capture_done()); + assert!(input.ui_toast.is_some()); } #[tokio::test] @@ -465,6 +491,7 @@ mod tests { assert_eq!(zoom.image_generation(), generation); assert_eq!(zoom.image().unwrap().data, vec![4; 8]); assert!(zoom.take_capture_done()); + assert!(input.ui_toast.is_some()); } #[tokio::test] @@ -492,6 +519,33 @@ mod tests { assert!(zoom.take_capture_done()); } + #[tokio::test] + async fn stale_live_output_count_discards_a_single_output_portal_image() { + let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); + let mut zoom = ZoomState::new_with_runtime_wake(None, wake.handle()); + let mut input = make_test_input_state(); + zoom.set_image(image(4)); + let generation = zoom.image_generation(); + zoom.set_active_geometry(Some(crop_geometry((0, 0)).with_known_output_count(Some(1)))); + let layout_generation = zoom.output_layout_generation; + zoom.request_activation(); + zoom.portal_task = Some(PortalTask::spawn( + &tokio::runtime::Handle::current(), + wake.handle(), + async move { Ok((None, layout_generation, image(9))) }, + )); + zoom.portal_in_progress = true; + + poll_until_finished_with_live_outputs(&mut zoom, &mut input, Some(2)).await; + + assert!(!zoom.active); + assert!(!zoom.pending_activation); + assert_eq!(zoom.image_generation(), generation); + assert_eq!(zoom.image().unwrap().data, vec![4; 8]); + assert!(zoom.take_capture_done()); + assert!(input.ui_toast.is_some()); + } + #[tokio::test] async fn supersession_is_ignored_and_explicit_abort_owns_task_cancellation() { let wake = crate::backend::wayland::RuntimeWakeSource::new().unwrap(); diff --git a/src/backend/wayland/zoom/state.rs b/src/backend/wayland/zoom/state.rs index dc3d4e4df..60b0138b1 100644 --- a/src/backend/wayland/zoom/state.rs +++ b/src/backend/wayland/zoom/state.rs @@ -4,8 +4,10 @@ use wayland_protocols_wlr::screencopy::v1::client::zwlr_screencopy_manager_v1::Z use crate::backend::wayland::RuntimeWakeHandle; use crate::backend::wayland::frozen::FrozenImage; use crate::backend::wayland::frozen_geometry::OutputGeometry; +use crate::backend::wayland::portal_capture::layout_token_matches; use crate::backend::wayland::portal_task::PortalTask; use crate::input::InputState; +use crate::input::state::{Toast, ToastPriority}; use super::capture::CaptureSession; use super::{MIN_ZOOM_SCALE, PortalCaptureResult}; @@ -29,6 +31,8 @@ pub struct ZoomState { pub(super) runtime_wake: Option, pub(super) preflight_pending: bool, pub(super) preflight_use_fallback: bool, + preflight_output_id: Option, + preflight_layout_generation: Option, pub(super) capture_done: bool, pub(super) pending_activation: bool, pub active: bool, @@ -72,6 +76,8 @@ impl ZoomState { runtime_wake, preflight_pending: false, preflight_use_fallback: false, + preflight_output_id: None, + preflight_layout_generation: None, capture_done: false, pending_activation: false, active: false, @@ -111,7 +117,7 @@ impl ZoomState { self.image_target_dimensions = self .active_geometry .as_ref() - .and_then(OutputGeometry::buffer_size) + .map(OutputGeometry::buffer_size) .or(Some((image.width, image.height))); self.image = Some(image); self.bump_image_generation(); @@ -145,6 +151,50 @@ impl ZoomState { Some(use_fallback) } + pub(super) fn snapshot_preflight_layout(&mut self) { + self.preflight_output_id = self.active_output_id; + self.preflight_layout_generation = Some(self.output_layout_generation); + } + + pub(super) fn ensure_preflight_layout_current(&self) -> Result<(), String> { + let Some(generation) = self.preflight_layout_generation else { + return Ok(()); + }; + if layout_token_matches( + self.preflight_output_id, + generation, + self.active_output_id, + self.output_layout_generation, + ) { + Ok(()) + } else { + Err("Zoom failed after the display layout changed".to_string()) + } + } + + #[cfg(test)] + pub fn preflight_layout_is_current(&self) -> bool { + self.ensure_preflight_layout_current().is_ok() + } + + pub(super) fn push_stale_layout_toast(input_state: &mut InputState) { + input_state.push_toast( + ToastPriority::Critical, + "zoom", + Toast::error("Zoom failed after the display layout changed"), + ); + } + + pub(super) fn finish_stale_direct_capture(&mut self, input_state: &mut InputState) { + Self::push_stale_layout_toast(input_state); + self.cancel(input_state, false); + } + + fn clear_preflight_layout_snapshot(&mut self) { + self.preflight_output_id = None; + self.preflight_layout_generation = None; + } + pub fn take_capture_done(&mut self) -> bool { let done = self.capture_done; self.capture_done = false; @@ -177,6 +227,7 @@ impl ZoomState { } self.preflight_pending = false; self.preflight_use_fallback = false; + self.clear_preflight_layout_snapshot(); self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { task.cancel(); @@ -206,6 +257,7 @@ impl ZoomState { } self.preflight_pending = false; self.preflight_use_fallback = false; + self.clear_preflight_layout_snapshot(); self.capture_done = true; self.portal_in_progress = false; if let Some(mut task) = self.portal_task.take() { @@ -234,6 +286,7 @@ impl ZoomState { #[cfg(test)] mod tests { use super::*; + use crate::input::state::test_support::make_test_input_state; #[test] fn aborting_pending_activation_completes_the_capture_lifecycle() { @@ -247,4 +300,28 @@ mod tests { assert!(!state.is_engaged()); assert!(state.take_capture_done()); } + + #[test] + fn stale_direct_capture_toasts_and_preserves_the_current_image() { + let mut state = ZoomState::new(None); + let mut input_state = make_test_input_state(); + state.set_image(FrozenImage { + width: 1, + height: 1, + stride: 4, + data: vec![4; 4], + }); + let generation = state.image_generation(); + + state.finish_stale_direct_capture(&mut input_state); + + let toast = input_state + .ui_toast + .as_ref() + .expect("visible stale rejection"); + assert!(toast.message.contains("display layout changed")); + assert_eq!(state.image_generation(), generation); + assert_eq!(state.image().unwrap().data, vec![4; 4]); + assert!(state.take_capture_done()); + } } diff --git a/src/capture/desktop_backdrop.rs b/src/capture/desktop_backdrop.rs index b4f39548a..662ee094b 100644 --- a/src/capture/desktop_backdrop.rs +++ b/src/capture/desktop_backdrop.rs @@ -73,12 +73,9 @@ pub(crate) fn desktop_backdrop_from_argb( request: &DesktopBackdropCaptureRequest, ) -> Result { validate_argb_buffer(&data, width, height)?; - let (logical_width, logical_height, expected_width, expected_height) = - expected_backdrop_dimensions(request)?; + let (geometry, expected_width, expected_height) = expected_backdrop_dimensions(request)?; - if let Some(expected_layout_size) = request - .geometry - .and_then(DesktopBackdropGeometry::screenshot_size) + if let Some(expected_layout_size) = geometry.screenshot_size() && (width, height) != expected_layout_size { return Err(CaptureError::ImageError(format!( @@ -88,14 +85,15 @@ pub(crate) fn desktop_backdrop_from_argb( } if width == expected_width && height == expected_height { - return desktop_backdrop_result(data, width, height, logical_width, logical_height); + return desktop_backdrop_result( + data, + width, + height, + geometry.logical_width, + geometry.logical_height, + ); } - let Some(geometry) = request.geometry else { - return Err(CaptureError::ImageError(format!( - "Desktop backdrop capture returned {width}x{height}, but active output is {expected_width}x{expected_height} and no output geometry is available" - ))); - }; let (origin_x, origin_y) = geometry.physical_origin().ok_or_else(|| { CaptureError::ImageError( "Active output crop origin is unavailable for desktop capture".to_string(), @@ -120,14 +118,14 @@ pub(crate) fn desktop_backdrop_from_argb( cropped, expected_width, expected_height, - logical_width, - logical_height, + geometry.logical_width, + geometry.logical_height, ) } fn expected_backdrop_dimensions( request: &DesktopBackdropCaptureRequest, -) -> Result<(u32, u32, u32, u32), CaptureError> { +) -> Result<(DesktopBackdropGeometry, u32, u32), CaptureError> { let scale = u32::try_from(request.scale).map_err(|_| { CaptureError::ImageError(format!("Invalid desktop backdrop scale: {}", request.scale)) })?; @@ -137,38 +135,24 @@ fn expected_backdrop_dimensions( )); } - if let Some(geometry) = request.geometry { - if (request.logical_width, request.logical_height) - != (geometry.logical_width, geometry.logical_height) - { - return Err(CaptureError::ImageError( - "Desktop backdrop surface does not match active output geometry".to_string(), - )); - } - let (width, height) = geometry.physical_size().ok_or_else(|| { - CaptureError::ImageError("Active output dimensions are too large".to_string()) - })?; - if geometry.logical_width == 0 || geometry.logical_height == 0 || width == 0 || height == 0 - { - return Err(CaptureError::ImageError( - "Desktop backdrop capture requires a non-empty active output geometry".to_string(), - )); - } - Ok(( - geometry.logical_width, - geometry.logical_height, - width, - height, - )) - } else { - let width = request.logical_width.checked_mul(scale).ok_or_else(|| { - CaptureError::ImageError("Active output width is too large".to_string()) - })?; - let height = request.logical_height.checked_mul(scale).ok_or_else(|| { - CaptureError::ImageError("Active output height is too large".to_string()) - })?; - Ok((request.logical_width, request.logical_height, width, height)) + let Some(geometry) = request.geometry else { + return Err(CaptureError::ImageError( + "Desktop backdrop capture requires verified active output geometry".to_string(), + )); + }; + if (request.logical_width, request.logical_height) + != (geometry.logical_width, geometry.logical_height) + { + return Err(CaptureError::ImageError( + "Desktop backdrop surface does not match active output geometry".to_string(), + )); } + let (width, height) = geometry.verified_physical_size().ok_or_else(|| { + CaptureError::ImageError( + "Active output pixel size is unavailable for desktop capture".to_string(), + ) + })?; + Ok((geometry, width, height)) } fn validate_argb_buffer(data: &[u8], width: u32, height: u32) -> Result<(), CaptureError> { diff --git a/src/capture/tests/desktop_backdrop.rs b/src/capture/tests/desktop_backdrop.rs index 0edd1cf35..badf9b410 100644 --- a/src/capture/tests/desktop_backdrop.rs +++ b/src/capture/tests/desktop_backdrop.rs @@ -21,7 +21,19 @@ fn request( #[test] fn desktop_backdrop_accepts_exact_active_output_size() { let data = vec![7u8; 4 * 2 * 4]; - let result = desktop_backdrop_from_argb(data.clone(), 4, 2, &request(2, 1, 2, None)) + let geometry = DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 2, + logical_height: 1, + physical_width: Some(4), + physical_height: Some(2), + crop_x: Some(0), + crop_y: Some(0), + screenshot_width: Some(4), + screenshot_height: Some(2), + }; + let result = desktop_backdrop_from_argb(data.clone(), 4, 2, &request(2, 1, 2, Some(geometry))) .expect("exact backdrop"); assert_eq!(result.width, 4); @@ -32,6 +44,43 @@ fn desktop_backdrop_accepts_exact_active_output_size() { assert_eq!(result.data.as_ref(), data.as_slice()); } +#[test] +fn desktop_backdrop_rejects_missing_geometry() { + let error = desktop_backdrop_from_argb(vec![7u8; 4 * 2 * 4], 4, 2, &request(2, 1, 2, None)) + .expect_err("backdrop capture must not guess logical * scale"); + + assert!( + error + .to_string() + .contains("requires verified active output geometry"), + "unexpected error: {error}" + ); +} + +#[test] +fn desktop_backdrop_rejects_unverified_physical_size() { + let geometry = DesktopBackdropGeometry { + logical_x: 2, + logical_y: 0, + logical_width: 2, + logical_height: 1, + physical_width: None, + physical_height: None, + crop_x: Some(2), + crop_y: Some(0), + screenshot_width: None, + screenshot_height: None, + }; + + let error = desktop_backdrop_from_argb(vec![0; 6 * 4], 6, 1, &request(2, 1, 1, Some(geometry))) + .expect_err("missing mode pixels must fail closed"); + + assert!( + error.to_string().contains("pixel size is unavailable"), + "unexpected error: {error}" + ); +} + #[test] fn desktop_backdrop_accepts_fractional_scale_output_mode_size() { let data = vec![7u8; 5 * 3 * 4]; @@ -40,7 +89,6 @@ fn desktop_backdrop_accepts_fractional_scale_output_mode_size() { logical_y: 0, logical_width: 3, logical_height: 2, - scale: 2, physical_width: Some(5), physical_height: Some(3), crop_x: Some(0), @@ -65,7 +113,6 @@ fn desktop_backdrop_rejects_a_surface_that_is_not_the_active_output() { logical_y: 0, logical_width: 4, logical_height: 2, - scale: 1, physical_width: Some(4), physical_height: Some(2), crop_x: Some(0), @@ -92,9 +139,8 @@ fn desktop_backdrop_crops_multi_output_capture_to_active_output() { logical_y: 0, logical_width: 2, logical_height: 1, - scale: 1, - physical_width: None, - physical_height: None, + physical_width: Some(2), + physical_height: Some(1), crop_x: Some(2), crop_y: Some(0), screenshot_width: None, @@ -117,9 +163,8 @@ fn desktop_backdrop_crop_failure_returns_error() { logical_y: 0, logical_width: 2, logical_height: 2, - scale: 1, - physical_width: None, - physical_height: None, + physical_width: Some(2), + physical_height: Some(2), crop_x: Some(2), crop_y: Some(0), screenshot_width: None, @@ -139,7 +184,7 @@ fn desktop_backdrop_crop_failure_returns_error() { fn desktop_backdrop_crops_mixed_scale_output_using_screenshot_origin() { let outputs = [output(-4, 0, 4, 1, 6, 1), output(0, 0, 4, 1, 4, 1)]; let geometry = - DesktopBackdropGeometry::from_outputs(outputs[1], &outputs, 1).expect("mixed scale origin"); + DesktopBackdropGeometry::from_outputs(outputs[1], &outputs).expect("mixed scale origin"); let mut data = Vec::new(); for pixel in 0u8..10 { data.extend_from_slice(&[pixel, pixel, pixel, 255]); @@ -160,14 +205,14 @@ fn desktop_backdrop_crops_mixed_scale_output_using_screenshot_origin() { fn desktop_backdrop_rejects_layout_bounds_that_exceed_supported_coordinates() { let output = output(i32::MAX - 1, 0, 4, 1, 4, 1); - assert!(DesktopBackdropGeometry::from_outputs(output, &[output], 1).is_none()); + assert!(DesktopBackdropGeometry::from_outputs(output, &[output]).is_none()); } #[test] fn desktop_backdrop_rejects_a_screenshot_whose_layout_size_does_not_match() { let outputs = [output(-4, 0, 4, 1, 6, 1), output(0, 0, 4, 1, 4, 1)]; let geometry = - DesktopBackdropGeometry::from_outputs(outputs[1], &outputs, 1).expect("desktop geometry"); + DesktopBackdropGeometry::from_outputs(outputs[1], &outputs).expect("desktop geometry"); let data = vec![0u8; 11 * 4]; let err = desktop_backdrop_from_argb(data, 11, 1, &request(4, 1, 1, Some(geometry))) @@ -183,7 +228,7 @@ fn desktop_backdrop_rejects_a_screenshot_whose_layout_size_does_not_match() { fn desktop_backdrop_single_output_at_nonzero_logical_origin_crops_at_zero() { let outputs = [output(10, 20, 4, 2, 8, 4)]; let geometry = - DesktopBackdropGeometry::from_outputs(outputs[0], &outputs, 2).expect("single output"); + DesktopBackdropGeometry::from_outputs(outputs[0], &outputs).expect("single output"); assert_eq!(geometry.physical_origin(), Some((0, 0))); } @@ -193,8 +238,8 @@ fn desktop_backdrop_origin_shifts_when_a_left_output_is_removed() { let left = output(-4, 0, 4, 1, 6, 1); let right = output(0, 0, 4, 1, 4, 1); let with_left = - DesktopBackdropGeometry::from_outputs(right, &[left, right], 1).expect("with left"); - let without_left = DesktopBackdropGeometry::from_outputs(right, &[right], 1).expect("removed"); + DesktopBackdropGeometry::from_outputs(right, &[left, right]).expect("with left"); + let without_left = DesktopBackdropGeometry::from_outputs(right, &[right]).expect("removed"); assert_eq!(with_left.physical_origin(), Some((6, 0))); assert_eq!(without_left.physical_origin(), Some((0, 0))); @@ -204,7 +249,7 @@ fn desktop_backdrop_origin_shifts_when_a_left_output_is_removed() { fn desktop_backdrop_normalizes_negative_output_origins() { let outputs = [output(-2, 0, 2, 2, 2, 2), output(0, 0, 3, 2, 3, 2)]; let geometry = - DesktopBackdropGeometry::from_outputs(outputs[0], &outputs, 1).expect("negative origin"); + DesktopBackdropGeometry::from_outputs(outputs[0], &outputs).expect("negative origin"); assert_eq!(geometry.physical_origin(), Some((0, 0))); } @@ -213,7 +258,7 @@ fn desktop_backdrop_normalizes_negative_output_origins() { fn desktop_backdrop_crops_rotated_output_using_transformed_size() { let outputs = [output(0, 0, 2, 4, 4, 2), output(2, 0, 2, 4, 2, 4)]; let geometry = - DesktopBackdropGeometry::from_outputs(outputs[0], &outputs, 1).expect("rotated output"); + DesktopBackdropGeometry::from_outputs(outputs[0], &outputs).expect("rotated output"); let mut data = Vec::new(); for pixel in 0u8..24 { data.extend_from_slice(&[pixel, pixel, pixel, 255]); @@ -222,7 +267,7 @@ fn desktop_backdrop_crops_rotated_output_using_transformed_size() { let result = desktop_backdrop_from_argb(data, 6, 4, &request(2, 4, 1, Some(geometry))).expect("crop"); - assert_eq!(geometry.physical_size(), Some((4, 2))); + assert_eq!(geometry.verified_physical_size(), Some((4, 2))); assert_eq!(geometry.physical_origin(), Some((0, 0))); assert_eq!(result.width, 4); assert_eq!(result.height, 2); diff --git a/src/capture/tests/manager.rs b/src/capture/tests/manager.rs index 6393d8658..6b34824d4 100644 --- a/src/capture/tests/manager.rs +++ b/src/capture/tests/manager.rs @@ -10,8 +10,8 @@ use std::{ use tokio::time::{Duration, sleep}; use crate::capture::{ - DesktopBackdropCaptureRequest, DocumentDeliveryRequest, ImageDeliveryRequest, - ImageFormatMetadata, ImageOperationKind, RenderedDocument, RenderedImage, + DesktopBackdropCaptureRequest, DesktopBackdropGeometry, DocumentDeliveryRequest, + ImageDeliveryRequest, ImageFormatMetadata, ImageOperationKind, RenderedDocument, RenderedImage, dependencies::{CaptureDependencies, CaptureFuture, CaptureSource}, file::FileSaveConfig, manager::{CaptureManager, CapturePoll, CaptureSubmitError}, @@ -424,7 +424,18 @@ async fn desktop_backdrop_completion_releases_the_manager_for_pdf_delivery() { logical_width: 100, logical_height: 100, scale: 1, - geometry: None, + geometry: Some(DesktopBackdropGeometry { + logical_x: 0, + logical_y: 0, + logical_width: 100, + logical_height: 100, + physical_width: Some(100), + physical_height: Some(100), + crop_x: Some(0), + crop_y: Some(0), + screenshot_width: Some(100), + screenshot_height: Some(100), + }), operation: ImageOperationKind::BoardPdfExport, }) .unwrap(); diff --git a/src/capture/types.rs b/src/capture/types.rs index 6c4f39e7d..895672686 100644 --- a/src/capture/types.rs +++ b/src/capture/types.rs @@ -193,7 +193,6 @@ pub struct DesktopBackdropGeometry { pub logical_y: i32, pub logical_width: u32, pub logical_height: u32, - pub scale: i32, pub physical_width: Option, pub physical_height: Option, pub crop_x: Option, @@ -206,14 +205,12 @@ impl DesktopBackdropGeometry { pub fn from_outputs( active: DesktopBackdropOutputGeometry, outputs: &[DesktopBackdropOutputGeometry], - scale: i32, ) -> Option { Some(Self { logical_x: active.logical_x, logical_y: active.logical_y, logical_width: active.logical_width, logical_height: active.logical_height, - scale, physical_width: Some(active.physical_width), physical_height: Some(active.physical_height), crop_x: Some(physical_axis_origin( @@ -231,21 +228,6 @@ impl DesktopBackdropGeometry { }) } - pub fn physical_size(self) -> Option<(u32, u32)> { - if let (Some(width), Some(height)) = (self.physical_width, self.physical_height) - && width > 0 - && height > 0 - { - return Some((width, height)); - } - - let scale = u32::try_from(self.scale).ok()?; - Some(( - self.logical_width.checked_mul(scale)?, - self.logical_height.checked_mul(scale)?, - )) - } - pub fn verified_physical_size(self) -> Option<(u32, u32)> { let width = self.physical_width?; let height = self.physical_height?;