From 73f21e20ce0e37fe8244e74c9913d0526e19eaba Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Sun, 4 Oct 2026 18:01:52 +0100 Subject: [PATCH] general improvement and bug fixes, cursor scaling, DMA-BUF routing and nested backend recovery ignore performance-baseline.md more improvement and code quality fixes --- .github/workflows/compositor-lifecycle.yml | 1 + compositor/src/backends/direct.rs | 30 +- compositor/src/backends/direct/capture.rs | 2 +- compositor/src/backends/direct/redraw.rs | 23 +- compositor/src/cursor.rs | 506 +++++++++++++++--- compositor/src/cursor/tests.rs | 217 ++++++++ compositor/src/dmabuf_imports.rs | 293 ++++++++++ compositor/src/handlers/compositor.rs | 6 +- compositor/src/handlers/mod.rs | 14 +- compositor/src/main.rs | 3 + compositor/src/render/scene.rs | 9 +- compositor/src/startup_tests.rs | 2 +- compositor/src/state.rs | 52 +- .../src/state/window_lifecycle_tests.rs | 184 +++++++ compositor/src/winit.rs | 374 +++++++++++-- docs/performance-baseline.md | 155 ------ docs/portals.md | 7 +- portal/src/backend.rs | 151 +++++- portal/src/bridge.rs | 148 ++++- portal/src/bridge/tests.rs | 158 ++++++ portal/src/capture.rs | 43 +- portal/src/desktop.rs | 248 +++++++-- portal/src/input_capture.rs | 118 +++- 23 files changed, 2287 insertions(+), 457 deletions(-) create mode 100644 compositor/src/cursor/tests.rs create mode 100644 compositor/src/dmabuf_imports.rs delete mode 100644 docs/performance-baseline.md create mode 100644 portal/src/bridge/tests.rs diff --git a/.github/workflows/compositor-lifecycle.yml b/.github/workflows/compositor-lifecycle.yml index 81b8e50..c3e9633 100644 --- a/.github/workflows/compositor-lifecycle.yml +++ b/.github/workflows/compositor-lifecycle.yml @@ -44,3 +44,4 @@ jobs: run: | cargo +1.95.0 test --locked -p ferese state::capture_privacy::tests -- --ignored --nocapture cargo +1.95.0 test --locked -p ferese backends::direct::capture::tests::capture_recomposes_cursor_without_touching_the_display_target -- --ignored --nocapture + cargo +1.95.0 test --locked -p ferese winit::tests::capture_failure_restores_display_target_and_subsequent_frames_render -- --ignored --nocapture diff --git a/compositor/src/backends/direct.rs b/compositor/src/backends/direct.rs index ba9c2e7..10c8368 100644 --- a/compositor/src/backends/direct.rs +++ b/compositor/src/backends/direct.rs @@ -107,6 +107,16 @@ impl DirectBackendState { self.active && !self.topology.reconciling && !self.lid.pending() } + pub(crate) fn process_dmabuf_imports(&mut self, imports: &mut crate::dmabuf_imports::ImportManager) { + if !self.active || self.topology.reconciling { + return; + } + + for device in self.devices.values_mut().filter(|device| device.drm.is_active()) { + imports.process(device.dmabuf_global, &mut device.renderer, Some(device.render_node)); + } + } + pub(crate) fn scene_output(&self, state: &Ferese, output: &Output) -> Output { let source = self .devices @@ -950,6 +960,7 @@ fn open_device(state: &mut Ferese, path: &Path) -> Result(&display_handle, &feedback); + state.dmabuf_imports.register(dmabuf_global); state .direct_backend .as_mut() @@ -1088,6 +1099,23 @@ fn cursor_affected_outputs(state: &Ferese) -> Vec { .collect() } +pub(crate) fn cursor_animation_visible(state: &Ferese) -> bool { + if let Some(backend) = state.direct_backend.as_ref() { + backend.can_render() + && backend.devices.values().any(|device| { + device + .outputs + .values() + .any(|output| output.power.can_render() && redraw::cursor_on_output(state, &output.output)) + }) + } else { + state + .space + .outputs() + .any(|output| redraw::cursor_on_output(state, output)) + } +} + #[track_caller] pub(crate) fn render_surface( state: &mut Ferese, @@ -1272,7 +1300,6 @@ fn render_output( let frame = state.sample_frame(&scene_output, horizon); let rendered = { (|| -> Result> { - state.process_dmabuf_imports(&mut device.renderer, Some(device.render_node)); let elements = if output.mirror_source.is_some() { mirror::elements(state, &mut device.renderer, &scene_output, &frame, &mut output)? } else { @@ -1562,6 +1589,7 @@ fn remove_device(state: &mut Ferese, node: DrmNode) { }; state.loop_handle.remove(device.notifier); + state.dmabuf_imports.remove(device.dmabuf_global); state .dmabuf_state .disable_global::(&state.display_handle, &device.dmabuf_global); diff --git a/compositor/src/backends/direct/capture.rs b/compositor/src/backends/direct/capture.rs index 325234a..853f157 100644 --- a/compositor/src/backends/direct/capture.rs +++ b/compositor/src/backends/direct/capture.rs @@ -67,7 +67,7 @@ pub(super) fn capture_mirror( } } -fn capture_request( +pub(crate) fn capture_request( state: &mut Ferese, output: &Output, include_cursor: bool, diff --git a/compositor/src/backends/direct/redraw.rs b/compositor/src/backends/direct/redraw.rs index 89bcc44..58be599 100644 --- a/compositor/src/backends/direct/redraw.rs +++ b/compositor/src/backends/direct/redraw.rs @@ -123,22 +123,11 @@ pub(super) fn cursor_on_output(state: &crate::Ferese, output: &smithay::output:: .to_f64() } - CursorImageStatus::Named(icon) => { - let Some(cursor) = state.named_cursors.get(icon) else { + CursorImageStatus::Named(_) => { + let Some(cursor) = state.named_cursor_frame() else { return false; }; - let scale = output.current_scale().fractional_scale(); - // Named buffers use buffer scale 1: Smithay scales their size - // with the output, while the hotspot is subtracted physically. - let size = cursor.size; - Rectangle::::new( - ( - location.x - f64::from(cursor.hotspot.x) / scale, - location.y - f64::from(cursor.hotspot.y) / scale, - ) - .into(), - (f64::from(size.w), f64::from(size.h)).into(), - ) + cursor.logical_rect(location) } CursorImageStatus::Hidden => return false, @@ -353,9 +342,9 @@ mod tests { ); assert!(cursor_on_output(&state, &outputs[0])); assert!(cursor_on_output(&state, &outputs[1]), "cursor image straddles the seam"); - if let CursorImageStatus::Named(icon) = &state.cursor_status { - let cursor = state.named_cursors.get(icon).unwrap(); - let x = 1920.0 - f64::from(cursor.size.w) * 0.75 + f64::from(cursor.hotspot.x) / 2.0; + if let Some(cursor) = state.named_cursor_frame() { + let x = 1920.0 - f64::from(cursor.size.w) / f64::from(cursor.buffer_scale) * 0.75 + + f64::from(cursor.hotspot.x) / f64::from(cursor.buffer_scale); pointer.motion( &mut state, None, diff --git a/compositor/src/cursor.rs b/compositor/src/cursor.rs index fe7b804..fcbc305 100644 --- a/compositor/src/cursor.rs +++ b/compositor/src/cursor.rs @@ -1,49 +1,328 @@ -use std::fs; -use std::sync::OnceLock; +use std::fs::File; +use std::io::Read; +use std::sync::{Arc, Condvar, Mutex, mpsc}; +use std::time::{Duration, Instant}; use smithay::backend::allocator::Fourcc; use smithay::backend::renderer::element::memory::MemoryRenderBuffer; -use smithay::input::pointer::CursorIcon; -use smithay::utils::{Buffer, Point, Size, Transform}; +use smithay::input::pointer::{CursorIcon, CursorImageStatus}; +use smithay::reexports::calloop::timer::{TimeoutAction, Timer}; +use smithay::reexports::calloop::{LoopSignal, RegistrationToken}; +use smithay::utils::{Buffer, Logical, Physical, Point, Rectangle, Size, Transform}; use xcursor::CursorTheme; -use xcursor::parser::Image; const DEFAULT_CURSOR_SIZE: u32 = 24; +const MAX_FILE_BYTES: usize = 8 * 1024 * 1024; +const MAX_FRAMES: usize = 256; +const MAX_PIXELS: usize = 2 * 1024 * 1024; +const MAX_CACHE_BYTES: usize = 32 * 1024 * 1024; #[derive(Debug, Clone)] -pub(crate) struct NamedCursor { +pub(crate) struct CursorFrame { pub buffer: MemoryRenderBuffer, pub hotspot: Point, pub size: Size, + pub buffer_scale: i32, + delay: Duration, } -pub(crate) fn cursor_theme() -> CursorTheme { - let name = std::env::var("XCURSOR_THEME").unwrap_or_else(|_| "default".into()); +impl CursorFrame { + pub(crate) fn logical_rect(&self, pointer: Point) -> Rectangle { + Rectangle::new( + pointer + - Point::from(( + f64::from(self.hotspot.x) / f64::from(self.buffer_scale), + f64::from(self.hotspot.y) / f64::from(self.buffer_scale), + )), + ( + f64::from(self.size.w) / f64::from(self.buffer_scale), + f64::from(self.size.h) / f64::from(self.buffer_scale), + ) + .into(), + ) + } - CursorTheme::load(&name) -} - -pub(crate) fn load_named_cursor(theme: &CursorTheme, icon: CursorIcon) -> NamedCursor { - let requested_size = cursor_size(); - let image = cursor_names(icon) - .iter() - .find_map(|name| load_image(theme, name, requested_size)); - - match image { - Some(image) => cursor_from_image(image), - None => fallback_cursor(), + pub(crate) fn physical_location(&self, pointer: Point, scale: f64) -> Point { + self.logical_rect(pointer).loc.to_physical(scale) } } -fn cursor_size() -> u32 { - static SIZE: OnceLock = OnceLock::new(); +#[derive(Debug, Clone)] +pub(crate) struct NamedCursor { + frames: Arc<[CursorFrame]>, + bytes: usize, + requested_scale: i32, +} - *SIZE.get_or_init(|| { - std::env::var("XCURSOR_SIZE") - .ok() - .and_then(|value| value.parse().ok()) - .filter(|size| *size > 0) - .unwrap_or(DEFAULT_CURSOR_SIZE) +impl NamedCursor { + fn sample(&self, elapsed: Duration) -> (usize, Option) { + if self.frames.len() <= 1 { + return (0, None); + } + + let period: Duration = self.frames.iter().map(|frame| frame.delay).sum(); + let mut offset = elapsed.as_nanos() % period.as_nanos(); + for (index, frame) in self.frames.iter().enumerate() { + if offset < frame.delay.as_nanos() { + return (index, Some(frame.delay - Duration::from_nanos(offset as u64))); + } + offset -= frame.delay.as_nanos(); + } + unreachable!("cursor phase is within the bounded, nonzero period") + } +} + +#[derive(Default)] +struct Requests { + pending: Option<(CursorIcon, i32)>, + in_flight: Option<(CursorIcon, i32)>, + closed: bool, +} + +struct Loaded { + icon: CursorIcon, + scale: i32, + cursor: Option, +} + +pub(crate) struct Loader { + shared: Arc<(Mutex, Condvar)>, + results: mpsc::Receiver, +} + +impl Loader { + pub(crate) fn new(signal: LoopSignal) -> std::io::Result { + Self::with_loader(signal, { + // Theme discovery and inherited-theme reads belong to the worker too. + let mut theme = None; + move |(icon, scale)| { + let theme = theme.get_or_insert_with(|| { + CursorTheme::load(&std::env::var("XCURSOR_THEME").unwrap_or_else(|_| "default".into())) + }); + load_named_cursor(theme, icon, scale) + } + }) + } + + fn with_loader( + signal: LoopSignal, + mut load: impl FnMut((CursorIcon, i32)) -> Option + Send + 'static, + ) -> std::io::Result { + let shared = Arc::new(( + Mutex::new(Requests { + pending: Some((CursorIcon::Default, 1)), + ..Requests::default() + }), + Condvar::new(), + )); + let worker = shared.clone(); + let (sender, results) = mpsc::sync_channel(1); + std::thread::Builder::new() + .name("ferese-cursors".into()) + .spawn(move || { + loop { + let (requests, ready) = &*worker; + let mut requests = ready + .wait_while(requests.lock().unwrap(), |requests| { + !requests.closed && requests.pending.is_none() + }) + .unwrap(); + if requests.closed { + break; + } + let request = requests.pending.take().unwrap(); + requests.in_flight = Some(request); + drop(requests); + let cursor = load(request); + let (icon, scale) = request; + if sender.send(Loaded { icon, scale, cursor }).is_err() { + break; + } + signal.wakeup(); + worker.0.lock().unwrap().in_flight = None; + } + })?; + Ok(Self { shared, results }) + } + + pub(crate) fn request(&self, icon: CursorIcon, scale: i32) { + let request = (icon, scale); + let mut requests = self.shared.0.lock().unwrap(); + if requests.in_flight == Some(request) || requests.pending == Some(request) { + return; + } + requests.pending = Some(request); + self.shared.1.notify_one(); + } +} + +impl Drop for Loader { + fn drop(&mut self) { + let mut requests = self.shared.0.lock().unwrap(); + requests.closed = true; + requests.pending = None; + self.shared.1.notify_one(); + } +} + +pub(crate) struct Animation { + icon: Option, + epoch: Instant, + frame: usize, + timer: Option, + pub(crate) reset: bool, + pub(crate) buffer_scale: i32, +} + +impl Default for Animation { + fn default() -> Self { + Self { + icon: None, + epoch: Instant::now(), + frame: 0, + timer: None, + reset: false, + buffer_scale: 1, + } + } +} + +impl crate::Ferese { + pub(crate) fn named_cursor_frame(&self) -> Option<&CursorFrame> { + let CursorImageStatus::Named(icon) = self.cursor_status else { + return None; + }; + let cursor = self + .named_cursors + .get(&icon) + .or_else(|| self.named_cursors.get(&CursorIcon::Default))?; + let frame = if self.cursor_animation.icon == Some(icon) { + self.cursor_animation.frame + } else { + 0 + }; + cursor.frames.get(frame).or_else(|| cursor.frames.first()) + } + + pub(crate) fn update_named_cursor(&mut self, now: Instant) { + self.cursor_animation.buffer_scale = if let Some(backend) = self.direct_backend.as_ref() { + backend + .physical_outputs() + .map(|output| output.current_scale().fractional_scale().ceil() as i32) + .max() + } else { + self.space + .outputs() + .map(|output| output.current_scale().fractional_scale().ceil() as i32) + .max() + } + .unwrap_or(1) + .clamp(1, 8); + while let Ok(Loaded { icon, scale, cursor }) = self.cursor_loader.results.try_recv() { + let mut cursor = cursor.unwrap_or_else(fallback_cursor); + cursor.requested_scale = scale; + let active = match self.cursor_status { + CursorImageStatus::Named(icon) => Some(icon), + _ => None, + }; + let affects_current = active == Some(icon) + || (icon == CursorIcon::Default + && active.is_some_and(|active| !self.named_cursors.contains_key(&active))); + let mut bytes: usize = self.named_cursors.values().map(|cursor| cursor.bytes).sum(); + while bytes + cursor.bytes > MAX_CACHE_BYTES { + let Some(evict) = self + .named_cursors + .keys() + .copied() + .find(|key| *key != CursorIcon::Default && *key != icon && Some(*key) != active) + else { + break; + }; + bytes -= self.named_cursors.remove(&evict).unwrap().bytes; + } + self.named_cursors.insert(icon, cursor); + if affects_current { + self.cursor_animation.reset = true; + self.cursor_redraw_pending |= crate::backends::direct::cursor_animation_visible(self); + } + } + + let icon = match self.cursor_status { + CursorImageStatus::Named(icon) => Some(icon), + _ => None, + }; + if let Some(icon) = icon + && self + .named_cursors + .get(&icon) + .is_none_or(|cursor| cursor.requested_scale != self.cursor_animation.buffer_scale) + { + self.cursor_loader.request(icon, self.cursor_animation.buffer_scale); + } + let animated = icon + .and_then(|icon| { + self.named_cursors + .get(&icon) + .or_else(|| self.named_cursors.get(&CursorIcon::Default)) + }) + .is_some_and(|cursor| cursor.frames.len() > 1); + let visible = animated && crate::backends::direct::cursor_animation_visible(self); + if !visible || self.cursor_animation.icon != icon || self.cursor_animation.reset { + if let Some(token) = self.cursor_animation.timer.take() { + self.loop_handle.remove(token); + } + self.cursor_animation.icon = icon.filter(|_| visible); + self.cursor_animation.epoch = now; + self.cursor_animation.frame = 0; + self.cursor_animation.reset = false; + } + if !visible { + return; + } + let icon = icon.unwrap(); + let cursor = self + .named_cursors + .get(&icon) + .or_else(|| self.named_cursors.get(&CursorIcon::Default)) + .unwrap(); + let (frame, remaining) = cursor.sample(now.saturating_duration_since(self.cursor_animation.epoch)); + if self.cursor_animation.frame != frame { + self.cursor_animation.frame = frame; + self.cursor_redraw_pending = true; + } + if let Some(remaining) = remaining + && self.cursor_animation.timer.is_none() + { + match self + .loop_handle + .insert_source(Timer::from_deadline(now + remaining), |_, _, state| { + state.cursor_animation.timer = None; + state.cursor_redraw_pending = true; + TimeoutAction::Drop + }) { + Ok(token) => self.cursor_animation.timer = Some(token), + Err(error) => tracing::warn!(%error, "could not schedule animated cursor"), + } + } + } +} + +fn load_named_cursor(theme: &CursorTheme, icon: CursorIcon, scale: i32) -> Option { + let requested = std::env::var("XCURSOR_SIZE") + .ok() + .and_then(|size| size.parse::().ok()) + .filter(|size| *size > 0 && *size <= 1024) + .unwrap_or(DEFAULT_CURSOR_SIZE); + cursor_names(icon).iter().find_map(|name| { + let path = theme.load_icon(name)?; + let file = File::open(path).ok()?; + let metadata = file.metadata().ok()?; + if !metadata.is_file() || metadata.len() > MAX_FILE_BYTES as u64 { + return None; + } + let mut bytes = Vec::new(); + file.take((MAX_FILE_BYTES + 1) as u64).read_to_end(&mut bytes).ok()?; + parse_cursor(&bytes, requested, scale) }) } @@ -56,37 +335,113 @@ fn cursor_names(icon: CursorIcon) -> [&'static str; 3] { } } -fn load_image(theme: &CursorTheme, name: &str, requested_size: u32) -> Option { - let path = theme.load_icon(name)?; - let bytes = fs::read(path).ok()?; - - xcursor::parser::parse_xcursor(&bytes)?.into_iter().min_by_key(|image| { - ( - image.size.abs_diff(requested_size), - image.width.abs_diff(requested_size), - ) +// Inspect every image before allocating buffers, then decode only the selected +// nominal size. TOC aliases count toward the budget just like separate frames. +fn parse_cursor(bytes: &[u8], requested: u32, scale: i32) -> Option { + fn word(bytes: &[u8], offset: usize) -> Option { + Some(u32::from_le_bytes( + bytes.get(offset..offset.checked_add(4)?)?.try_into().ok()?, + )) + } + if bytes.len() > MAX_FILE_BYTES || bytes.get(..4)? != b"Xcur" { + return None; + } + let header = word(bytes, 4)? as usize; + let count = word(bytes, 12)? as usize; + if header < 16 || count > MAX_FRAMES || header.checked_add(count.checked_mul(12)?)? > bytes.len() { + return None; + } + let mut images = Vec::new(); + let mut total_pixels = 0usize; + for i in 0..count { + let toc = header + i * 12; + if word(bytes, toc)? != 0xfffd0002 { + continue; + } + let nominal = word(bytes, toc + 4)?; + let pos = word(bytes, toc + 8)? as usize; + if word(bytes, pos)? != 36 + || word(bytes, pos.checked_add(4)?)? != 0xfffd0002 + || word(bytes, pos.checked_add(8)?)? != nominal + || word(bytes, pos.checked_add(12)?)? != 1 + { + return None; + } + let width = word(bytes, pos + 16)?; + let height = word(bytes, pos + 20)?; + let xhot = word(bytes, pos + 24)?; + let yhot = word(bytes, pos + 28)?; + if nominal == 0 || width == 0 || height == 0 || width > 1024 || height > 1024 || xhot > width || yhot > height { + return None; + } + let pixels = (width as usize).checked_mul(height as usize)?; + total_pixels = total_pixels.checked_add(pixels)?; + if total_pixels > MAX_PIXELS { + return None; + } + let start = pos.checked_add(36)?; + let end = start.checked_add(pixels.checked_mul(4)?)?; + let rgba = bytes.get(start..end)?; + images.push((nominal, width, height, xhot, yhot, word(bytes, pos + 32)?, rgba)); + } + let base = images + .iter() + .map(|image| image.0) + .min_by_key(|size| (size.abs_diff(requested), *size))?; + let scale = scale.clamp(1, 8); + let desired = base.checked_mul(scale as u32)?; + let nominal = images + .iter() + .map(|image| image.0) + .min_by_key(|size| (size.abs_diff(desired), *size))?; + let mut frames = Vec::new(); + let mut asset_bytes = 0usize; + for (_, width, height, xhot, yhot, delay, rgba) in images.into_iter().filter(|image| image.0 == nominal) { + let ratio = f64::from(desired) / f64::from(nominal); + let w = (f64::from(width) * ratio).round().max(1.0) as u32; + let h = (f64::from(height) * ratio).round().max(1.0) as u32; + if w > 1024 || h > 1024 { + return None; + } + let size: Size = (w as i32, h as i32).into(); + asset_bytes = asset_bytes.checked_add((w as usize).checked_mul(h as usize)?.checked_mul(4)?)?; + if asset_bytes > MAX_PIXELS * 4 { + return None; + } + let resized; + let pixels = if w != width || h != height { + let original = image::RgbaImage::from_raw(width, height, rgba.to_vec())?; + resized = image::imageops::resize(&original, w, h, image::imageops::FilterType::Triangle); + resized.as_raw().as_slice() + } else { + rgba + }; + frames.push(CursorFrame { + buffer: MemoryRenderBuffer::from_slice(pixels, Fourcc::Abgr8888, size, scale, Transform::Normal, None), + hotspot: ( + (f64::from(xhot) * ratio).round() as i32, + (f64::from(yhot) * ratio).round() as i32, + ) + .into(), + size, + buffer_scale: scale, + // Zero-delay themes need a finite cadence rather than a busy loop. + delay: Duration::from_millis(if delay == 0 { 16 } else { u64::from(delay) }), + }); + } + Some(NamedCursor { + frames: frames.into(), + bytes: asset_bytes, + requested_scale: scale, }) } -fn cursor_from_image(image: Image) -> NamedCursor { - let size = Size::::from((image.width as i32, image.height as i32)); - let buffer = MemoryRenderBuffer::from_slice(&image.pixels_rgba, Fourcc::Abgr8888, size, 1, Transform::Normal, None); - - NamedCursor { - buffer, - hotspot: Point::from((image.xhot as i32, image.yhot as i32)), - size, - } -} - -fn fallback_cursor() -> NamedCursor { +pub(crate) fn fallback_cursor() -> NamedCursor { const WIDTH: usize = 12; const HEIGHT: usize = 18; let mut pixels = vec![0_u8; WIDTH * HEIGHT * 4]; - for y in 0..HEIGHT { let body_width = (y / 2 + 1).min(8); - for x in 0..body_width { let edge = x == 0 || x + 1 == body_width || y == 0; let offset = (y * WIDTH + x) * 4; @@ -94,39 +449,26 @@ fn fallback_cursor() -> NamedCursor { pixels[offset..offset + 4].copy_from_slice(&[color, color, color, 255]); } } - NamedCursor { - buffer: MemoryRenderBuffer::from_slice( - &pixels, - Fourcc::Abgr8888, - Size::::from((WIDTH as i32, HEIGHT as i32)), - 1, - Transform::Normal, - None, - ), - hotspot: Point::default(), - size: (WIDTH as i32, HEIGHT as i32).into(), + frames: vec![CursorFrame { + buffer: MemoryRenderBuffer::from_slice( + &pixels, + Fourcc::Abgr8888, + (WIDTH as i32, HEIGHT as i32), + 1, + Transform::Normal, + None, + ), + hotspot: Point::default(), + size: (WIDTH as i32, HEIGHT as i32).into(), + buffer_scale: 1, + delay: Duration::from_millis(16), + }] + .into(), + bytes: pixels.len(), + requested_scale: 1, } } #[cfg(test)] -mod tests { - use smithay::input::pointer::CursorIcon; - - use super::cursor_names; - - #[test] - fn supplies_legacy_fallback_names_for_common_cursors() { - assert_eq!(cursor_names(CursorIcon::Default), ["default", "left_ptr", "arrow"]); - assert_eq!(cursor_names(CursorIcon::Pointer), ["pointer", "hand2", "left_ptr"]); - assert_eq!(cursor_names(CursorIcon::Text), ["text", "xterm", "left_ptr"]); - } - - #[test] - fn falls_back_to_default_for_other_named_cursors() { - assert_eq!( - cursor_names(CursorIcon::Crosshair), - ["crosshair", "default", "left_ptr"] - ); - } -} +mod tests; diff --git a/compositor/src/cursor/tests.rs b/compositor/src/cursor/tests.rs new file mode 100644 index 0000000..81edbad --- /dev/null +++ b/compositor/src/cursor/tests.rs @@ -0,0 +1,217 @@ +use super::*; +use smithay::input::SeatHandler; +use smithay::input::pointer::MotionEvent; +use smithay::output::{Mode, Output, PhysicalProperties, Subpixel}; +use smithay::reexports::calloop::EventLoop; +use smithay::utils::SERIAL_COUNTER; + +fn file(frames: &[(u32, u32, u32, u32)]) -> Vec { + let mut bytes = b"Xcur".to_vec(); + bytes.extend([16u32, 1, frames.len() as u32].into_iter().flat_map(u32::to_le_bytes)); + let mut pos = 16 + frames.len() * 12; + for &(size, width, _, _) in frames { + bytes.extend([0xfffd0002, size, pos as u32].into_iter().flat_map(u32::to_le_bytes)); + pos += 36 + width as usize * width as usize * 4; + } + for &(size, width, hotspot, delay) in frames { + bytes.extend( + [36, 0xfffd0002, size, 1, width, width, hotspot, hotspot, delay] + .into_iter() + .flat_map(u32::to_le_bytes), + ); + bytes.extend([255, 255, 255, 255].repeat(width as usize * width as usize)); + } + bytes +} + +#[test] +fn hotspot_and_footprint_use_the_images_scale() { + let bytes = file(&[(24, 24, 12, 50), (48, 48, 24, 50)]); + for scale in [1.0f64, 1.25, 2.0] { + let asset = parse_cursor(&bytes, 24, scale.ceil() as i32).unwrap(); + let frame = &asset.frames[0]; + let pointer: Point = (100.0, 120.0).into(); + let location = frame.physical_location(pointer, scale); + let hotspot = f64::from(frame.hotspot.x) * scale / f64::from(frame.buffer_scale); + assert_eq!(location.x + hotspot, pointer.x * scale); + assert_eq!(location.y + hotspot, pointer.y * scale); + assert_eq!(frame.logical_rect(pointer).size, (24.0, 24.0).into()); + if scale > 1.0 { + assert_eq!(frame.size.w, 48); + } + } +} + +#[test] +fn unavailable_hidpi_size_preserves_logical_size_and_hotspot() { + let asset = parse_cursor(&file(&[(32, 32, 16, 50)]), 24, 2).unwrap(); + let frame = &asset.frames[0]; + assert_eq!(frame.buffer_scale, 2); + assert_eq!(frame.size.w, 64); + assert_eq!(frame.hotspot.x, 32); + assert_eq!(frame.logical_rect((100.0, 100.0).into()).size, (32.0, 32.0).into()); +} + +#[test] +fn selects_a_nominal_sequence_and_preserves_frame_delays() { + let asset = parse_cursor(&file(&[(48, 48, 24, 70), (24, 24, 12, 30), (24, 24, 12, 80)]), 24, 1).unwrap(); + assert_eq!(asset.frames.len(), 2); + for (time, index, remaining) in [ + (0, 0, 30), + (29, 0, 1), + (30, 1, 80), + (109, 1, 1), + (110, 0, 30), + (400, 1, 40), + ] { + assert_eq!( + asset.sample(Duration::from_millis(time)), + (index, Some(Duration::from_millis(remaining))) + ); + } + let still = parse_cursor(&file(&[(24, 24, 12, 0)]), 24, 1).unwrap(); + assert_eq!(still.sample(Duration::from_secs(300)), (0, None)); + let zero = parse_cursor(&file(&[(24, 24, 12, 0), (24, 24, 12, 0)]), 24, 1).unwrap(); + assert_eq!(zero.sample(Duration::from_millis(16)).0, 1); +} + +#[test] +fn rejects_oversized_and_malformed_assets_before_buffer_allocation() { + assert!(parse_cursor(&vec![0; MAX_FILE_BYTES + 1], 24, 1).is_none()); + let mut bytes = file(&[(24, 24, 12, 30)]); + bytes[12..16].copy_from_slice(&((MAX_FRAMES + 1) as u32).to_le_bytes()); + assert!(parse_cursor(&bytes, 24, 1).is_none()); + let mut bytes = file(&[(24, 24, 12, 30)]); + bytes[16 + 8..16 + 12].copy_from_slice(&u32::MAX.to_le_bytes()); + assert!(parse_cursor(&bytes, 24, 1).is_none()); + let mut bytes = file(&[(24, 24, 12, 30)]); + bytes[28 + 16..28 + 20].copy_from_slice(&2048u32.to_le_bytes()); + assert!(parse_cursor(&bytes, 24, 1).is_none()); + // Repeated TOC references must not bypass the aggregate pixel limit. + let mut bytes = file(&[(1024, 1024, 0, 30)]); + let frame = bytes.split_off(28); + bytes.truncate(16); + bytes[12..16].copy_from_slice(&3u32.to_le_bytes()); + for _ in 0..3 { + bytes.extend([0xfffd0002, 1024, 52].into_iter().flat_map(u32::to_le_bytes)); + } + bytes.extend(frame); + assert!(parse_cursor(&bytes, 24, 1).is_none()); + assert!(parse_cursor(&file(&[(24, 24, 12, 30)])[..40], 24, 1).is_none()); +} + +#[test] +fn delayed_loading_does_not_block_cursor_callbacks_or_pointer_dispatch() { + if !crate::startup_tests::private_runtime( + "cursor::tests::delayed_loading_does_not_block_cursor_callbacks_or_pointer_dispatch", + ) { + return; + } + let mut events = EventLoop::try_new().unwrap(); + let mut state = crate::startup_tests::state(&mut events); + let (started, calls) = mpsc::channel(); + let (release, held) = mpsc::channel(); + state.cursor_loader = Loader::with_loader(events.get_signal(), move |request| { + started.send(request).unwrap(); + if request.0 == CursorIcon::Default { + held.recv_timeout(Duration::from_secs(2)).unwrap(); + } + None + }) + .unwrap(); + assert_eq!( + calls.recv_timeout(Duration::from_secs(2)).unwrap().0, + CursorIcon::Default + ); + let seat = state.seat.clone(); + state.cursor_image(&seat, CursorImageStatus::Named(CursorIcon::Pointer)); + state.cursor_image(&seat, CursorImageStatus::Named(CursorIcon::Text)); + let pointer = state.seat.get_pointer().unwrap(); + pointer.motion( + &mut state, + None, + &MotionEvent { + location: (10.0, 20.0).into(), + serial: SERIAL_COUNTER.next_serial(), + time: 1, + }, + ); + state.cursor_image(&seat, CursorImageStatus::Named(CursorIcon::Text)); + crate::after_dispatch_with_redraw(&mut state, |_, _| {}); + assert_eq!(pointer.current_location(), (10.0, 20.0).into()); + assert!( + state.named_cursor_frame().is_some(), + "cached fallback must remain visible" + ); + assert!(calls.try_recv().is_err(), "decoder is still deliberately blocked"); + release.send(()).unwrap(); + assert_eq!(calls.recv_timeout(Duration::from_secs(2)).unwrap().0, CursorIcon::Text); +} + +#[test] +fn cursor_animation_advances_and_cancels_on_hide_and_shape_change() { + if !crate::startup_tests::private_runtime( + "cursor::tests::cursor_animation_advances_and_cancels_on_hide_and_shape_change", + ) { + return; + } + let mut events = EventLoop::try_new().unwrap(); + let mut state = crate::startup_tests::state(&mut events); + // No background result can replace the synthetic fixture. + state.cursor_loader = Loader::with_loader(events.get_signal(), |_| None).unwrap(); + let output = Output::new( + "cursor-test".into(), + PhysicalProperties { + size: (0, 0).into(), + subpixel: Subpixel::Unknown, + make: "test".into(), + model: "test".into(), + }, + ); + output.change_current_state( + Some(Mode { + size: (800, 600).into(), + refresh: 60_000, + }), + None, + None, + None, + ); + state.space.map_output(&output, (0, 0)); + let asset = parse_cursor(&file(&[(24, 24, 12, 30), (24, 24, 12, 80)]), 24, 1).unwrap(); + state.named_cursors.insert(CursorIcon::Wait, asset); + state.cursor_status = CursorImageStatus::Named(CursorIcon::Wait); + let now = Instant::now(); + state.update_named_cursor(now); + assert_eq!(state.cursor_animation.frame, 0); + assert!(state.cursor_animation.timer.is_some()); + // Invoke the same deadline callback by dispatching the real timer. + while Instant::now() < now + Duration::from_millis(35) { + events.dispatch(Duration::from_millis(5), &mut state).unwrap(); + } + state.update_named_cursor(now + Duration::from_millis(30)); + assert_eq!(state.cursor_animation.frame, 1); + assert!(state.cursor_animation.timer.is_some()); + let seat = state.seat.clone(); + state.cursor_image(&seat, CursorImageStatus::Hidden); + state.update_named_cursor(now + Duration::from_millis(31)); + assert!(state.cursor_animation.timer.is_none()); + state.cursor_image(&seat, CursorImageStatus::Named(CursorIcon::Wait)); + state.update_named_cursor(now + Duration::from_millis(32)); + assert!(state.cursor_animation.timer.is_some()); + state.named_cursors.insert(CursorIcon::Text, fallback_cursor()); + state.cursor_image(&seat, CursorImageStatus::Named(CursorIcon::Text)); + state.update_named_cursor(now + Duration::from_millis(33)); + assert!(state.cursor_animation.timer.is_none()); +} + +#[test] +fn supplies_legacy_fallback_names_for_common_cursors() { + assert_eq!(cursor_names(CursorIcon::Default), ["default", "left_ptr", "arrow"]); + assert_eq!(cursor_names(CursorIcon::Pointer), ["pointer", "hand2", "left_ptr"]); + assert_eq!(cursor_names(CursorIcon::Text), ["text", "xterm", "left_ptr"]); + assert_eq!( + cursor_names(CursorIcon::Crosshair), + ["crosshair", "default", "left_ptr"] + ); +} diff --git a/compositor/src/dmabuf_imports.rs b/compositor/src/dmabuf_imports.rs new file mode 100644 index 0000000..8db4bd7 --- /dev/null +++ b/compositor/src/dmabuf_imports.rs @@ -0,0 +1,293 @@ +use std::collections::{HashSet, VecDeque}; +use std::time::{Duration, Instant}; + +use smithay::backend::allocator::dmabuf::Dmabuf; +use smithay::backend::drm::DrmNode; +use smithay::backend::renderer::ImportDma; +use smithay::reexports::calloop::RegistrationToken; +use smithay::reexports::calloop::timer::{TimeoutAction, Timer}; +use smithay::reexports::wayland_server::backend::ClientId; +use smithay::wayland::dmabuf::{DmabufGlobal, ImportNotifier}; + +use crate::Ferese; + +const MAX_PENDING: usize = 64; +const MAX_PER_CLIENT: usize = 16; +const MAX_WAIT: Duration = Duration::from_secs(5); + +struct Pending { + owner: K, + client: C, + deadline: Instant, + value: T, +} + +struct Queue { + pending: VecDeque>, +} + +impl Default for Queue { + fn default() -> Self { + Self { + pending: VecDeque::new(), + } + } +} + +impl Queue { + fn enqueue(&mut self, owner: K, client: C, value: T, now: Instant) -> Result<(), T> { + if self.pending.len() >= MAX_PENDING + || self.pending.iter().filter(|entry| entry.client == client).count() >= MAX_PER_CLIENT + { + return Err(value); + } + + self.pending.push_back(Pending { + owner, + client, + value, + deadline: now + MAX_WAIT, + }); + Ok(()) + } + + fn process(&mut self, owner: &K, mut complete: impl FnMut(T)) { + self.remove_if(|entry| &entry.owner == owner, |entry| complete(entry.value)); + } + + fn remove_if( + &mut self, + mut remove: impl FnMut(&Pending) -> bool, + mut complete: impl FnMut(Pending), + ) { + for _ in 0..self.pending.len() { + let entry = self.pending.pop_front().unwrap(); + if remove(&entry) { + complete(entry); + } else { + self.pending.push_back(entry); + } + } + } + + fn deadline(&self) -> Option { + self.pending.front().map(|entry| entry.deadline) + } +} + +struct Request { + buffer: Dmabuf, + notifier: ImportNotifier, +} + +#[derive(Default)] +pub(crate) struct ImportManager { + globals: HashSet, + queue: Queue, + timer: Option, +} + +impl Drop for ImportManager { + fn drop(&mut self) { + self.queue.remove_if(|_| true, |entry| entry.value.notifier.failed()); + } +} + +impl ImportManager { + pub(crate) fn register(&mut self, global: DmabufGlobal) { + self.globals.insert(global); + } + + pub(crate) fn remove(&mut self, global: DmabufGlobal) { + self.globals.remove(&global); + self.queue.process(&global, |request| request.notifier.failed()); + } + + pub(crate) fn enqueue(&mut self, global: DmabufGlobal, buffer: Dmabuf, notifier: ImportNotifier, now: Instant) { + let Some(client) = notifier.client() else { + notifier.failed(); + return; + }; + + if !self.globals.contains(&global) { + notifier.failed(); + return; + } + + if let Err(request) = self + .queue + .enqueue(global, client.id(), Request { buffer, notifier }, now) + { + request.notifier.failed(); + } + } + + pub(crate) fn process(&mut self, global: DmabufGlobal, renderer: &mut R, node: Option) { + self.process_with(global, node, |buffer| renderer.import_dmabuf(buffer, None).is_ok()); + } + + pub(crate) fn process_with( + &mut self, + global: DmabufGlobal, + node: Option, + mut import: impl FnMut(&Dmabuf) -> bool, + ) { + self.queue.process(&global, |request| { + if request.notifier.client().is_none() { + request.notifier.failed(); + return; + } + + if !import(&request.buffer) { + request.notifier.failed(); + return; + } + + if let Some(node) = node { + request.buffer.set_node(node); + } + + if let Err(error) = request.notifier.successful::() { + tracing::debug!(?error, "dma-buf client disappeared before import completed"); + } + }); + } + + pub(crate) fn expire(&mut self, now: Instant) { + self.queue.remove_if( + |entry| entry.deadline <= now || entry.value.notifier.client().is_none(), + |entry| entry.value.notifier.failed(), + ); + } +} + +impl Ferese { + pub(crate) fn process_pending_dmabuf_imports(&mut self) { + self.dmabuf_imports.expire(Instant::now()); + if self.dmabuf_imports.queue.deadline().is_some() { + if let Some(backend) = self.direct_backend.as_mut() { + backend.process_dmabuf_imports(&mut self.dmabuf_imports); + } else if let Some(backend) = self.nested_backend.as_ref() { + let mut backend = backend.borrow_mut(); + // Nested Ferese publishes exactly one DMA-BUF global. + if let Some(global) = self.dmabuf_imports.globals.iter().next().copied() { + self.dmabuf_imports.process(global, backend.renderer(), None); + } + } + } + + if let Some(deadline) = self.dmabuf_imports.queue.deadline() { + if self.dmabuf_imports.timer.is_none() { + match self + .loop_handle + .insert_source(Timer::from_deadline(deadline), |_, _, state| { + state.dmabuf_imports.expire(Instant::now()); + if let Some(deadline) = state.dmabuf_imports.queue.deadline() { + TimeoutAction::ToInstant(deadline) + } else { + state.dmabuf_imports.timer = None; + TimeoutAction::Drop + } + }) { + Ok(token) => self.dmabuf_imports.timer = Some(token), + Err(error) => { + tracing::warn!(%error, "could not schedule DMA-BUF import deadline"); + self.dmabuf_imports + .queue + .remove_if(|_| true, |entry| entry.value.notifier.failed()); + } + } + } + } else if let Some(token) = self.dmabuf_imports.timer.take() { + self.loop_handle.remove(token); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::os::fd::AsRawFd; + + #[test] + fn renderer_order_cannot_consume_another_globals_requests() { + for order in [[1, 2], [2, 1]] { + let now = Instant::now(); + let mut queue = Queue::default(); + queue.enqueue(2, 1, "B-only", now).unwrap(); + let mut results = Vec::new(); + for owner in order { + queue.process(&owner, |buffer| { + assert_eq!(owner, 2); + assert_eq!(buffer, "B-only"); + results.push(true); + }); + } + + assert_eq!(results, [true]); + assert!(queue.pending.is_empty()); + } + } + + #[test] + fn unavailable_imports_are_bounded_and_recover_before_the_deadline() { + let now = Instant::now(); + let mut queue = Queue::default(); + for index in 0..MAX_PENDING { + queue.enqueue(1, index / MAX_PER_CLIENT, index, now).unwrap(); + } + + assert_eq!(queue.enqueue(1, 99, 100, now), Err(100)); + let mut completed = Vec::new(); + queue.remove_if( + |entry| entry.deadline <= now + MAX_WAIT / 2, + |_| panic!("expired early"), + ); + queue.process(&1, |value| completed.push(value)); + assert_eq!(completed, (0..MAX_PENDING).collect::>()); + assert!(queue.deadline().is_none()); + } + + #[test] + fn per_client_admission_leaves_room_for_other_clients() { + let now = Instant::now(); + let mut queue = Queue::default(); + for index in 0..MAX_PER_CLIENT { + queue.enqueue(1, 1, index, now).unwrap(); + } + + assert_eq!(queue.enqueue(2, 1, 100, now), Err(100)); + queue.enqueue(2, 2, 101, now).unwrap(); + } + + #[test] + fn expiry_and_device_removal_release_resources_once_without_touching_other_owners() { + if !crate::startup_tests::private_runtime( + "dmabuf_imports::tests::expiry_and_device_removal_release_resources_once_without_touching_other_owners", + ) { + return; + } + + let now = Instant::now(); + let mut queue = Queue::default(); + let removed = tempfile::tempfile().unwrap(); + let expired = tempfile::tempfile().unwrap(); + let fd_removed = removed.as_raw_fd(); + let fd_expired = expired.as_raw_fd(); + queue.enqueue(1, 1, removed, now).unwrap(); + queue.enqueue(2, 1, expired, now + Duration::from_secs(1)).unwrap(); + let mut failures = 0; + queue.process(&1, |_| failures += 1); + // SAFETY: F_GETFD observes descriptor validity without modifying it. + assert_eq!(unsafe { libc::fcntl(fd_removed, libc::F_GETFD) }, -1); + assert_ne!(unsafe { libc::fcntl(fd_expired, libc::F_GETFD) }, -1); + queue.remove_if(|entry| entry.deadline <= now + MAX_WAIT, |_| panic!("expired early")); + queue.remove_if( + |entry| entry.deadline <= now + MAX_WAIT + Duration::from_secs(1), + |_| failures += 1, + ); + assert_eq!(unsafe { libc::fcntl(fd_expired, libc::F_GETFD) }, -1); + assert_eq!(failures, 2); + assert!(queue.deadline().is_none()); + } +} diff --git a/compositor/src/handlers/compositor.rs b/compositor/src/handlers/compositor.rs index 2dbab21..a5ae9cc 100644 --- a/compositor/src/handlers/compositor.rs +++ b/compositor/src/handlers/compositor.rs @@ -112,9 +112,9 @@ impl DmabufHandler for Ferese { &mut self.dmabuf_state } - fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) { - self.queue_dmabuf_import(dmabuf, notifier); - crate::backends::direct::render_all(self); + fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) { + self.dmabuf_imports + .enqueue(*global, dmabuf, notifier, std::time::Instant::now()); } } diff --git a/compositor/src/handlers/mod.rs b/compositor/src/handlers/mod.rs index 7565e69..9a39221 100644 --- a/compositor/src/handlers/mod.rs +++ b/compositor/src/handlers/mod.rs @@ -46,11 +46,17 @@ impl SeatHandler for Ferese { } fn cursor_image(&mut self, _seat: &Seat, image: CursorImageStatus) { - if let CursorImageStatus::Named(icon) = &image { - self.named_cursors - .entry(*icon) - .or_insert_with(|| crate::cursor::load_named_cursor(&self.cursor_theme, *icon)); + if let CursorImageStatus::Named(icon) = &image + && !self.named_cursors.contains_key(icon) + { + self.cursor_loader.request(*icon, self.cursor_animation.buffer_scale); } + self.cursor_animation.reset |= match (&self.cursor_status, &image) { + (CursorImageStatus::Named(previous), CursorImageStatus::Named(next)) => previous != next, + (CursorImageStatus::Hidden, CursorImageStatus::Hidden) => false, + (CursorImageStatus::Surface(previous), CursorImageStatus::Surface(next)) => previous != next, + _ => true, + }; self.cursor_status = image; self.update_capture_cursor_privacy(); // Pointer focus transitions invoke this with the pointer lock held. diff --git a/compositor/src/main.rs b/compositor/src/main.rs index 3d717fd..0b5d80f 100644 --- a/compositor/src/main.rs +++ b/compositor/src/main.rs @@ -4,6 +4,7 @@ mod cursor; mod daemon; mod dimming; mod display_presentation; +mod dmabuf_imports; mod effects; mod floating; mod frame_scheduler; @@ -161,6 +162,8 @@ fn after_dispatch(state: &mut Ferese) { } fn after_dispatch_with_redraw(state: &mut Ferese, mut redraw: impl FnMut(&mut Ferese, RedrawRequest<'_>)) { + state.process_pending_dmabuf_imports(); + state.update_named_cursor(std::time::Instant::now()); state.poll_theme(); // The decoder wakes the loop after publishing its result, including // when no output has a pending frame. diff --git a/compositor/src/render/scene.rs b/compositor/src/render/scene.rs index 338fa5b..0a92164 100644 --- a/compositor/src/render/scene.rs +++ b/compositor/src/render/scene.rs @@ -487,15 +487,14 @@ pub(super) fn cursor_elements( }) .collect() } - CursorImageStatus::Named(icon) => { - let Some(cursor) = state.named_cursors.get(icon) else { + CursorImageStatus::Named(_) => { + let Some(cursor) = state.named_cursor_frame() else { return Vec::new(); }; - let hotspot = Point::::from((cursor.hotspot.x, cursor.hotspot.y)); - let physical_location = pointer_location.to_physical_precise_round(scale) - hotspot; + let physical_location = cursor.physical_location(pointer_location, scale); let Ok(element) = MemoryRenderBufferRenderElement::from_buffer( renderer, - physical_location.to_f64(), + physical_location, &cursor.buffer, None, None, diff --git a/compositor/src/startup_tests.rs b/compositor/src/startup_tests.rs index 5abb8cf..c6783c5 100644 --- a/compositor/src/startup_tests.rs +++ b/compositor/src/startup_tests.rs @@ -27,7 +27,7 @@ pub(crate) fn private_runtime(test: &str) -> bool { let log_path = directory.path().join("test.log"); let log = std::fs::File::create(&log_path).unwrap(); let mut child = Command::new(std::env::current_exe().unwrap()) - .args(["--exact", test, "--nocapture"]) + .args(["--exact", test, "--include-ignored", "--nocapture"]) .env(CHILD, test) .env("XDG_RUNTIME_DIR", directory.path()) .env("XDG_CONFIG_HOME", directory.path()) diff --git a/compositor/src/state.rs b/compositor/src/state.rs index b7e6ca9..20f10c6 100644 --- a/compositor/src/state.rs +++ b/compositor/src/state.rs @@ -33,10 +33,8 @@ use ferese_layout::{ Axis, ColumnWidth, Direction, GapConfig, LayoutResult, Rect, SizeConstraints, ViewportFocusStrategy, WindowId, }; use ferese_protocols::shell::v1::server::ferese_shell_v1::FereseShellV1; -use smithay::backend::allocator::dmabuf::Dmabuf; use smithay::backend::drm::{DrmEventTime, DrmNode}; use smithay::backend::input::Keycode; -use smithay::backend::renderer::ImportDma; use smithay::desktop::space::SpaceElement; use smithay::desktop::utils::send_frames_surface_tree; use smithay::desktop::{LayerSurface, PopupKind, PopupManager, Space, Window, WindowSurfaceType, layer_map_for_output}; @@ -55,7 +53,7 @@ use smithay::utils::{Logical, Point, Rectangle, Size}; use smithay::wayland::alpha_modifier::AlphaModifierState; use smithay::wayland::compositor::{CompositorClientState, CompositorState, with_states}; use smithay::wayland::cursor_shape::CursorShapeManagerState; -use smithay::wayland::dmabuf::{DmabufState, ImportNotifier}; +use smithay::wayland::dmabuf::DmabufState; use smithay::wayland::fractional_scale::FractionalScaleManagerState; use smithay::wayland::idle_inhibit::IdleInhibitManagerState; use smithay::wayland::idle_notify::IdleNotifierState; @@ -84,7 +82,7 @@ use window_registry::WindowRegistry; use crate::backends::direct::DirectBackendState; use crate::config::{Binding, BindingSet, DaemonConfig, InputSettings, OutputProfile, ThemeSettings}; -use crate::cursor::{NamedCursor, cursor_theme, load_named_cursor}; +use crate::cursor::NamedCursor; use crate::dimming::DimAnimation; use crate::gestures::{Swipe, SwipeDirection}; use crate::handlers::screencopy::PendingScreencopy; @@ -468,7 +466,8 @@ pub struct Ferese { pub(crate) output_redraw_pending: Vec, pub(crate) locked_pointer_hint: Option, pub(crate) last_pointer_time: u32, - pub(crate) cursor_theme: xcursor::CursorTheme, + pub(crate) cursor_loader: crate::cursor::Loader, + pub(crate) cursor_animation: crate::cursor::Animation, pub(crate) named_cursors: HashMap, pub intercepted_keys: HashSet, pub(crate) swipe: Swipe, @@ -479,7 +478,7 @@ pub struct Ferese { pub active_shortcuts_inhibitor: Option, pub direct_backend: Option, _ipc_socket: Option, - pending_dmabuf_imports: Vec<(Dmabuf, ImportNotifier)>, + pub(crate) dmabuf_imports: crate::dmabuf_imports::ImportManager, pub(crate) activation_inputs: crate::handlers::activation::InputHistory, pub(crate) pending_screencopies: Vec, // Screenshot requests outlive the readback that filled them: the @@ -630,8 +629,8 @@ impl Ferese { let socket_name = Self::init_wayland_listener(display, event_loop)?; let start_time = Instant::now(); - let cursor_theme = cursor_theme(); - let default_cursor = load_named_cursor(&cursor_theme, CursorIcon::Default); + let cursor_loader = crate::cursor::Loader::new(event_loop.get_signal())?; + let default_cursor = crate::cursor::fallback_cursor(); let named_cursors = HashMap::from([(CursorIcon::Default, default_cursor)]); let mut state = Self { @@ -712,7 +711,8 @@ impl Ferese { output_redraw_pending: Vec::new(), locked_pointer_hint: None, last_pointer_time: 0, - cursor_theme, + cursor_loader, + cursor_animation: crate::cursor::Animation::default(), named_cursors, intercepted_keys: HashSet::new(), swipe: Swipe::default(), @@ -723,7 +723,7 @@ impl Ferese { active_shortcuts_inhibitor: None, direct_backend: None, _ipc_socket: None, - pending_dmabuf_imports: Vec::new(), + dmabuf_imports: crate::dmabuf_imports::ImportManager::default(), activation_inputs: Default::default(), pending_screencopies: Vec::new(), screenshot: Coordinator::new(), @@ -809,10 +809,6 @@ impl Ferese { if input_changed && let Some(keyboard) = self.seat.get_keyboard() { if keyboard_changed { - self.input_capture.close_all(); - if self.input_capture.restore_focus { - self.restore_input_capture_focus(); - } let options = (!config.input_settings.xkb_options.is_empty()) .then(|| config.input_settings.xkb_options.join(",")); @@ -827,6 +823,11 @@ impl Ferese { }, ) .map_err(|e| format!("keymap reload failed: {e}"))?; + + self.input_capture.close_all(); + if self.input_capture.restore_focus { + self.restore_input_capture_focus(); + } } keyboard.change_repeat_info(config.input_settings.repeat_rate, config.input_settings.repeat_delay_ms); } @@ -931,29 +932,6 @@ impl Ferese { Ok(()) } - pub(crate) fn queue_dmabuf_import(&mut self, dmabuf: Dmabuf, notifier: ImportNotifier) { - self.pending_dmabuf_imports.push((dmabuf, notifier)); - } - - pub(crate) fn process_dmabuf_imports(&mut self, renderer: &mut R, node: Option) - where - R: ImportDma, - { - for (dmabuf, notifier) in self.pending_dmabuf_imports.drain(..) { - if renderer.import_dmabuf(&dmabuf, None).is_ok() { - if let Some(node) = node { - dmabuf.set_node(node); - } - - if let Err(error) = notifier.successful::() { - tracing::debug!(?error, "dma-buf client disappeared before import completed"); - } - } else { - notifier.failed(); - } - } - } - fn init_wayland_listener( display: Display, event_loop: &mut EventLoop<'static, Self>, diff --git a/compositor/src/state/window_lifecycle_tests.rs b/compositor/src/state/window_lifecycle_tests.rs index b53e0dc..0a14714 100644 --- a/compositor/src/state/window_lifecycle_tests.rs +++ b/compositor/src/state/window_lifecycle_tests.rs @@ -31,6 +31,190 @@ fn fixture() -> (smithay::reexports::calloop::EventLoop<'static, Ferese>, Ferese (events, state, output) } +#[test] +fn late_keymap_failure_preserves_capture_sessions_and_runtime_settings() { + if !crate::startup_tests::private_runtime( + "state::window_lifecycle_tests::late_keymap_failure_preserves_capture_sessions_and_runtime_settings", + ) { + return; + } + + let (mut events, mut state, _) = fixture(); + let (window, _wire) = window(&mut state, &mut events, 0xff112233); + let focus = state.focused_window; + let session = state.input_capture.register(42, 1).unwrap(); + state.input_capture.enable(session).unwrap(); + let previous_layout = state.input_settings.xkb_layout.clone(); + let previous_repeat = state.input_settings.repeat_rate; + let mut runtime = crate::config::Config::default().runtime_config().unwrap(); + // Preparation succeeded; force the real late application failure. + runtime.input_settings.xkb_layout = "ferese-nonexistent-layout-for-regression".into(); + runtime.input_settings.repeat_rate = previous_repeat + 1; + let result = state.apply_runtime_config(runtime, &serde_json::json!({"input": "changed"})); + assert!(result.unwrap_err().contains("keymap reload failed")); + assert!(state.input_capture.has_sessions()); + state.input_capture.enable(session).unwrap(); + assert!(!state.input_capture.restore_focus); + assert_eq!(state.focused_window, focus); + assert_eq!(state.input_settings.xkb_layout, previous_layout); + assert_eq!(state.input_settings.repeat_rate, previous_repeat); + assert!(state.windows.ids().contains_key(&window)); +} + +#[test] +fn dmabuf_validation_uses_the_originating_global_without_a_desktop_frame() { + use smithay::backend::allocator::{Buffer, Format, Fourcc, Modifier}; + use std::io::Read; + use std::os::fd::AsRawFd; + use std::os::unix::net::UnixStream; + use std::sync::Arc; + + if !crate::startup_tests::private_runtime( + "state::window_lifecycle_tests::dmabuf_validation_uses_the_originating_global_without_a_desktop_frame", + ) { + return; + } + + let mut events = smithay::reexports::calloop::EventLoop::try_new().unwrap(); + let mut state = crate::startup_tests::state(&mut events); + let formats = [Format { + code: Fourcc::Argb8888, + modifier: Modifier::Linear, + }]; + let a = state + .dmabuf_state + .create_global::(&state.display_handle, formats); + state.dmabuf_imports.register(a); + let (server, mut wire) = UnixStream::pair().unwrap(); + let client = state + .display_handle + .insert_client(server, Arc::new(ClientState::default())) + .unwrap(); + request(&mut wire, 1, 1, &[2], None); + dispatch(&mut events, &mut state); + let read = |wire: &mut UnixStream| { + wire.set_nonblocking(true).unwrap(); + let mut bytes = Vec::new(); + let _ = wire.read_to_end(&mut bytes); + wire.set_nonblocking(false).unwrap(); + bytes + }; + let mut registry = read(&mut wire); + // Register B after the initial registry roundtrip so notification order + // identifies A and B without depending on backend map iteration order. + let b = state + .dmabuf_state + .create_global::(&state.display_handle, formats); + state.dmabuf_imports.register(b); + state.display_handle.flush_clients().unwrap(); + registry.extend(read(&mut wire)); + let mut offset = 0; + let mut names = Vec::new(); + while offset < registry.len() { + let header = u32::from_ne_bytes(registry[offset + 4..offset + 8].try_into().unwrap()); + let length = (header >> 16) as usize; + let message = ®istry[offset..offset + length]; + if message + .windows(b"zwp_linux_dmabuf_v1\0".len()) + .any(|text| text == b"zwp_linux_dmabuf_v1\0") + { + names.push(u32::from_ne_bytes(message[8..12].try_into().unwrap())); + } + offset += length; + } + assert_eq!(names.len(), 2); + let interface = b"zwp_linux_dmabuf_v1\0"; + for (name, id) in names.into_iter().zip([3, 4]) { + let mut args = vec![name, interface.len() as u32]; + let mut padded = interface.to_vec(); + padded.resize(padded.len().next_multiple_of(4), 0); + args.extend( + padded + .chunks_exact(4) + .map(|word| u32::from_ne_bytes(word.try_into().unwrap())), + ); + args.extend([3, id]); + request(&mut wire, 2, 0, &args, None); + } + dispatch(&mut events, &mut state); + read(&mut wire); + let file = tempfile::tempfile().unwrap(); + file.set_len(64).unwrap(); + // Both asynchronous and immediate imports must survive the wrong GPU + // being visited first. The mock capability accepts only B's buffers. + for (params, buffer) in [(5, None), (6, Some(7))] { + request(&mut wire, 4, 1, &[params], None); + request(&mut wire, params, 1, &[0, 0, 16, 0, 0], Some(file.as_raw_fd())); + let mut args = vec![4, 4, Fourcc::Argb8888 as u32, 0]; + if let Some(buffer) = buffer { + args.insert(0, buffer); + } + request(&mut wire, params, if buffer.is_some() { 3 } else { 2 }, &args, None); + dispatch(&mut events, &mut state); + assert!(state.space.outputs().next().is_none()); + state.process_pending_dmabuf_imports(); + assert!( + read(&mut wire).is_empty(), + "unavailable renderer must defer notification" + ); + state + .dmabuf_imports + .process_with(a, None, |_| panic!("B's request reached A's renderer")); + let mut calls = 0; + state.dmabuf_imports.process_with(b, None, |dmabuf| { + calls += 1; + assert_eq!(dmabuf.size(), (4, 4).into()); + true + }); + assert_eq!(calls, 1); + state.display_handle.flush_clients().unwrap(); + let response = read(&mut wire); + if buffer.is_none() { + assert_eq!(u32::from_ne_bytes(response[..4].try_into().unwrap()), params); + assert_eq!(u32::from_ne_bytes(response[4..8].try_into().unwrap()) & 0xffff, 0); + } else { + assert!(response.is_empty(), "create_immed success must not emit a failed event"); + } + assert!(client.get_credentials(&state.display_handle).is_ok()); + } + + // Deferred requests receive one terminal failure on timeout or removal. + for params in [8, 9] { + request(&mut wire, 4, 1, &[params], None); + request(&mut wire, params, 1, &[0, 0, 16, 0, 0], Some(file.as_raw_fd())); + request(&mut wire, params, 2, &[4, 4, Fourcc::Argb8888 as u32, 0], None); + dispatch(&mut events, &mut state); + state.process_pending_dmabuf_imports(); + if params == 8 { + state.dmabuf_imports.expire(Instant::now() + Duration::from_secs(5)); + } else { + state.dmabuf_imports.remove(b); + } + + state.display_handle.flush_clients().unwrap(); + let response = read(&mut wire); + assert_eq!(response.len(), 8); + assert_eq!(u32::from_ne_bytes(response[..4].try_into().unwrap()), params); + assert_eq!(u32::from_ne_bytes(response[4..8].try_into().unwrap()) & 0xffff, 1); + state + .dmabuf_imports + .process_with(b, None, |_| panic!("failed request survived")); + assert!(client.get_credentials(&state.display_handle).is_ok()); + } + + state.dmabuf_imports.register(b); + request(&mut wire, 4, 1, &[10], None); + request(&mut wire, 10, 1, &[0, 0, 16, 0, 0], Some(file.as_raw_fd())); + request(&mut wire, 10, 2, &[4, 4, Fourcc::Argb8888 as u32, 0], None); + dispatch(&mut events, &mut state); + drop(wire); + dispatch(&mut events, &mut state); + state.process_pending_dmabuf_imports(); + state + .dmabuf_imports + .process_with(b, None, |_| panic!("disconnected client's request survived")); +} + #[test] fn hidden_null_commit_removes_membership_and_remaps_without_workspace_switch() { if !crate::startup_tests::private_runtime( diff --git a/compositor/src/winit.rs b/compositor/src/winit.rs index cf70869..83bf3d8 100644 --- a/compositor/src/winit.rs +++ b/compositor/src/winit.rs @@ -12,9 +12,7 @@ use smithay::desktop::layer_map_for_output; use smithay::output::{Mode, Output, PhysicalProperties, Scale, Subpixel}; use smithay::reexports::calloop::EventLoop; use smithay::reexports::calloop::timer::{TimeoutAction, Timer}; -use smithay::reexports::wayland_protocols::wp::presentation_time::server::wp_presentation_feedback::Kind as PresentationKind; -use smithay::utils::{Clock, Monotonic, Physical, Rectangle, Transform}; -use smithay::wayland::presentation::Refresh; +use smithay::utils::{Physical, Rectangle, Size, Transform}; use crate::Ferese; use crate::metrics::{FrameEffectMetrics, RenderMetrics}; @@ -32,14 +30,18 @@ type DamageRenderResult = Result< pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<(), Box> { let (mut backend, event_source) = winit::init::()?; + configure_nested_protocols(state); + let initial_size = backend.window_size(); + let drawable = Rc::new(Cell::new(usable_size(initial_size))); let dmabuf_formats = backend.renderer().dmabuf_formats(); let display_handle = state.display_handle.clone(); - state + let dmabuf_global = state .dmabuf_state .create_global::(&display_handle, dmabuf_formats); + state.dmabuf_imports.register(dmabuf_global); let initial_scale = normalized_scale(backend.scale_factor()); let mode = Mode { - size: backend.window_size(), + size: if drawable.get() { initial_size } else { (1, 1).into() }, refresh: backend .window() .current_monitor() @@ -67,8 +69,6 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() state.register_output(&output, "nested-primary".to_owned()); let mut damage_tracker = OutputDamageTracker::from_output(&output); - let clock = Clock::::new(); - let mut sequence = 0_u64; let mut missed_deadlines = 0_u64; let mut render_metrics = RenderMetrics::from_environment(output.name()); @@ -82,11 +82,16 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() state.nested_backend = Some(backend.clone()); let redraw_backend = backend.clone(); let redraw_refresh = refresh.clone(); + let redraw_drawable = drawable.clone(); event_loop .handle() .insert_source(Timer::from_duration(refresh.get()), move |_, _, _| { - redraw_backend.borrow().window().request_redraw(); - TimeoutAction::ToDuration(redraw_refresh.get()) + if redraw_drawable.get() { + redraw_backend.borrow().window().request_redraw(); + TimeoutAction::ToDuration(redraw_refresh.get()) + } else { + TimeoutAction::ToDuration(Duration::from_millis(250)) + } })?; let mut capture_texture = None; @@ -97,21 +102,16 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() match event { WinitEvent::Resized { size, scale_factor } => { tracing::debug!(target: "ferese::nested_input", ?size, scale_factor, "host resized nested output"); - let scale = normalized_scale(scale_factor); let rate = backend .window() .current_monitor() .and_then(|monitor| monitor.refresh_rate_millihertz()) .unwrap_or(60_000) as i32; - refresh.set(Duration::from_nanos(1_000_000_000_000 / rate.max(1) as u64)); + if !resize_nested_output(state, &output, &drawable, size, scale_factor, rate) { + return; + } - let mut runtime = state.current_desktop_outputs(); - let resized = runtime.iter_mut().find(|entry| entry.output == output).unwrap(); - resized.mode = Mode { size, refresh: rate }; - resized.scale = Scale::Fractional(scale); - state.begin_desktop_transition(); - let changes = state.publish_desktop(runtime).expect("valid nested output geometry"); - state.finish_desktop_transition(changes); + refresh.set(Duration::from_nanos(1_000_000_000_000 / rate.max(1) as u64)); if let Err(error) = state.display_handle.flush_clients() { tracing::debug!(%error, "failed to flush output-resize configure"); } @@ -125,13 +125,17 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() "host input in nested output"); } WinitEvent::Redraw => { + if !drawable.get() || !usable_size(backend.window_size()) { + drawable.set(false); + return; + } + state.advance_animations(Instant::now()); let age = backend.buffer_age().unwrap_or(0); let render_started = Instant::now(); let rendered = (|| -> DamageRenderResult { { let (renderer, mut framebuffer) = backend.bind()?; - state.process_dmabuf_imports(renderer, None); let elements = animated_window_elements(state, renderer, &output); let effects = frame_effect_metrics(&elements, output.current_scale().fractional_scale()); let result = damage_tracker.render_output( @@ -143,31 +147,41 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() )?; // Keep the existing readback-only path when the // displayed scene has no privacy exclusions. - let mut capture_changed_binding = !state.has_capture_exclusions() - && state.process_screencopies(renderer, &framebuffer, &output, true); - if state.has_pending_screencopy(&output, false) - || state.has_pending_screencopy(&output, true) - { - let scene = state.sample_frame(&output, Duration::ZERO); - crate::backends::direct::capture::capture_output( - state, - renderer, - &mut capture_texture, - &output, - &scene, - ); - capture_changed_binding = true; - } - if capture_changed_binding { - // Restore the display framebuffer binding after offscreen readback. - let _ = renderer - .render( - &mut framebuffer, - output.current_mode().expect("output has a mode").size, - output.current_transform(), - )? - .finish()?; - } + capture_with_display_restore( + renderer, + &mut framebuffer, + |renderer, framebuffer| { + let mut changed = !state.has_capture_exclusions() + && state.process_screencopies(renderer, framebuffer, &output, true); + if state.has_pending_screencopy(&output, false) + || state.has_pending_screencopy(&output, true) + { + let scene = state.sample_frame(&output, Duration::ZERO); + crate::backends::direct::capture::capture_output( + state, + renderer, + &mut capture_texture, + &output, + &scene, + ); + changed = true; + } + + changed + }, + |renderer, framebuffer| { + // Capture failures are handled locally. Failure to restore + // the display target means the renderer itself is unusable. + let _ = renderer + .render( + framebuffer, + output.current_mode().unwrap().size, + output.current_transform(), + )? + .finish()?; + Ok::<_, smithay::backend::renderer::gles::GlesError>(()) + }, + )?; Ok((result.damage.cloned(), effects, result.states)) } @@ -212,19 +226,6 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() } render_metrics.record_frame(elapsed, &damage, missed_deadlines, effects); - let mut presentation = crate::presentation::take_output_feedback( - state, - &output, - &rendered_states, - PresentationKind::Vsync, - ); - sequence = sequence.wrapping_add(1); - presentation.presented( - clock.now(), - Refresh::fixed(refresh.get()), - sequence, - PresentationKind::Vsync, - ); send_nested_frame_callbacks(state, &output); state.space.refresh(); state.popups.cleanup(); @@ -240,6 +241,74 @@ pub fn init(event_loop: &mut EventLoop, state: &mut Ferese) -> Result<() Ok(()) } +fn configure_nested_protocols(state: &mut Ferese) { + // Winit's swap has no host presentation timestamp or retrace counter. + // Do not advertise timing we cannot report accurately. + state + .display_handle + .disable_global::(state.presentation_state.global()); +} + +fn usable_size(size: Size) -> bool { + size.w > 0 && size.h > 0 +} + +fn resize_nested_output( + state: &mut Ferese, + output: &Output, + drawable: &Cell, + size: Size, + scale_factor: f64, + rate: i32, +) -> bool { + if !usable_size(size) { + drawable.set(false); + return false; + } + + let mut runtime = state.current_desktop_outputs(); + let Some(resized) = runtime.iter_mut().find(|entry| entry.output == *output) else { + drawable.set(false); + return false; + }; + resized.mode = Mode { + size, + refresh: rate.max(1), + }; + resized.scale = Scale::Fractional(normalized_scale(scale_factor)); + // Paused host time must not advance springs when publication relayouts. + if !drawable.get() { + state.reset_animation_clock(); + } + + state.begin_desktop_transition(); + let changes = match state.publish_desktop(runtime) { + Ok(changes) => changes, + Err(error) => { + state.desktop_transition = None; + drawable.set(false); + tracing::warn!(%error, "ignoring invalid nested output geometry"); + return false; + } + }; + state.finish_desktop_transition(changes); + drawable.set(true); + true +} + +fn capture_with_display_restore( + renderer: &mut R, + framebuffer: &mut F, + capture: impl FnOnce(&mut R, &mut F) -> bool, + restore: impl FnOnce(&mut R, &mut F) -> Result<(), E>, +) -> Result<(), E> { + if capture(renderer, framebuffer) { + restore(renderer, framebuffer)?; + } + + Ok(()) +} + fn send_nested_frame_callbacks(state: &mut Ferese, output: &Output) { let eligible = state.callback_outputs(); if state.session_lock.active() { @@ -268,7 +337,196 @@ fn normalized_scale(scale: f64) -> f64 { #[cfg(test)] mod tests { - use super::normalized_scale; + use super::*; + + #[test] + fn zero_host_resize_preserves_geometry_and_recovers() { + if !crate::startup_tests::private_runtime("winit::tests::zero_host_resize_preserves_geometry_and_recovers") { + return; + } + + let mut events = EventLoop::try_new().unwrap(); + let mut state = crate::startup_tests::state(&mut events); + let output = Output::new( + "nested".into(), + PhysicalProperties { + size: (0, 0).into(), + subpixel: Subpixel::Unknown, + make: "test".into(), + model: "test".into(), + }, + ); + output.change_current_state( + Some(Mode { + size: (800, 600).into(), + refresh: 60_000, + }), + None, + None, + None, + ); + state.space.map_output(&output, (0, 0)); + state.register_output(&output, "nested-primary".into()); + let drawable = Cell::new(true); + assert!(resize_nested_output( + &mut state, + &output, + &drawable, + (900, 600).into(), + 1.25, + 60_000 + )); + let before = output.current_mode(); + for size in [(0, 600), (900, 0), (0, 0), (-1, 600)] { + assert!(!resize_nested_output( + &mut state, + &output, + &drawable, + size.into(), + 2.0, + 144_000 + )); + assert_eq!(output.current_mode(), before); + assert_eq!(output.current_scale().fractional_scale(), 1.25); + assert!(!drawable.get()); + assert!(state.desktop_transition.is_none()); + } + + assert!(resize_nested_output( + &mut state, + &output, + &drawable, + (1200, 800).into(), + 1.5, + 120_000 + )); + assert_eq!(output.current_mode().unwrap().size, (1200, 800).into()); + assert!(drawable.get()); + assert!(state.desktop_transition.is_none()); + } + + #[test] + fn nested_registry_does_not_advertise_unobserved_presentation_timing() { + use std::io::{Read, Write}; + use std::os::unix::net::UnixStream; + use std::sync::Arc; + if !crate::startup_tests::private_runtime( + "winit::tests::nested_registry_does_not_advertise_unobserved_presentation_timing", + ) { + return; + } + + let mut events = EventLoop::try_new().unwrap(); + let mut state = crate::startup_tests::state(&mut events); + configure_nested_protocols(&mut state); + let (server, mut wire) = UnixStream::pair().unwrap(); + let _client = state + .display_handle + .insert_client(server, Arc::new(crate::state::ClientState::default())) + .unwrap(); + wire.write_all( + &[1u32, 12 << 16 | 1, 2] + .into_iter() + .flat_map(u32::to_ne_bytes) + .collect::>(), + ) + .unwrap(); + events.dispatch(Duration::from_millis(1), &mut state).unwrap(); + state.display_handle.flush_clients().unwrap(); + wire.set_nonblocking(true).unwrap(); + let mut globals = Vec::new(); + let _ = wire.read_to_end(&mut globals); + assert!( + globals + .windows(b"wl_compositor\0".len()) + .any(|bytes| bytes == b"wl_compositor\0") + ); + assert!( + !globals + .windows(b"wp_presentation\0".len()) + .any(|bytes| bytes == b"wp_presentation\0") + ); + } + + #[test] + #[ignore = "requires an EGL device and private Wayland sockets"] + fn capture_failure_restores_display_target_and_subsequent_frames_render() { + use smithay::backend::allocator::Fourcc; + use smithay::backend::egl::{EGLContext, EGLDevice, EGLDisplay}; + use smithay::backend::renderer::gles::{GlesError, GlesTexture}; + use smithay::backend::renderer::{Bind, Color32F, ExportMem, Offscreen}; + use smithay::reexports::calloop::channel; + use smithay::utils::Buffer; + if !crate::startup_tests::private_runtime( + "winit::tests::capture_failure_restores_display_target_and_subsequent_frames_render", + ) { + return; + } + + let mut events = EventLoop::try_new().unwrap(); + let mut state = crate::startup_tests::state(&mut events); + let display = unsafe { EGLDisplay::new(EGLDevice::enumerate().unwrap().last().unwrap()).unwrap() }; + let context = EGLContext::new(&display).unwrap(); + let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; + let output = Output::new( + "nested".into(), + PhysicalProperties { + size: (0, 0).into(), + subpixel: Subpixel::Unknown, + make: "test".into(), + model: "test".into(), + }, + ); + let size: Size = (32, 24).into(); + let mut displayed: GlesTexture = renderer.create_buffer(Fourcc::Abgr8888, (32, 24).into()).unwrap(); + let mut offscreen: GlesTexture = renderer.create_buffer(Fourcc::Abgr8888, (32, 24).into()).unwrap(); + let damage = [Rectangle::from_size(size)]; + let mut framebuffer = renderer.bind(&mut displayed).unwrap(); + for changed_binding in [false, true] { + let (sender, receiver) = channel::channel(); + state + .pending_screencopies + .push(crate::handlers::screencopy::PendingScreencopy::owned( + 1, + 0, + sender, + output.clone(), + Rectangle::::from_size((32, 24).into()), + )); + capture_with_display_restore( + &mut renderer, + &mut framebuffer, + |renderer, _| { + if changed_binding { + let _target = renderer.bind(&mut offscreen).unwrap(); + } + assert!( + crate::backends::direct::capture::capture_request(&mut state, &output, false, || { + Err::<(), _>("injected offscreen allocation/render failure") + }) + .is_none() + ); + true + }, + |renderer, framebuffer| { + let _ = renderer.render(framebuffer, size, Transform::Normal)?.finish()?; + Ok::<_, GlesError>(()) + }, + ) + .unwrap(); + assert!(receiver.try_recv().unwrap().result.is_err()); + assert!(receiver.try_recv().is_err()); + assert!(state.pending_screencopies.is_empty()); + let mut frame = renderer.render(&mut framebuffer, size, Transform::Normal).unwrap(); + frame.clear(Color32F::new(0.0, 0.0, 1.0, 1.0), &damage).unwrap(); + frame.finish().unwrap().wait().unwrap(); + let pixels = renderer + .copy_framebuffer(&framebuffer, Rectangle::from_size((32, 24).into()), Fourcc::Abgr8888) + .unwrap(); + assert_eq!(&renderer.map_texture(&pixels).unwrap()[..4], &[0, 0, 255, 255]); + } + } + #[test] fn accepts_positive_finite_scale() { assert_eq!(normalized_scale(1.25), 1.25); diff --git a/docs/performance-baseline.md b/docs/performance-baseline.md deleted file mode 100644 index ac7c79d..0000000 --- a/docs/performance-baseline.md +++ /dev/null @@ -1,155 +0,0 @@ -# Performance measurements - -Use release builds and a fixed workload when comparing performance. Record the -commit, output size, scale, refresh rate, graphics driver, and whether the session -is nested or direct. Keep the nested preview visible: host occlusion changes -rendering behavior. - -## Collect a sample - -```sh -cargo build --release --locked -p ferese -p ferese-shell -p feresectl -FERESE_TRACE_PERFORMANCE=1 target/release/ferese --backend nested \ - --grant-effects --grant-shell-control -- target/release/ferese-shell -``` - -Measure idle, several windows, overview, Settings navigation, translucent popovers -and screen capture separately. Wait for startup to settle, then repeat each sample. -Record process CPU, child-process CPU, RSS, render attempts and damaged frames. -Do not build or run unrelated workloads during the sample. - -CPU percentages below use one core as 100%. These short samples describe observed -behavior; they are not guarantees or hardware-session benchmarks. - -## Direct frame timing - -The DRM backend schedules each output against its own predicted presentation -time. Render cost and a safety margin determine when compositing starts. Idle -outputs wake on demand; callback-only updates are paced to the output refresh -cycle without submitting unchanged buffers. Animation forecasts are restored -before processing input or rendering another output. - -For a direct session, enable `FERESE_TRACE_PERFORMANCE=1` and -`RUST_LOG=ferese::render=debug` before starting Ferese. The timing records include -the planned render start, actual render start, timer lateness, presentation -target, DRM presentation timestamp, and request-to-presentation duration. -Missed deadlines are counted against submitted frames' presentation targets; -idle gaps do not count. Request-to-presentation timing starts at the compositor's -redraw request. It does not include device input latency, client rendering before -the request or the display's pixel response. Render cost measures CPU preparation -and queueing. Feedback-driven margins allow conservatively for asynchronous GPU work. - -Check sustained animation, idle wakeup, callback-only clients, and mixed-refresh -outputs separately. Nested sessions do not exercise the DRM scheduler. - -## Redraw and animation fallback counters - -With the same environment variables, direct sessions report `redraw requests` -by source file and line, with call counts, requested outputs and connected outputs. -`render performance` adds per-output redraw requests and counts requests made -while a render timer was scheduled or a frame was in flight. Compare these with -`frames`, `no_damage_frames`, render time and missed deadlines; requests can -coalesce and are not themselves rendered frames. - -`animation fallback scheduling` reports watchdog arms, cancellations, wakeups, -recovery requests and animations whose normal frame chain is intact. Ferese emits -summaries every five seconds during redraw activity and on normal exit, resetting -the fallback counters each time. When an animation loses its output's frame chain, -the watchdog deadline is two refresh intervals later. Recovery keeps a requested -final frame even if another output settles the animation while scheduling is -unavailable. Idle, powered-off and disconnected outputs do not set that deadline. -The watchdog is suppressed when a render is already scheduled or a page flip is -pending. It does not replace DRM error recovery or diagnose a permanently lost flip. - -The regression replays use a 60 Hz animation beside an idle 240 Hz output. -The old fallback rule produces 60 unnecessary wakes in one simulated second; -the new rule produces zero while the normal chain remains intact. Three cursor -moves within one output require three output requests rather than the old six. -These counts come from deterministic scheduling replays. They do not measure live -DRM CPU use, latency or battery consumption. Measure those separately with the same -hardware workload before claiming power savings. - -## Settled overview damage - -On 2026-10-02, a release-build comparison against `6f2951c` used four parked -foot terminals in a private nested desktop: 1418 × 1692 pixels, 2× scale, 60 Hz, -Intel Core Ultra 7 258V and Mesa 26.2.3. Ten-second process samples measured -5.9% CPU before and 1.9% after stabilizing caption/thumbnail drawing order and -removing forced full damage from unchanged rounded thumbnails. - -A settled five-second interval changed from 300 damaged frames to zero. -The nested backend still makes about 300 no-damage render attempts per interval. -The change skips unchanged submissions; it does not alter nested scheduling. -These short samples exclude host compositor CPU and do not measure battery -discharge or direct DRM performance. - -The opt-in regression checks a full settled interval, a real client content -commit, and settling again. It fails against the original release. - -```sh -FERESE_TEST_OVERVIEW_DAMAGE=1 FERESE_TEST_BINARY=target/release/ferese \ - FERESE_TEST_CTL=target/release/feresectl \ - python3 scripts/tests/test_overview_damage_isolated.py -``` - -## Idle shell audio polling - -On 2026-10-02, the installed release shell was compared with the audio-event -cache in private nested desktops on the same machine. Each release had ten -seconds to settle, followed by three twenty-second samples without `perf`. -CPU includes waited child processes; one core is 100%. - -| Measurement | Periodic audio queries | Persistent audio watcher | -| --- | --- | --- | -| Combined CPU, sample range | 2.25–3.15% | 0.90% | -| Combined CPU, median | 3.10% | 0.90% | -| Shell CPU, median | 0.75% | 0.75% | -| Child-process CPU, median | 2.35% | 0.15% | -| Shell RSS | 32.3–32.4 MiB | 34.9 MiB | - -A separate twenty-second `perf` sample recorded twenty `wpctl` executions before -and zero afterward. The new connection observes sink properties, device routes, -and default-sink metadata. Relevant events and control actions invalidate the -cached reading; the existing `wpctl` reader preserves volume and device semantics. -Disconnected subscriptions retain bounded polling, and missing sinks retry. -Brightness and other status polling remain unchanged. - -These short nested samples exclude the host compositor and do not measure battery -discharge. Raw measurements and profiles were saved locally in -`/tmp/ferese-shell-profile-20261002`. - -The subscription regression starts its own PipeWire server, changes default -metadata, mute and volume, removes a sink, restarts the server, and checks shutdown. -It does not change the desktop's audio server. - -```sh -cargo test --release --locked -p ferese-shell native_subscription -- --ignored -``` - -## Historical results - -On 2026-09-27 at `08a5866`, two nested runs used a 1422 × 1696 output at 2× scale, -reporting 60 Hz on a 120 Hz host. Samples lasted 5–10 seconds. - -| Scenario | Compositor CPU, two runs | Shell CPU, two runs | -| --- | --- | --- | -| Idle, no application window | 5.3% / 5.8% | 1.5% / 1.6% | -| Overview with one terminal | 18.6% / 16.2% | 1.3% / 1.4% | -| After transitions | 6.6% / 6.4% | 1.5% / 1.4% | - -Compositor RSS increased from about 81–82 MiB to 91–92 MiB during the exercise. -These samples do not establish a leak or long-term stability. A follow-up had -lower rendering rates, so the original observations did not prove duplicate scheduling. - -A 20-second status-worker comparison removed five commands per poll (15 to 10). -Total measured CPU time including child processes fell from 2.41 to 1.90 seconds. -The timing wrapper was included, and this was one comparison. - -For push status delivery (`4bf0757` to `b0583eb`), a synthetic transport test -measured median delivery delay of 129.398 ms before and 0.130 ms after. It excluded -queries, UI dispatch, and painting. Visible shell samples suggested lower typical -CPU, but an after-run spike raised the mean; they did not establish a robust -average CPU saving. - -These results predate later changes. Run fresh comparisons for current claims, -and validate direct rendering and multiple monitors separately. diff --git a/docs/portals.md b/docs/portals.md index 65e0de5..cad41d6 100644 --- a/docs/portals.md +++ b/docs/portals.md @@ -42,8 +42,11 @@ remembered InputCapture grants are not supported. InputCapture forwards local input; it does not inject remote input. Wallpaper approval copies the image into Ferese's data directory and updates the -config. Set `theme.background.lock-path` for a separate lock-screen image; otherwise -it follows the desktop wallpaper. +active light or dark appearance in the config. The other appearance keeps its +wallpaper. A desktop-only change keeps the current lock-screen image, and a +lock-screen-only change keeps the desktop image. Set `theme.background.lock-path` +for a shared lock-screen image, or override it under `theme.light.background` and +`theme.dark.background`. Without a lock-screen path, it follows the desktop wallpaper. Global shortcuts cannot replace Ferese bindings or reserved escape/console keys. They stop when the app session closes. They never activate on the lock screen. diff --git a/portal/src/backend.rs b/portal/src/backend.rs index e3d4ad7..052eb3f 100644 --- a/portal/src/backend.rs +++ b/portal/src/backend.rs @@ -8,7 +8,7 @@ use std::time::Duration; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::process::{Child, Command}; -use tokio::sync::{Mutex, Notify}; +use tokio::sync::{Mutex, Notify, OwnedSemaphorePermit, Semaphore}; use zbus::Connection; use zbus::message::Header; use zbus::object_server::SignalEmitter; @@ -22,6 +22,7 @@ pub(crate) type Options = HashMap; pub(crate) static CONFIG_TRANSACTION: std::sync::Mutex<()> = std::sync::Mutex::new(()); const FRONTEND: &str = "org.freedesktop.portal.Desktop"; const PATH: &str = "/org/freedesktop/portal/desktop"; +const MAX_SESSIONS: usize = 8; #[derive(Debug)] enum StartError { @@ -82,11 +83,22 @@ struct Session { persist_mode: u32, restore: Option, cancel: Arc, + work: Arc, } -#[derive(Clone, Default)] +#[derive(Clone)] pub struct Backend { sessions: Arc>>, + work: Arc, +} + +impl Default for Backend { + fn default() -> Self { + Self { + sessions: Arc::default(), + work: Arc::new(Semaphore::new(MAX_SESSIONS)), + } + } } // This private interface only transfers stop controls to the shell's own recorder. @@ -252,11 +264,17 @@ impl Backend { return Err(error("Invalid session")); } let mut sessions = self.sessions.lock().await; - if sessions.len() >= 8 || sessions.contains_key(session_handle.as_str()) { + if sessions.len() >= MAX_SESSIONS || sessions.contains_key(session_handle.as_str()) { return Err(zbus::fdo::Error::LimitsExceeded( "Session limit reached or duplicate session".into(), )); } + let work = Arc::new( + self.work + .clone() + .try_acquire_owned() + .map_err(|_| zbus::fdo::Error::LimitsExceeded("Capture work is still shutting down".into()))?, + ); let inserted = connection .object_server() .at( @@ -284,6 +302,7 @@ impl Backend { persist_mode: 0, restore: None, cancel: Arc::default(), + work, }, ); Ok((0, Options::new())) @@ -347,7 +366,7 @@ impl Backend { return Err(error("Invalid request path")); } - let (multiple, cursor, source_types, bar_controlled, persist_mode, restore, cancel) = { + let (multiple, cursor, source_types, bar_controlled, persist_mode, restore, cancel, work) = { let mut sessions = self.sessions.lock().await; let session = sessions .get_mut(session_handle.as_str()) @@ -366,6 +385,7 @@ impl Backend { session.persist_mode, session.restore.clone(), session.cancel.clone(), + session.work.clone(), ) }; let inserted = connection @@ -400,6 +420,8 @@ impl Backend { requested_persistence: persist_mode, restore, bar_controlled, + cancel: cancel.clone(), + work, }, ); let result = tokio::select! { @@ -543,6 +565,39 @@ struct StreamOptions { requested_persistence: u32, restore: Option, bar_controlled: bool, + cancel: Arc, + work: Arc, +} + +async fn capture_work( + cancel: Arc, + permit: Arc, + operation: impl FnOnce(&AtomicBool) -> Result + Send + 'static, +) -> Result { + tokio::task::spawn_blocking(move || { + // A dropped async future does not stop spawn_blocking. This lease + // stays with the worker until its connection and work are released. + let _permit = permit; + if cancel.stopped.load(Ordering::SeqCst) { + return Err("Capture cancelled".into()); + } + + operation(&cancel.stopped) + }) + .await + .map_err(|error| error.to_string())? +} + +async fn capture_ipc( + cancel: &Arc, + work: &Arc, + command: &'static str, + args: serde_json::Value, +) -> Result { + capture_work(cancel.clone(), work.clone(), move |_| { + crate::desktop::ipc_sync(command, args) + }) + .await } async fn start_streams( @@ -557,20 +612,20 @@ async fn start_streams( requested_persistence, restore, bar_controlled, + cancel, + work, } = options; - let mut discovery = tokio::task::spawn_blocking(|| Capture::connect(&AtomicBool::new(false))) - .await - .map_err(|e| e.to_string())??; + let mut discovery = capture_work(cancel.clone(), work.clone(), Capture::connect).await?; let mut sources = if source_types & 1 != 0 { discovery.sources() } else { Vec::new() }; if source_types & 2 != 0 && discovery.supports_windows() { - let windows = crate::desktop::ipc("get-windows", serde_json::json!({})).await?; + let windows = capture_ipc(&cancel, &work, "get-windows", serde_json::json!({})).await?; sources.extend(crate::desktop::sharing_window_sources(&windows)?); } - let outputs = crate::desktop::ipc("get-outputs", serde_json::json!({})).await?; + let outputs = capture_ipc(&cancel, &work, "get-outputs", serde_json::json!({})).await?; let rememberable = sources .iter() .filter(|source| { @@ -616,13 +671,21 @@ async fn start_streams( return Err("Invalid persistence selection".into()); } let selected = validate_selection(&prompt.sources, &selection.names, multiple)?; - crate::desktop::wait_for_surface_removal(picker_pid).await?; + crate::desktop::wait_for_surface_removal_using(|| { + capture_ipc( + &cancel, + &work, + "has-client-surfaces", + serde_json::json!({"pid": picker_pid}), + ) + }) + .await?; (selected, selection.persist_mode) }; let mut reply = Options::new(); - let fresh_outputs = crate::desktop::ipc("get-outputs", serde_json::json!({})).await?; + let fresh_outputs = capture_ipc(&cancel, &work, "get-outputs", serde_json::json!({})).await?; let monitors = selected .iter() @@ -698,11 +761,12 @@ async fn start_streams( children.push(child); } - tokio::task::spawn_blocking(move || discovery.validate_sources(&generations)) - .await - .map_err(|error| error.to_string())??; + capture_work(cancel.clone(), work.clone(), move |stop| { + discovery.validate_sources(&generations, stop) + }) + .await?; - let final_outputs = crate::desktop::ipc("get-outputs", serde_json::json!({})).await?; + let final_outputs = capture_ipc(&cancel, &work, "get-outputs", serde_json::json!({})).await?; if !crate::restore::selection_unchanged(&monitors, &outputs, &final_outputs) { return Err("A selected display changed during stream startup".into()); } @@ -711,7 +775,7 @@ async fn start_streams( return Err("A selected stream ended during startup".into()); } - let final_windows = crate::desktop::ipc("get-windows", serde_json::json!({})).await?; + let final_windows = capture_ipc(&cancel, &work, "get-windows", serde_json::json!({})).await?; let streams = streams .into_iter() .map(|(name, ready)| { @@ -914,6 +978,61 @@ pub async fn run() -> Result<(), Box> { #[cfg(test)] mod tests { use super::*; + + #[tokio::test] + async fn cancelled_async_work_holds_session_capacity_until_worker_exits() { + let backend = Backend::default(); + let cancel = Arc::new(Cancel::default()); + let permits: Vec<_> = (0..MAX_SESSIONS) + .map(|_| Arc::new(backend.work.clone().try_acquire_owned().unwrap())) + .collect(); + let (started, received) = tokio::sync::oneshot::channel(); + let (release, blocked) = std::sync::mpsc::channel(); + let permit = permits[0].clone(); + let flag = cancel.clone(); + let task = tokio::spawn(async move { + capture_work(flag, permit, move |stop| { + started.send(()).unwrap(); + blocked.recv_timeout(Duration::from_secs(2)).unwrap(); + assert!(stop.load(Ordering::SeqCst)); + Ok(()) + }) + .await + }); + received.await.unwrap(); + cancel.stop(); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + // End the corresponding session while its blocking worker is alive. + let mut permits = permits; + permits.remove(0); + assert!(backend.work.clone().try_acquire_owned().is_err()); + release.send(()).unwrap(); + let recovered = tokio::time::timeout(Duration::from_secs(1), backend.work.clone().acquire_owned()) + .await + .unwrap() + .unwrap(); + assert_eq!(recovered.num_permits(), 1); + assert_eq!(backend.work.available_permits(), 0); + drop(recovered); + drop(permits); + assert_eq!(backend.work.available_permits(), MAX_SESSIONS); + } + + #[tokio::test] + async fn already_cancelled_capture_never_starts_blocking_discovery() { + let backend = Backend::default(); + let cancel = Arc::new(Cancel::default()); + cancel.stop(); + let permit = Arc::new(backend.work.clone().try_acquire_owned().unwrap()); + let result = capture_work(cancel, permit, |_| -> Result<(), String> { + panic!("Cancelled discovery must not start"); + }) + .await; + assert!(result.unwrap_err().contains("cancelled")); + assert_eq!(backend.work.available_permits(), MAX_SESSIONS); + } + #[test] fn bar_control_claims_are_bound_to_the_recorder_connection() { assert!(owns_session( diff --git a/portal/src/bridge.rs b/portal/src/bridge.rs index 718b308..0d00a80 100644 --- a/portal/src/bridge.rs +++ b/portal/src/bridge.rs @@ -1,15 +1,25 @@ use std::os::unix::net::UnixStream; use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use serde_json::Value as Json; +use tokio::sync::Notify; #[derive(Clone)] pub(crate) struct Bridge(Arc); +struct Connection { + stream: UnixStream, + id: u64, +} + struct BridgeInner { - stream: std::sync::Mutex, + connection: std::sync::Mutex, shutdown: UnixStream, + closed: AtomicBool, + changed: Notify, + timeout: Duration, } impl Bridge { @@ -19,19 +29,32 @@ impl Bridge { .ok_or("Missing runtime directory")? .join("ferese/control.sock"); let stream = UnixStream::connect(path).map_err(|error| error.to_string())?; + Self::from_stream(stream, Duration::from_secs(2)) + } + + fn from_stream(stream: UnixStream, timeout: Duration) -> Result { stream - .set_read_timeout(Some(Duration::from_secs(2))) + .set_read_timeout(Some(timeout)) .map_err(|error| error.to_string())?; stream - .set_write_timeout(Some(Duration::from_secs(2))) + .set_write_timeout(Some(timeout)) .map_err(|error| error.to_string())?; let shutdown = stream.try_clone().map_err(|error| error.to_string())?; Ok(Self(Arc::new(BridgeInner { - stream: std::sync::Mutex::new(stream), + connection: std::sync::Mutex::new(Connection { stream, id: 0 }), shutdown, + closed: AtomicBool::new(false), + changed: Notify::new(), + timeout, }))) } + #[cfg(test)] + pub(crate) fn test_pair() -> (Self, UnixStream) { + let (stream, peer) = UnixStream::pair().unwrap(); + (Self::from_stream(stream, Duration::from_secs(1)).unwrap(), peer) + } + pub(crate) async fn call(&self, command: &'static str, args: Json) -> Result { self.request(command, args, false).await } @@ -43,28 +66,68 @@ impl Bridge { async fn request(&self, command: &'static str, args: Json, wait: bool) -> Result { let bridge = self.clone(); - tokio::task::spawn_blocking(move || { - let mut stream = bridge.0.stream.lock().map_err(|_| "Compositor connection failed")?; - stream - .set_read_timeout(if wait { None } else { Some(Duration::from_secs(2)) }) - .map_err(|error| error.to_string())?; + match tokio::task::spawn_blocking(move || bridge.request_sync(command, args, wait)).await { + Ok(result) => result, + Err(error) => { + self.close(); + Err(error.to_string()) + } + } + } + + fn request_sync(&self, command: &'static str, args: Json, wait: bool) -> Result { + let mut connection = match self.0.connection.lock() { + Ok(connection) => connection, + Err(_) => { + self.close(); + return Err("Compositor connection failed".into()); + } + }; + + let response = (|| { + if self.is_closed() { + return Err("Compositor connection is closed".to_owned()); + } + + connection.id = connection.id.checked_add(1).ok_or("Compositor request IDs exhausted")?; let request = ferese_ipc::Request { version: ferese_ipc::VERSION, - id: 1, + id: connection.id, kind: "command".into(), command: command.into(), args, }; - ferese_ipc::write_frame(&mut *stream, &request).map_err(|error| error.to_string())?; - let response: ferese_ipc::Response = - ferese_ipc::read_frame(&mut *stream).map_err(|error| error.to_string())?; - if let Some(error) = response.error { - return Err(error.message); + connection + .stream + .set_read_timeout(if wait { None } else { Some(self.0.timeout) }) + .map_err(|error| error.to_string())?; + ferese_ipc::write_frame(&mut connection.stream, &request).map_err(|error| error.to_string())?; + let value: Json = ferese_ipc::read_frame(&mut connection.stream).map_err(|error| error.to_string())?; + let has_result = value.get("result").is_some(); + let response: ferese_ipc::Response = serde_json::from_value(value).map_err(|error| error.to_string())?; + if response.id != request.id || response.version != ferese_ipc::VERSION { + return Err("Unexpected compositor response ID or protocol version".into()); } - response.result.ok_or("Missing compositor response".into()) - }) - .await - .map_err(|error| error.to_string())? + + if has_result == response.error.is_some() { + return Err("Invalid compositor response outcome".into()); + } + + Ok(response) + })(); + + match response { + Ok(response) => match response.error { + Some(error) => Err(error.message), + None => Ok(response.result.unwrap_or(Json::Null)), + }, + Err(error) => { + // Poison the stream before releasing the request lock so a + // queued caller cannot read a late or partial response. + self.close(); + Err(error) + } + } } pub(crate) async fn capture_watch(&self, session: u64) -> Result { @@ -72,24 +135,52 @@ impl Bridge { .await } + pub(crate) fn is_closed(&self) -> bool { + self.0.closed.load(Ordering::SeqCst) + } + + pub(crate) async fn terminated(&self) { + let changed = self.0.changed.notified(); + tokio::pin!(changed); + changed.as_mut().enable(); + if !self.is_closed() { + changed.await; + } + } + pub(crate) async fn closed(&self) { - use std::io::Read; let Ok(reader) = self.0.shutdown.try_clone() else { return; }; - if reader.set_nonblocking(true).is_err() { - return; - } let Ok(reader) = tokio::io::unix::AsyncFd::new(reader) else { return; }; loop { - let Ok(mut ready) = reader.readable().await else { + let readable = tokio::select! { + _ = self.terminated() => return, + ready = reader.readable() => ready, + }; + let Ok(mut ready) = readable else { return; }; match ready.try_io(|reader| { - let mut socket = reader.get_ref(); - socket.read(&mut [0_u8; 1]) + use std::os::fd::AsRawFd; + let mut byte = 0u8; + // Per-call nonblocking peek leaves the shared descriptor flags + // and response bytes untouched for the request reader. + let result = unsafe { + libc::recv( + reader.get_ref().as_raw_fd(), + (&mut byte as *mut u8).cast(), + 1, + libc::MSG_PEEK | libc::MSG_DONTWAIT, + ) + }; + if result < 0 { + Err(std::io::Error::last_os_error()) + } else { + Ok(result) + } }) { Ok(_) => return, Err(_) => continue, @@ -98,6 +189,11 @@ impl Bridge { } pub(crate) fn close(&self) { + self.0.closed.store(true, Ordering::SeqCst); let _ = self.0.shutdown.shutdown(std::net::Shutdown::Both); + self.0.changed.notify_waiters(); } } + +#[cfg(test)] +mod tests; diff --git a/portal/src/bridge/tests.rs b/portal/src/bridge/tests.rs new file mode 100644 index 0000000..ad75263 --- /dev/null +++ b/portal/src/bridge/tests.rs @@ -0,0 +1,158 @@ +use super::*; +use ferese_ipc::{Request, Response, read_frame, write_frame}; +use serde_json::json; +use std::io::{Read, Write}; + +fn pair() -> (Bridge, UnixStream) { + pair_with_timeout(Duration::from_secs(1)) +} + +fn pair_with_timeout(timeout: Duration) -> (Bridge, UnixStream) { + let (client, server) = UnixStream::pair().unwrap(); + server.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); + (Bridge::from_stream(client, timeout).unwrap(), server) +} + +#[tokio::test] +async fn valid_errors_preserve_connection_and_request_ids_are_unique() { + let (bridge, mut server) = pair(); + let peer = std::thread::spawn(move || { + let first: Request = read_frame(&mut server).unwrap(); + write_frame(&mut server, &Response::error(first.id, "denied", "Denied")).unwrap(); + let second: Request = read_frame(&mut server).unwrap(); + assert_eq!(second.id, first.id + 1); + write_frame(&mut server, &Response::success(second.id, json!({"enabled":false}))).unwrap(); + let third: Request = read_frame(&mut server).unwrap(); + assert_eq!(third.id, second.id + 1); + write_frame(&mut server, &Response::success(third.id, Json::Null)).unwrap(); + }); + assert_eq!(bridge.call("enable", json!({})).await.unwrap_err(), "Denied"); + assert!(!bridge.is_closed()); + assert_eq!( + bridge.call("disable", json!({})).await.unwrap(), + json!({"enabled":false}) + ); + assert!(!bridge.is_closed()); + assert_eq!(bridge.call("empty-result", json!({})).await.unwrap(), Json::Null); + peer.join().unwrap(); +} + +#[tokio::test] +async fn timeout_closes_stream_before_a_late_reply_can_be_reused() { + let (bridge, mut server) = pair_with_timeout(Duration::from_millis(40)); + let (release, blocked) = std::sync::mpsc::channel(); + let peer = std::thread::spawn(move || { + let request: Request = read_frame(&mut server).unwrap(); + blocked.recv_timeout(Duration::from_secs(2)).unwrap(); + assert!(write_frame(&mut server, &Response::success(request.id, json!({"enabled":true}))).is_err()); + assert_eq!(server.read(&mut [0u8; 1]).unwrap(), 0); + }); + assert!(bridge.call("enable", json!({})).await.is_err()); + assert!(bridge.is_closed()); + tokio::time::timeout(Duration::from_secs(1), bridge.terminated()) + .await + .unwrap(); + assert!(bridge.call("disable", json!({})).await.unwrap_err().contains("closed")); + release.send(()).unwrap(); + peer.join().unwrap(); +} + +#[tokio::test] +async fn partial_reply_timeout_cannot_corrupt_the_next_frame() { + let (bridge, mut server) = pair_with_timeout(Duration::from_millis(40)); + let (release, blocked) = std::sync::mpsc::channel(); + let peer = std::thread::spawn(move || { + let _: Request = read_frame(&mut server).unwrap(); + server.write_all(&20u32.to_be_bytes()).unwrap(); + server.write_all(b"{\"id\":").unwrap(); + blocked.recv_timeout(Duration::from_secs(2)).unwrap(); + assert_eq!(server.read(&mut [0u8; 1]).unwrap(), 0); + }); + assert!(bridge.call("enable", json!({})).await.is_err()); + assert!(bridge.is_closed()); + assert!(bridge.call("disable", json!({})).await.is_err()); + release.send(()).unwrap(); + peer.join().unwrap(); +} + +#[tokio::test] +async fn malformed_or_mismatched_replies_poison_the_bridge() { + for case in 0..6 { + let (bridge, mut server) = pair(); + let peer = std::thread::spawn(move || { + let request: Request = read_frame(&mut server).unwrap(); + let mut response = Response::success(request.id, json!({})); + match case { + 0 => response.id += 1, + 1 => response.version += 1, + 2 => response.result = None, + 3 => response.error = Response::error(request.id, "failed", "Failed").error, + 4 => { + server.write_all(&1u32.to_be_bytes()).unwrap(); + server.write_all(b"{").unwrap(); + return; + } + _ => { + server.write_all(&u32::MAX.to_be_bytes()).unwrap(); + return; + } + } + write_frame(&mut server, &response).unwrap(); + }); + assert!(bridge.call("enable", json!({})).await.is_err(), "case {case}"); + assert!(bridge.is_closed(), "case {case}"); + assert!(bridge.call("disable", json!({})).await.is_err()); + peer.join().unwrap(); + } +} + +#[tokio::test] +async fn closing_any_clone_interrupts_a_blocked_watch() { + let (bridge, mut server) = pair(); + let (started, received) = tokio::sync::oneshot::channel(); + let peer = std::thread::spawn(move || { + let request: Request = read_frame(&mut server).unwrap(); + assert_eq!(request.command, "input-capture-watch"); + started.send(()).unwrap(); + assert_eq!(server.read(&mut [0u8; 1]).unwrap(), 0); + }); + let watcher = bridge.clone(); + let task = tokio::spawn(async move { watcher.capture_watch(7).await }); + received.await.unwrap(); + bridge.close(); + assert!( + tokio::time::timeout(Duration::from_secs(1), task) + .await + .unwrap() + .unwrap() + .is_err() + ); + assert!(bridge.call("enable", json!({})).await.is_err()); + peer.join().unwrap(); +} + +#[tokio::test] +async fn peer_watch_keeps_descriptor_flags_and_response_bytes_untouched() { + use std::os::fd::AsRawFd; + + let (bridge, mut server) = pair(); + let flags = unsafe { libc::fcntl(bridge.0.shutdown.as_raw_fd(), libc::F_GETFL) }; + server.write_all(b"x").unwrap(); + tokio::time::timeout(Duration::from_secs(1), bridge.closed()) + .await + .unwrap(); + assert_eq!( + unsafe { libc::fcntl(bridge.0.shutdown.as_raw_fd(), libc::F_GETFL) }, + flags + ); + let mut byte = [0u8; 1]; + bridge + .0 + .connection + .lock() + .unwrap() + .stream + .read_exact(&mut byte) + .unwrap(); + assert_eq!(byte, *b"x"); +} diff --git a/portal/src/capture.rs b/portal/src/capture.rs index b990e84..5e11a5c 100644 --- a/portal/src/capture.rs +++ b/portal/src/capture.rs @@ -87,6 +87,10 @@ pub struct Capture { impl Capture { pub fn connect(stop: &AtomicBool) -> Result { let connection = Connection::connect_to_env().map_err(|e| e.to_string())?; + Self::connect_with_connection(connection, stop) + } + + fn connect_with_connection(connection: Connection, stop: &AtomicBool) -> Result { let queue = connection.new_event_queue(); connection.display().get_registry(&queue.handle(), ()); let mut this = Self { @@ -135,8 +139,8 @@ impl Capture { Ok(()) } - pub fn validate_sources(&mut self, selected: &[(String, u32)]) -> Result<(), String> { - self.sync(&AtomicBool::new(false))?; + pub fn validate_sources(&mut self, selected: &[(String, u32)], stop: &AtomicBool) -> Result<(), String> { + self.sync(stop)?; for (name, generation) in selected { if self.generation(name) != Some(*generation) { return Err("A selected display disconnected during approval".into()); @@ -534,6 +538,41 @@ impl Dispatch for State { #[cfg(test)] mod tests { use super::*; + + #[test] + fn discovery_and_final_validation_cancel_on_a_stalled_compositor() { + use std::io::Read; + use std::os::unix::net::UnixStream; + + for discovery in [true, false] { + let (client, mut server) = UnixStream::pair().unwrap(); + server.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); + let stop = Arc::new(AtomicBool::new(false)); + let flag = stop.clone(); + let worker = std::thread::spawn(move || { + let connection = Connection::from_socket(client).unwrap(); + if discovery { + Capture::connect_with_connection(connection, &flag).err().unwrap() + } else { + let queue = connection.new_event_queue(); + let mut capture = Capture { + connection, + queue, + state: State::default(), + selected_output: None, + }; + capture.validate_sources(&[], &flag).unwrap_err() + } + }); + // Wait for the real Wayland request before cancelling its roundtrip. + assert!(server.read(&mut [0u8; 64]).unwrap() > 0); + let cancelled = Instant::now(); + stop.store(true, Ordering::SeqCst); + assert!(worker.join().unwrap().to_lowercase().contains("cancel")); + assert!(cancelled.elapsed() < Duration::from_secs(1)); + } + } + #[test] fn capture_orientation_matches_presentation_for_all_transforms() { use wl_output::Transform::*; diff --git a/portal/src/desktop.rs b/portal/src/desktop.rs index a5f48ee..8487048 100644 --- a/portal/src/desktop.rs +++ b/portal/src/desktop.rs @@ -235,7 +235,7 @@ pub(crate) async fn ipc(command: &'static str, args: serde_json::Value) -> Resul .map_err(|error| error.to_string())? } -fn ipc_sync(command: &'static str, args: serde_json::Value) -> Result { +pub(crate) fn ipc_sync(command: &'static str, args: serde_json::Value) -> Result { let path = std::env::var_os("XDG_RUNTIME_DIR") .map(PathBuf::from) .ok_or("Missing runtime directory")? @@ -294,8 +294,16 @@ async fn select_geometry(point: bool) -> Result, String> { } pub(crate) async fn wait_for_surface_removal(pid: u32) -> Result<(), String> { + wait_for_surface_removal_using(|| ipc("has-client-surfaces", serde_json::json!({"pid": pid}))).await +} + +pub(crate) async fn wait_for_surface_removal_using< + F: std::future::Future>, +>( + mut check: impl FnMut() -> F, +) -> Result<(), String> { for _ in 0..100 { - if ipc("has-client-surfaces", serde_json::json!({"pid": pid})).await? == false { + if check().await? == false { return Ok(()); } tokio::time::sleep(Duration::from_millis(10)).await; @@ -633,7 +641,7 @@ async fn wallpaper( return Ok(None); } let (_, path) = file.keep().map_err(|error| error.to_string())?; - if let Err(error) = update_wallpaper(&path, target) { + if let Err(error) = update_wallpaper(&path, target).await { let _ = std::fs::remove_file(path); return Err(error); } @@ -699,44 +707,99 @@ fn prepare_wallpaper(input: &Path, cancel: &Cancel) -> Result Result<(), String> { - let _transaction = crate::backend::CONFIG_TRANSACTION - .lock() - .map_err(|_| "Configuration transaction failed")?; - let path = ferese_config::config_path().ok_or("Missing config path")?; - edit_wallpaper(&path, image, target) +async fn update_wallpaper(image: &Path, target: &str) -> Result<(), String> { + let value = crate::bridge::Bridge::connect()? + .call("theme-get", serde_json::json!({})) + .await?; + let snapshot = ferese_ipc::theme::Snapshot::decode(value, ferese_config::families::builtins)?; + let image = image.to_owned(); + let target = target.to_owned(); + tokio::task::spawn_blocking(move || { + let _transaction = crate::backend::CONFIG_TRANSACTION + .lock() + .map_err(|_| "Configuration transaction failed")?; + let path = ferese_config::config_path().ok_or("Missing config path")?; + edit_wallpaper(&path, &image, &target, snapshot.theme.appearance) + }) + .await + .map_err(|error| error.to_string())? } -fn edit_wallpaper(path: &Path, image: &Path, target: &str) -> Result<(), String> { +fn wallpaper_theme( + document: &ferese_config::Document, + directory: &Path, + appearance: ferese_config::theme::Appearance, +) -> Result { + let mut document = document.clone(); + let mode = match appearance { + ferese_config::theme::Appearance::Light => "light", + ferese_config::theme::Appearance::Dark => "dark", + }; + // The compositor knows the active appearance, including system overrides. + // Resolve that variant without changing the user's automatic policy. + document + .set("theme.mode", mode.into()) + .map_err(|error| error.to_string())?; + ferese_config::theme::resolve(&document, directory, jiff::Timestamp::now(), |file| { + std::fs::read_to_string(file).map_err(|error| error.to_string()) + }) + .map(|candidate| candidate.theme) +} + +fn edit_wallpaper( + path: &Path, + image: &Path, + target: &str, + appearance: ferese_config::theme::Appearance, +) -> Result<(), String> { + use ferese_config::theme::Appearance; + let source = match std::fs::read_to_string(path) { Ok(source) => source, Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), Err(error) => return Err(error.to_string()), }; let mut document = ferese_config::Document::parse(&source).map_err(|error| error.to_string())?; - if target == "background" && document.get("theme.background.lock_path").is_none() { - let old = document - .get("theme.background.path") - .and_then(serde_json::Value::as_str) - .unwrap_or(ferese_config::default_wallpaper()) - .to_owned(); + let directory = path.parent().ok_or("Invalid config path")?; + let before = wallpaper_theme(&document, directory, appearance)?.tokens.background; + let (variant, other) = match appearance { + Appearance::Light => ("light", Appearance::Dark), + Appearance::Dark => ("dark", Appearance::Light), + }; + let other_before = wallpaper_theme(&document, directory, other)?.tokens.background; + let base = format!("theme.{variant}.background"); + if target == "background" && before.lock_path.is_none() { + let old = before + .path + .unwrap_or_else(|| ferese_config::default_wallpaper_for(appearance).into()); document - .set("theme.background.lock_path", old.into()) + .set(&format!("{base}.lock_path"), old.to_string_lossy().into_owned().into()) .map_err(|error| error.to_string())?; } + if target != "lockscreen" { document - .set("theme.background.path", image.to_string_lossy().into_owned().into()) + .set(&format!("{base}.path"), image.to_string_lossy().into_owned().into()) .map_err(|error| error.to_string())?; } + if target != "background" { document .set( - "theme.background.lock_path", + &format!("{base}.lock_path"), image.to_string_lossy().into_owned().into(), ) .map_err(|error| error.to_string())?; } + + let after = wallpaper_theme(&document, directory, appearance)?.tokens.background; + if (target != "lockscreen" && after.path.as_deref() != Some(image)) + || (target != "background" && after.lock_path.as_deref() != Some(image)) + || wallpaper_theme(&document, directory, other)?.tokens.background != other_before + { + return Err("Wallpaper change did not resolve to the requested image".into()); + } + let parent = path.parent().ok_or("Invalid config path")?; std::fs::create_dir_all(parent).map_err(|error| error.to_string())?; let mut temp = tempfile::NamedTempFile::new_in(parent).map_err(|error| error.to_string())?; @@ -821,27 +884,144 @@ mod tests { #[test] fn wallpaper_targets_preserve_independent_paths_and_other_settings() { + use ferese_config::theme::Appearance::{Dark, Light}; + + for appearance in [Light, Dark] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.kdl"); + std::fs::write( + &path, + "// Keep my settings\ntheme { background { path \"/old.png\"; }; }; commands { terminal \"foot\"; }\n", + ) + .unwrap(); + let resolve = |mode| { + let document = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + wallpaper_theme(&document, dir.path(), mode).unwrap().tokens.background + }; + let other = if appearance == Light { Dark } else { Light }; + let untouched = resolve(other); + edit_wallpaper(&path, Path::new("/desktop.png"), "background", appearance).unwrap(); + assert_eq!(resolve(appearance).path.as_deref(), Some(Path::new("/desktop.png"))); + assert_eq!(resolve(appearance).lock_path.as_deref(), Some(Path::new("/old.png"))); + assert_eq!(resolve(other), untouched); + assert!(std::fs::read_to_string(&path).unwrap().contains("Keep my settings")); + + edit_wallpaper(&path, Path::new("/lock.png"), "lockscreen", appearance).unwrap(); + assert_eq!(resolve(appearance).path.as_deref(), Some(Path::new("/desktop.png"))); + assert_eq!(resolve(appearance).lock_path.as_deref(), Some(Path::new("/lock.png"))); + assert_eq!(resolve(other), untouched); + + edit_wallpaper(&path, Path::new("/both.png"), "both", appearance).unwrap(); + assert_eq!(resolve(appearance).path.as_deref(), Some(Path::new("/both.png"))); + assert_eq!(resolve(appearance).lock_path.as_deref(), Some(Path::new("/both.png"))); + assert_eq!(resolve(other), untouched); + let document = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + assert_eq!(document.get("theme.background.path").unwrap(), "/old.png"); + assert_eq!(document.get("commands.terminal").unwrap(), &serde_json::json!(["foot"])); + } + } + + #[test] + fn wallpaper_replaces_active_overrides_without_changing_auto_policy() { + use ferese_config::theme::Appearance::{Dark, Light}; + + for appearance in [Light, Dark] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.kdl"); + std::fs::write( + &path, + r#"theme { + mode "auto" + schedule { source "system"; } + background { path "/shared.png"; lock-path "/shared-lock.png"; } + light { background { path "light.png"; lock-path "light-lock.png"; }; } + dark { background { path "dark.png"; lock-path "dark-lock.png"; }; } + } + "#, + ) + .unwrap(); + let before = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let other = if appearance == Light { Dark } else { Light }; + let old_active = wallpaper_theme(&before, dir.path(), appearance) + .unwrap() + .tokens + .background; + let old_other = wallpaper_theme(&before, dir.path(), other).unwrap().tokens.background; + edit_wallpaper(&path, Path::new("/new.png"), "background", appearance).unwrap(); + let after = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let current = wallpaper_theme(&after, dir.path(), appearance) + .unwrap() + .tokens + .background; + assert_eq!(current.path.as_deref(), Some(Path::new("/new.png"))); + assert_eq!(current.lock_path, old_active.lock_path); + assert_eq!( + wallpaper_theme(&after, dir.path(), other).unwrap().tokens.background, + old_other + ); + assert_eq!(after.get("theme.mode").unwrap(), "auto"); + assert_eq!(after.get("theme.background"), before.get("theme.background")); + + edit_wallpaper(&path, Path::new("/new-lock.png"), "lockscreen", appearance).unwrap(); + let after = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let current = wallpaper_theme(&after, dir.path(), appearance) + .unwrap() + .tokens + .background; + assert_eq!(current.path.as_deref(), Some(Path::new("/new.png"))); + assert_eq!(current.lock_path.as_deref(), Some(Path::new("/new-lock.png"))); + assert_eq!( + wallpaper_theme(&after, dir.path(), other).unwrap().tokens.background, + old_other + ); + } + } + + #[test] + fn desktop_only_change_pins_the_effective_variant_lock_wallpaper() { + use ferese_config::theme::Appearance::{Dark, Light}; + let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("config.kdl"); std::fs::write( &path, - "// Keep my settings\ntheme { background { path \"/old.png\"; }; }; commands { terminal \"foot\"; }\n", + r#"theme { + background { path "shared.png"; } + light { background { path "light.png"; }; } + dark { background { path "dark.png"; }; } + } + "#, ) .unwrap(); - edit_wallpaper(&path, Path::new("/desktop.png"), "background").unwrap(); - let document = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(document.get("theme.background.path").unwrap(), "/desktop.png"); - assert_eq!(document.get("theme.background.lock_path").unwrap(), "/old.png"); - assert!(std::fs::read_to_string(&path).unwrap().contains("Keep my settings")); - edit_wallpaper(&path, Path::new("/lock.png"), "lockscreen").unwrap(); - let document = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(document.get("theme.background.path").unwrap(), "/desktop.png"); - assert_eq!(document.get("theme.background.lock_path").unwrap(), "/lock.png"); - edit_wallpaper(&path, Path::new("/both.png"), "both").unwrap(); - let document = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); - assert_eq!(document.get("theme.background.path").unwrap(), "/both.png"); - assert_eq!(document.get("theme.background.lock_path").unwrap(), "/both.png"); - assert_eq!(document.get("commands.terminal").unwrap(), &serde_json::json!(["foot"])); + let before = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let previous = wallpaper_theme(&before, dir.path(), Dark).unwrap().tokens.background; + assert_eq!(previous.lock_path, None); + edit_wallpaper(&path, Path::new("/new.png"), "background", Dark).unwrap(); + let after = ferese_config::Document::parse(&std::fs::read_to_string(&path).unwrap()).unwrap(); + let current = wallpaper_theme(&after, dir.path(), Dark).unwrap().tokens.background; + assert_eq!(current.lock_path, previous.path); + assert_eq!( + wallpaper_theme(&after, dir.path(), Light).unwrap().tokens.background, + wallpaper_theme(&before, dir.path(), Light).unwrap().tokens.background + ); + } + + #[test] + fn wallpaper_validation_failure_preserves_config() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.kdl"); + let source = "theme { dark { file \"missing.kdl\"; }; }\n"; + std::fs::write(&path, source).unwrap(); + assert!( + edit_wallpaper( + &path, + Path::new("/new.png"), + "both", + ferese_config::theme::Appearance::Dark + ) + .is_err() + ); + assert_eq!(std::fs::read_to_string(&path).unwrap(), source); } #[test] diff --git a/portal/src/input_capture.rs b/portal/src/input_capture.rs index 901fe69..29d9dd0 100644 --- a/portal/src/input_capture.rs +++ b/portal/src/input_capture.rs @@ -105,6 +105,20 @@ impl InputCapture { } } + async fn finish_native_call( + &self, + connection: &Connection, + path: &OwnedObjectPath, + bridge: &Bridge, + result: Result, + ) -> zbus::fdo::Result { + if bridge.is_closed() { + self.end(connection, path.as_str()).await; + } + + result.map_err(failed) + } + async fn native(session: &Session) -> zbus::fdo::Result<(Bridge, u64)> { let state = session.state.lock().await; if !state.started || session.cancel.stopped.load(Ordering::SeqCst) { @@ -127,9 +141,16 @@ impl InputCapture { bridge: Bridge, id: u64, ) { + let control = session.state.lock().await.bridge.clone(); + let Some(control) = control else { + self.end(&connection, path.as_str()).await; + return; + }; + loop { let response = tokio::select! { _ = session.cancel.wait() => break, + _ = control.terminated() => break, result = bridge.capture_watch(id) => result, }; let Ok(response) = response else { @@ -453,7 +474,9 @@ impl InputCapture { if cancel.stopped.load(Ordering::SeqCst) || session.cancel.stopped.load(Ordering::SeqCst) || result.is_none() { return Ok((1, Options::new())); } - let result = result.unwrap().map_err(failed)?; + let result = self + .finish_native_call(connection, &session_handle, &bridge, result.unwrap()) + .await?; let zones: Vec<(u32, u32, i32, i32)> = serde_json::from_value(result["zones"].clone()).map_err(|_| invalid("Invalid native capture zones"))?; Ok(( @@ -515,7 +538,9 @@ impl InputCapture { self.end(connection, session_handle.as_str()).await; return Ok((1, Options::new())); } - let result = result.unwrap().map_err(failed)?; + let result = self + .finish_native_call(connection, &session_handle, &bridge, result.unwrap()) + .await?; let failed_barriers: Vec = serde_json::from_value(result["failed_barriers"].clone()) .map_err(|_| invalid("Invalid native barrier response"))?; Ok(( @@ -580,10 +605,9 @@ impl InputCapture { } tokio::time::sleep(Duration::from_millis(10)).await; } - bridge - .call("input-capture-enable", json!({"session":id})) - .await - .map_err(failed)?; + let result = bridge.call("input-capture-enable", json!({"session":id})).await; + self.finish_native_call(connection, &session_handle, &bridge, result) + .await?; Ok((0, Options::new())) } @@ -597,10 +621,9 @@ impl InputCapture { ) -> zbus::fdo::Result<(u32, Options)> { let session = self.session(connection, &header, &session_handle, &app_id).await?; let (bridge, id) = Self::native(&session).await?; - bridge - .call("input-capture-disable", json!({"session":id})) - .await - .map_err(failed)?; + let result = bridge.call("input-capture-disable", json!({"session":id})).await; + self.finish_native_call(connection, &session_handle, &bridge, result) + .await?; Ok((0, Options::new())) } @@ -633,13 +656,14 @@ impl InputCapture { Ok(position) }) .transpose()?; - bridge + let result = bridge .call( "input-capture-release", json!({"session":id,"activation_id":activation,"cursor_position":position}), ) - .await - .map_err(failed)?; + .await; + self.finish_native_call(connection, &session_handle, &bridge, result) + .await?; Ok((0, Options::new())) } @@ -705,6 +729,74 @@ impl SessionObject { mod tests { use super::*; + #[tokio::test] + async fn broken_control_transport_ends_capture_but_valid_rejection_does_not() { + use std::process::Stdio; + use tokio::io::{AsyncBufReadExt, BufReader}; + + let mut bus = tokio::process::Command::new("dbus-daemon") + .args(["--session", "--nofork", "--print-address=1"]) + .stdout(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .unwrap(); + let mut address = String::new(); + BufReader::new(bus.stdout.take().unwrap()) + .read_line(&mut address) + .await + .unwrap(); + let connection = zbus::connection::Builder::address(address.trim()) + .unwrap() + .build() + .await + .unwrap(); + + for transport_failure in [false, true] { + let capture = InputCapture::default(); + let path: OwnedObjectPath = "/org/freedesktop/portal/desktop/session/test/capture" + .try_into() + .unwrap(); + let (control, mut peer) = Bridge::test_pair(); + let (watch, _peer) = Bridge::test_pair(); + let session = Arc::new(Session { + owner: connection.unique_name().unwrap().to_string(), + app: "test".into(), + cancel: Arc::default(), + state: Mutex::new(State { + started: true, + bridge: Some(control.clone()), + watch: Some(watch.clone()), + ..State::default() + }), + }); + capture.sessions.lock().await.insert(path.to_string(), session.clone()); + let response = std::thread::spawn(move || { + let request: ferese_ipc::Request = ferese_ipc::read_frame(&mut peer).unwrap(); + if !transport_failure { + ferese_ipc::write_frame(&mut peer, &ferese_ipc::Response::error(request.id, "denied", "Denied")) + .unwrap(); + } + }); + let result = control.call("input-capture-disable", json!({})).await; + assert!( + capture + .finish_native_call(&connection, &path, &control, result) + .await + .is_err() + ); + response.join().unwrap(); + assert_eq!(session.cancel.stopped.load(Ordering::SeqCst), transport_failure); + assert_eq!(watch.is_closed(), transport_failure); + assert_eq!( + capture.sessions.lock().await.contains_key(path.as_str()), + !transport_failure + ); + capture.end(&connection, path.as_str()).await; + } + bus.kill().await.unwrap(); + bus.wait().await.unwrap(); + } + #[test] fn requested_capabilities_are_required_and_supported_subset_is_returned() { assert!(capabilities(&Options::new()).is_err());