diff --git a/libs/scrap/src/wayland/display.rs b/libs/scrap/src/wayland/display.rs index 1e19bb415..ee5fdb551 100644 --- a/libs/scrap/src/wayland/display.rs +++ b/libs/scrap/src/wayland/display.rs @@ -248,15 +248,21 @@ pub fn wayland_failure_stamped() -> bool { LAST_FAILED_LOOKUP.lock().unwrap().is_some() } -/// Cursor polls must neither start discovery nor wait for an in-progress display refresh. #[cfg(feature = "drm")] -pub fn get_cached_displays() -> Option> { +pub enum CachedDisplays { + Busy, + Ready(Option>), +} + +/// Cursor polls must neither wait for discovery nor mistake contention for missing metadata. +#[cfg(feature = "drm")] +pub fn get_cached_displays() -> CachedDisplays { match DISPLAYS.try_lock() { - Ok(cache) => cache.clone(), - Err(std::sync::TryLockError::WouldBlock) => None, + Ok(cache) => CachedDisplays::Ready(cache.clone()), + Err(std::sync::TryLockError::WouldBlock) => CachedDisplays::Busy, Err(err) => { warn!("Failed to read cached Wayland displays: {}", err); - None + CachedDisplays::Ready(None) } } } @@ -535,7 +541,8 @@ mod tests { let cache = DISPLAYS.lock().unwrap(); let (tx, rx) = std::sync::mpsc::channel(); let reader = std::thread::spawn(move || { - tx.send(get_cached_displays().is_none()).unwrap(); + tx.send(matches!(get_cached_displays(), CachedDisplays::Busy)) + .unwrap(); }); // Discovery owns this lock until it completes or times out. let result = rx.recv_timeout(Duration::from_secs(1)); @@ -545,7 +552,7 @@ mod tests { clear_wayland_displays_cache(); for _ in 0..100 { - assert!(get_cached_displays().is_none()); + assert!(matches!(get_cached_displays(), CachedDisplays::Ready(None))); } } diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index db98eed79..1aa2a6295 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -14,6 +14,9 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Condvar, Mutex}; use std::time::{Duration, Instant}; +#[cfg(feature = "flutter")] +mod cursor_metadata; + const HANDSHAKE_TIMEOUT_MS: u64 = 3000; const DRM_CONNECT_TIMEOUT_MS: u64 = 1000; /// The service may hold the list back while it wakes sleeping displays: ~3.6s (DRM_WAKE_*). @@ -978,30 +981,40 @@ pub fn drm_cursor() -> Option { pub fn drm_cursor_snapshot( f: impl Fn(&DrmCursorData) -> T, ) -> Option<(T, Option)> { + use scrap::wayland::display::{get_cached_displays, wayland_snapshot_generation}; + // Keep cursor identity and output together, then release the map before DRM_STATE. - let (value, display, hidden) = { + let (value, display, epoch, hidden) = { let map = DRM_CURSOR.lock().unwrap(); - let (display, (_, cursor)) = map + let (display, (epoch, cursor)) = map .iter() .find(|(_, (_, cursor))| cursor.id != scrap::drm_reader::HIDDEN_CURSOR_ID) .or_else(|| map.iter().next())?; ( f(cursor), *display, + *epoch, cursor.id == scrap::drm_reader::HIDDEN_CURSOR_ID, ) }; let monitor = if hidden { None } else { - // Display discovery runs outside the cursor service; missing metadata means unknown DPI. - scrap::wayland::display::get_cached_displays().and_then(|wayland| { - cursor_monitor( - display.max(0) as usize, - &DRM_STATE.lock().unwrap(), - &wayland.displays, - ) - }) + cursor_metadata::monitor( + cursor_metadata::Context { + display, + epoch, + layout_generation: wayland_snapshot_generation(), + }, + get_cached_displays(), + |wayland| { + cursor_monitor( + display.max(0) as usize, + &DRM_STATE.lock().unwrap(), + &wayland.displays, + ) + }, + ) }; Some((value, monitor)) } diff --git a/src/server/drm_capturer/cursor_metadata.rs b/src/server/drm_capturer/cursor_metadata.rs new file mode 100644 index 000000000..81cbc92b3 --- /dev/null +++ b/src/server/drm_capturer/cursor_metadata.rs @@ -0,0 +1,123 @@ +use base::platform::linux::WaylandDisplayInfo; +use scrap::wayland::display::{CachedDisplays, Displays}; +use std::cell::Cell; + +#[derive(Clone, Copy, PartialEq, Eq)] +pub(super) struct Context { + pub display: i32, + pub epoch: u64, + pub layout_generation: u64, +} + +thread_local! { + static LAST_MONITOR: Cell> = const { Cell::new(None) }; +} + +pub(super) fn monitor( + context: Context, + snapshot: CachedDisplays, + resolve: impl FnOnce(&Displays) -> Option, +) -> Option { + // ID polling and bitmap retrieval run on the same cursor-service thread. + // Only contention may reuse metadata, and only for this output, stream and layout. + LAST_MONITOR.with(|last| match snapshot { + CachedDisplays::Busy => { + let cached = last.take(); + let monitor = cached + .as_ref() + .filter(|(key, _)| *key == context) + .map(|(_, monitor)| monitor.clone()); + last.set(cached); + monitor + } + CachedDisplays::Ready(displays) => { + let monitor = displays.as_deref().and_then(resolve); + // An accessible cache with no matching metadata invalidates the old density. + last.set(monitor.clone().map(|monitor| (context, monitor))); + monitor + } + }) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + const CONTEXT: Context = Context { + display: 0, + epoch: 7, + layout_generation: 11, + }; + + fn displays(logical_width: i32) -> CachedDisplays { + CachedDisplays::Ready(Some(Arc::new(Displays { + primary: 0, + displays: vec![WaylandDisplayInfo { + name: "DP-1".into(), + x: 0, + y: 0, + width: 1280, + height: 800, + logical_size: Some((logical_width, logical_width * 800 / 1280)), + refresh_rate: 60000, + transform: 0, + }], + }))) + } + + fn read(context: Context, snapshot: CachedDisplays) -> Option { + monitor(context, snapshot, |displays| { + displays.displays.first().cloned() + }) + } + + #[test] + fn busy_cache_preserves_metadata_until_a_completed_read() { + for width in [640, 1280] { + let expected = read(CONTEXT, displays(width)).unwrap().logical_size; + for _ in 0..5 { + let retained = monitor(CONTEXT, CachedDisplays::Busy, |_| { + panic!("a busy cursor lookup must not resolve displays") + }); + assert_eq!(retained.unwrap().logical_size, expected); + } + } + } + + #[test] + fn busy_cache_does_not_borrow_another_output_stream_or_layout() { + read(CONTEXT, displays(640)); + for changed in [ + Context { + display: 1, + ..CONTEXT + }, + Context { + epoch: 8, + ..CONTEXT + }, + Context { + layout_generation: 12, + ..CONTEXT + }, + ] { + assert!(read(changed, CachedDisplays::Busy).is_none()); + } + assert_eq!( + read(CONTEXT, CachedDisplays::Busy).unwrap().logical_size, + Some((640, 400)) + ); + } + + #[test] + fn missing_or_unmatched_metadata_invalidates_retained_density() { + read(CONTEXT, displays(640)); + assert!(read(CONTEXT, CachedDisplays::Ready(None)).is_none()); + assert!(read(CONTEXT, CachedDisplays::Busy).is_none()); + + read(CONTEXT, displays(640)); + assert!(monitor(CONTEXT, displays(640), |_| None).is_none()); + assert!(read(CONTEXT, CachedDisplays::Busy).is_none()); + } +}