From 9584219ffa84c2da482f651bd70e70087eefdb5c Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Tue, 23 Nov 2021 13:46:58 +0100 Subject: [PATCH] swapchain: Use `UserDataMap` instead of generic parameter --- CHANGELOG.md | 1 + Cargo.toml | 1 + examples/raw_drm.rs | 14 +- src/backend/allocator/swapchain.rs | 34 +-- src/backend/drm/surface/gbm.rs | 57 +++-- src/utils/mod.rs | 2 + src/utils/user_data.rs | 324 +++++++++++++++++++++++++++++ 7 files changed, 379 insertions(+), 54 deletions(-) create mode 100644 src/utils/user_data.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ce3aa8f1a..e3b1538eff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - `render_texture` was removed from `Frame`, use `render_texture_at` or `render_texture_from_to` instead or use `Gles2Renderer::render_texture` as a direct replacement. - Remove `InputBackend::dispatch_new_events`, turning `InputBackend` into a definition of backend event types. Future input backends should be a `calloop::EventSource`. - Remove `InputBackend::EventError` associated type as it is unneeded since `dispatch_new_events` was removed. +- `Swapchain` does not have a generic Userdata-parameter anymore, but utilizes `UserDataMap` instead ### Additions diff --git a/Cargo.toml b/Cargo.toml index 1ac994b5d1..5e70305c5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ libc = "0.2.103" libseat= { version = "0.1.1", optional = true } libloading = { version="0.7.0", optional = true } nix = "0.22" +once_cell = "1.8.0" rand = "0.8.4" slog = "2" slog-stdlog = { version = "4", optional = true } diff --git a/examples/raw_drm.rs b/examples/raw_drm.rs index 211345a7e6..4c2013294b 100644 --- a/examples/raw_drm.rs +++ b/examples/raw_drm.rs @@ -105,9 +105,9 @@ fn main() { }) .collect::>(); let mut swapchain = Swapchain::new(allocator, w.into(), h.into(), Fourcc::Argb8888, mods); - let first_buffer: Slot, _> = swapchain.acquire().unwrap().unwrap(); + let first_buffer: Slot> = swapchain.acquire().unwrap().unwrap(); let framebuffer = surface.add_framebuffer(first_buffer.handle(), 32, 32).unwrap(); - *first_buffer.userdata() = Some(framebuffer); + first_buffer.userdata().insert_if_missing(|| framebuffer); // Get the device as an allocator into the let mut vblank_handler = VBlankHandler { @@ -137,8 +137,8 @@ fn main() { } pub struct VBlankHandler { - swapchain: Swapchain, DumbBuffer, framebuffer::Handle>, - current: Slot, framebuffer::Handle>, + swapchain: Swapchain, DumbBuffer>, + current: Slot>, surface: Rc>, } @@ -147,9 +147,9 @@ impl VBlankHandler { { // Next buffer let next = self.swapchain.acquire().unwrap().unwrap(); - if next.userdata().is_none() { + if next.userdata().get::().is_none() { let fb = self.surface.add_framebuffer(next.handle(), 32, 32).unwrap(); - *next.userdata() = Some(fb); + next.userdata().insert_if_missing(|| fb); } // now we could render to the mapping via software rendering. @@ -165,7 +165,7 @@ impl VBlankHandler { self.current = next; } - let fb = self.current.userdata().unwrap(); + let fb = *self.current.userdata().get::().unwrap(); self.surface .page_flip([(fb, self.surface.plane())].iter(), true) .unwrap(); diff --git a/src/backend/allocator/swapchain.rs b/src/backend/allocator/swapchain.rs index ef4b4ba194..c365feca78 100644 --- a/src/backend/allocator/swapchain.rs +++ b/src/backend/allocator/swapchain.rs @@ -1,10 +1,11 @@ use std::ops::Deref; use std::sync::{ atomic::{AtomicBool, Ordering}, - Arc, Mutex, MutexGuard, + Arc, }; use crate::backend::allocator::{Allocator, Buffer, Fourcc, Modifier}; +use crate::utils::user_data::UserDataMap; pub const SLOT_CAP: usize = 4; @@ -36,7 +37,7 @@ pub const SLOT_CAP: usize = 4; /// you can store then in the `Slot`s userdata field. If a buffer is re-used, its userdata is preserved for the next time /// it is returned by `acquire()`. #[derive(Debug)] -pub struct Swapchain, B: Buffer, U: 'static> { +pub struct Swapchain, B: Buffer> { /// Allocator used by the swapchain pub allocator: A, @@ -45,7 +46,7 @@ pub struct Swapchain, B: Buffer, U: 'static> { fourcc: Fourcc, modifiers: Vec, - slots: [Arc>; SLOT_CAP], + slots: [Arc>; SLOT_CAP], } /// Slot of a swapchain containing an allocated buffer and its userdata. @@ -53,50 +54,49 @@ pub struct Swapchain, B: Buffer, U: 'static> { /// The buffer is marked for re-use once all copies are dropped. /// Holding on to this struct will block the buffer in the swapchain. #[derive(Debug)] -pub struct Slot(Arc>); +pub struct Slot(Arc>); #[derive(Debug)] -struct InternalSlot { +struct InternalSlot { buffer: Option, acquired: AtomicBool, - userdata: Mutex>, + userdata: UserDataMap, } -impl Slot { +impl Slot { /// Retrieve userdata for this slot. - pub fn userdata(&self) -> MutexGuard<'_, Option> { - self.0.userdata.lock().unwrap() + pub fn userdata(&self) -> &UserDataMap { + &self.0.userdata } } -impl Default for InternalSlot { +impl Default for InternalSlot { fn default() -> Self { InternalSlot { buffer: None, acquired: AtomicBool::new(false), - userdata: Mutex::new(None), + userdata: UserDataMap::new(), } } } -impl Deref for Slot { +impl Deref for Slot { type Target = B; fn deref(&self) -> &B { Option::as_ref(&self.0.buffer).unwrap() } } -impl Drop for Slot { +impl Drop for Slot { fn drop(&mut self) { self.0.acquired.store(false, Ordering::SeqCst); } } -impl Swapchain +impl Swapchain where A: Allocator, B: Buffer, - U: 'static, { /// Create a new swapchain with the desired allocator, dimensions and pixel format for the created buffers. pub fn new( @@ -105,7 +105,7 @@ where height: u32, fourcc: Fourcc, modifiers: Vec, - ) -> Swapchain { + ) -> Swapchain { Swapchain { allocator, width, @@ -120,7 +120,7 @@ where /// /// The swapchain has an internal maximum of four re-usable buffers. /// This function returns the first free one. - pub fn acquire(&mut self) -> Result>, A::Error> { + pub fn acquire(&mut self) -> Result>, A::Error> { if let Some(free_slot) = self .slots .iter_mut() diff --git a/src/backend/drm/surface/gbm.rs b/src/backend/drm/surface/gbm.rs index c75c606ecc..292dd54644 100644 --- a/src/backend/drm/surface/gbm.rs +++ b/src/backend/drm/surface/gbm.rs @@ -19,7 +19,7 @@ use slog::{debug, error, o, trace, warn}; /// Simplified abstraction of a swapchain for gbm-buffers displayed on a [`DrmSurface`]. pub struct GbmBufferedSurface { buffers: Buffers, - swapchain: Swapchain, BufferObject<()>, (Dmabuf, FbHandle)>, + swapchain: Swapchain, BufferObject<()>>, drm: Arc>, } @@ -127,7 +127,7 @@ where let mode = drm.pending_mode(); - let mut swapchain: Swapchain, BufferObject<()>, (Dmabuf, FbHandle)> = Swapchain::new( + let mut swapchain: Swapchain, BufferObject<()>> = Swapchain::new( allocator, mode.size().0 as u32, mode.size().1 as u32, @@ -148,7 +148,8 @@ where let fb = attach_framebuffer(&drm, &*buffer)?; let dmabuf = buffer.export()?; let handle = fb.fb; - *buffer.userdata() = Some((dmabuf, fb)); + buffer.userdata().insert_if_missing(|| dmabuf); + buffer.userdata().insert_if_missing(|| fb); match drm.test_buffer(handle, &mode, true) { Ok(_) => { @@ -284,14 +285,12 @@ impl Drop for FbHandle { } } -type DmabufSlot = Slot, (Dmabuf, FbHandle)>; - struct Buffers { drm: Arc>, - _current_fb: DmabufSlot, - pending_fb: Option>, - queued_fb: Option>, - next_fb: Option>, + _current_fb: Slot>, + pending_fb: Option>>, + queued_fb: Option>>, + next_fb: Option>>, } // TODO: Replace with #[derive(Debug)] once gbm::BufferObject implements debug @@ -307,7 +306,7 @@ impl Buffers where D: AsRawFd + 'static, { - pub fn new(drm: Arc>, slot: DmabufSlot) -> Buffers { + pub fn new(drm: Arc>, slot: Slot>) -> Buffers { Buffers { drm, _current_fb: slot, @@ -319,28 +318,26 @@ where pub fn next( &mut self, - swapchain: &mut Swapchain, BufferObject<()>, (Dmabuf, FbHandle)>, + swapchain: &mut Swapchain, BufferObject<()>>, ) -> Result { - if let Some(slot) = self.next_fb.as_ref() { - return Ok(slot.userdata().as_ref().unwrap().0.clone()); + if self.next_fb.is_none() { + let slot = swapchain.acquire()?.ok_or(Error::NoFreeSlotsError)?; + + let maybe_buffer = slot.userdata().get::().clone(); + if maybe_buffer.is_none() { + let dmabuf = slot.export().map_err(Error::AsDmabufError)?; + let fb_handle = attach_framebuffer(&self.drm, &*slot)?; + + let userdata = slot.userdata(); + userdata.insert_if_missing(|| dmabuf); + userdata.insert_if_missing(|| fb_handle); + } + + self.next_fb = Some(slot); } - let slot = swapchain.acquire()?.ok_or(Error::NoFreeSlotsError)?; - - let maybe_buffer = slot.userdata().as_ref().map(|(buf, _)| buf.clone()); - let dmabuf = match maybe_buffer { - Some(buf) => buf, - None => { - let dmabuf = slot.export()?; - let fb_handle = attach_framebuffer(&self.drm, &*slot)?; - *slot.userdata() = Some((dmabuf.clone(), fb_handle)); - dmabuf - } - }; - - self.next_fb = Some(slot); - - Ok(dmabuf) + let slot = self.next_fb.as_ref().unwrap(); + Ok(slot.userdata().get::().unwrap().clone()) } pub fn queue(&mut self) -> Result<(), Error> { @@ -367,7 +364,7 @@ where fn submit(&mut self) -> Result<(), Error> { // yes it does not look like it, but both of these lines should be safe in all cases. let slot = self.queued_fb.take().unwrap(); - let fb = slot.userdata().as_ref().unwrap().1.fb; + let fb = slot.userdata().get::>().unwrap().fb; let flip = if self.drm.commit_pending() { self.drm.commit([(fb, self.drm.plane())].iter(), true) diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 48ce1fde6b..1732236b33 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -6,6 +6,8 @@ pub mod signaling; #[cfg(feature = "x11rb_event_source")] pub mod x11rb; +pub mod user_data; + pub use self::geometry::{Buffer, Logical, Physical, Point, Raw, Rectangle, Size}; /// This resource is not managed by Smithay diff --git a/src/utils/user_data.rs b/src/utils/user_data.rs new file mode 100644 index 0000000000..29d543301e --- /dev/null +++ b/src/utils/user_data.rs @@ -0,0 +1,324 @@ +//! Various utilities used for user data implementations + +use once_cell::sync::OnceCell; + +use std::any::Any; +use std::mem::ManuallyDrop; +use std::thread::{self, ThreadId}; + +use self::list::AppendList; + +/// A wrapper for user data, able to store any type, and correctly +/// handling access from a wrong thread +#[derive(Debug)] +pub struct UserData { + inner: OnceCell, +} + +#[derive(Debug)] +enum UserDataInner { + ThreadSafe(Box), + NonThreadSafe(Box>, ThreadId), +} + +// UserData itself is always threadsafe, as it only gives access to its +// content if it is send+sync or we are on the right thread +unsafe impl Send for UserData {} +unsafe impl Sync for UserData {} + +impl UserData { + /// Create a new UserData instance + pub const fn new() -> UserData { + UserData { + inner: OnceCell::new(), + } + } + + /// Sets the UserData to a given value + /// + /// The provided closure is called to init the UserData, + /// does nothing is the UserData had already been set. + pub fn set T>(&self, f: F) { + self.inner.get_or_init(|| { + UserDataInner::NonThreadSafe(Box::new(ManuallyDrop::new(f())), thread::current().id()) + }); + } + + /// Sets the UserData to a given threadsafe value + /// + /// The provided closure is called to init the UserData, + /// does nothing is the UserData had already been set. + pub fn set_threadsafe T>(&self, f: F) { + self.inner + .get_or_init(|| UserDataInner::ThreadSafe(Box::new(f()))); + } + + /// Attempt to access the wrapped user data + /// + /// Will return `None` if either: + /// + /// - The requested type `T` does not match the type used for construction + /// - This `UserData` has been created using the non-threadsafe variant and access + /// is attempted from an other thread than the one it was created on + pub fn get(&self) -> Option<&T> { + match self.inner.get() { + Some(&UserDataInner::ThreadSafe(ref val)) => ::downcast_ref::(&**val), + Some(&UserDataInner::NonThreadSafe(ref val, threadid)) => { + // only give access if we are on the right thread + if threadid == thread::current().id() { + ::downcast_ref::(&***val) + } else { + None + } + } + None => None, + } + } +} + +impl Drop for UserData { + fn drop(&mut self) { + // only drop non-Send user data if we are on the right thread, leak it otherwise + if let Some(&mut UserDataInner::NonThreadSafe(ref mut val, threadid)) = self.inner.get_mut() { + if threadid == thread::current().id() { + unsafe { + ManuallyDrop::drop(&mut **val); + } + } + } + } +} + +/// A storage able to store several values of `UserData` +/// of different types. It behaves similarly to a `TypeMap`. +#[derive(Debug)] +pub struct UserDataMap { + list: AppendList, +} + +impl UserDataMap { + /// Create a new map + pub fn new() -> UserDataMap { + UserDataMap { + list: AppendList::new(), + } + } + + /// Attempt to access the wrapped user data of a given type + /// + /// Will return `None` if no value of type `T` is stored in this `UserDataMap` + /// and accessible from this thread + pub fn get(&self) -> Option<&T> { + for user_data in &self.list { + if let Some(val) = user_data.get::() { + return Some(val); + } + } + None + } + + /// Insert a value in the map if it is not already there + /// + /// This is the non-threadsafe variant, the type you insert don't have to be + /// threadsafe, but they will not be visible from other threads (even if they are + /// actually threadsafe). + /// + /// If the value does not already exists, the closure is called to create it and + /// this function returns `true`. If the value already exists, the closure is not + /// called, and this function returns `false`. + pub fn insert_if_missing T>(&self, init: F) -> bool { + if self.get::().is_some() { + return false; + } + let data = UserData::new(); + data.set(init); + self.list.append(data); + true + } + + /// Insert a value in the map if it is not already there + /// + /// This is the threadsafe variant, the type you insert must be threadsafe and will + /// be visible from all threads. + /// + /// If the value does not already exists, the closure is called to create it and + /// this function returns `true`. If the value already exists, the closure is not + /// called, and this function returns `false`. + pub fn insert_if_missing_threadsafe T>(&self, init: F) -> bool { + if self.get::().is_some() { + return false; + } + let data = UserData::new(); + data.set_threadsafe(init); + self.list.append(data); + true + } +} + +impl Default for UserDataMap { + fn default() -> UserDataMap { + UserDataMap::new() + } +} + +mod list { + /* + * This is a lock-free append-only list, it is used as an implementation + * detail of the UserDataMap. + * + * It was extracted from https://github.com/Diggsey/lockless under MIT license + * Copyright © Diggory Blake + */ + + use std::sync::atomic::{AtomicPtr, Ordering}; + use std::{mem, ptr}; + + type NodePtr = Option>>; + + #[derive(Debug)] + struct Node { + value: T, + next: AppendList, + } + + #[derive(Debug)] + pub struct AppendList(AtomicPtr>); + + impl AppendList { + fn node_into_raw(ptr: NodePtr) -> *mut Node { + match ptr { + Some(b) => Box::into_raw(b), + None => ptr::null_mut(), + } + } + unsafe fn node_from_raw(ptr: *mut Node) -> NodePtr { + if ptr.is_null() { + None + } else { + Some(Box::from_raw(ptr)) + } + } + + fn new_internal(ptr: NodePtr) -> Self { + AppendList(AtomicPtr::new(Self::node_into_raw(ptr))) + } + + pub fn new() -> Self { + Self::new_internal(None) + } + + pub fn append(&self, value: T) { + self.append_list(AppendList::new_internal(Some(Box::new(Node { + value, + next: AppendList::new(), + })))); + } + + unsafe fn append_ptr(&self, p: *mut Node) { + loop { + match self + .0 + .compare_exchange_weak(ptr::null_mut(), p, Ordering::AcqRel, Ordering::Acquire) + { + Ok(_) => return, + Err(head) => { + if !head.is_null() { + return (*head).next.append_ptr(p); + } + } + } + } + } + + pub fn append_list(&self, other: AppendList) { + let p = other.0.load(Ordering::Acquire); + mem::forget(other); + unsafe { self.append_ptr(p) }; + } + + pub fn iter(&self) -> AppendListIterator<'_, T> { + AppendListIterator(&self.0) + } + + pub fn iter_mut(&mut self) -> AppendListMutIterator<'_, T> { + AppendListMutIterator(&mut self.0) + } + } + + impl<'a, T> IntoIterator for &'a AppendList { + type Item = &'a T; + type IntoIter = AppendListIterator<'a, T>; + + fn into_iter(self) -> AppendListIterator<'a, T> { + self.iter() + } + } + + impl<'a, T> IntoIterator for &'a mut AppendList { + type Item = &'a mut T; + type IntoIter = AppendListMutIterator<'a, T>; + + fn into_iter(self) -> AppendListMutIterator<'a, T> { + self.iter_mut() + } + } + + impl Drop for AppendList { + fn drop(&mut self) { + unsafe { Self::node_from_raw(mem::replace(self.0.get_mut(), ptr::null_mut())) }; + } + } + + #[derive(Debug)] + pub struct AppendListIterator<'a, T>(&'a AtomicPtr>); + + impl<'a, T: 'a> Iterator for AppendListIterator<'a, T> { + type Item = &'a T; + + fn next(&mut self) -> Option<&'a T> { + let p = self.0.load(Ordering::Acquire); + if p.is_null() { + None + } else { + unsafe { + self.0 = &(*p).next.0; + Some(&(*p).value) + } + } + } + } + + #[derive(Debug)] + pub struct AppendListMutIterator<'a, T>(&'a mut AtomicPtr>); + + impl<'a, T: 'a> Iterator for AppendListMutIterator<'a, T> { + type Item = &'a mut T; + + fn next(&mut self) -> Option<&'a mut T> { + let p = self.0.load(Ordering::Acquire); + if p.is_null() { + None + } else { + unsafe { + self.0 = &mut (*p).next.0; + Some(&mut (*p).value) + } + } + } + } +} + +#[cfg(test)] +mod tests { + use super::UserDataMap; + + #[test] + fn insert_twice() { + let map = UserDataMap::new(); + + assert_eq!(map.get::(), None); + assert!(map.insert_if_missing(|| 42usize)); + assert!(!map.insert_if_missing(|| 43usize)); + assert_eq!(map.get::(), Some(&42)); + } +}