From 9584219ffa84c2da482f651bd70e70087eefdb5c Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Tue, 23 Nov 2021 13:46:58 +0100 Subject: [PATCH 1/4] 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 6ce3aa8..e3b1538 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 1ac994b..5e70305 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 211345a..4c20132 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 ef4b4ba..c365fec 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 c75c606..292dd54 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 48ce1fd..1732236 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 0000000..29d5433 --- /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)); + } +} From 643ff5395d8637029a2f3940ee971605f7ec94b8 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Tue, 23 Nov 2021 17:18:28 +0100 Subject: [PATCH 2/4] gbm: Refactor: remove internal `Buffers` struct --- src/backend/drm/surface/gbm.rs | 165 +++++++++++---------------------- 1 file changed, 56 insertions(+), 109 deletions(-) diff --git a/src/backend/drm/surface/gbm.rs b/src/backend/drm/surface/gbm.rs index 292dd54..b6b8be2 100644 --- a/src/backend/drm/surface/gbm.rs +++ b/src/backend/drm/surface/gbm.rs @@ -17,22 +17,16 @@ use crate::backend::SwapBuffersError; use slog::{debug, error, o, trace, warn}; /// Simplified abstraction of a swapchain for gbm-buffers displayed on a [`DrmSurface`]. +#[derive(Debug)] pub struct GbmBufferedSurface { - buffers: Buffers, + current_fb: Slot>, + pending_fb: Option>>, + queued_fb: Option>>, + next_fb: Option>>, swapchain: Swapchain, BufferObject<()>>, drm: Arc>, } -// TODO: Replace with #[derive(Debug)] once gbm::BufferObject implements debug -impl std::fmt::Debug for GbmBufferedSurface { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("GbmBufferedSurface") - .field("buffers", &self.buffers) - .field("drm", &self.drm) - .finish_non_exhaustive() - } -} - impl GbmBufferedSurface where D: AsRawFd + 'static, @@ -154,9 +148,11 @@ where match drm.test_buffer(handle, &mode, true) { Ok(_) => { debug!(logger, "Choosen format: {:?}", format); - let buffers = Buffers::new(drm.clone(), buffer); Ok(GbmBufferedSurface { - buffers, + current_fb: buffer, + pending_fb: None, + queued_fb: None, + next_fb: None, swapchain, drm, }) @@ -176,7 +172,24 @@ where /// *Note*: This function can be called multiple times and /// will return the same buffer until it is queued (see [`GbmBufferedSurface::queue_buffer`]). pub fn next_buffer(&mut self) -> Result { - self.buffers.next(&mut self.swapchain) + if self.next_fb.is_none() { + let slot = self.swapchain.acquire()?.ok_or(Error::NoFreeSlotsError)?; + + let maybe_buffer = slot.userdata().get::().cloned(); + 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 = self.next_fb.as_ref().unwrap(); + Ok(slot.userdata().get::().unwrap().clone()) } /// Queues the current buffer for rendering. @@ -185,7 +198,11 @@ where /// when a vblank event is received, that denotes successful scanout of the buffer. /// Otherwise the underlying swapchain will eventually run out of buffers. pub fn queue_buffer(&mut self) -> Result<(), Error> { - self.buffers.queue() + self.queued_fb = self.next_fb.take(); + if self.pending_fb.is_none() && self.queued_fb.is_some() { + self.submit()?; + } + Ok(()) } /// Marks the current frame as submitted. @@ -194,7 +211,30 @@ where /// was received after calling [`GbmBufferedSurface::queue_buffer`] on this surface. /// Otherwise the underlying swapchain will run out of buffers eventually. pub fn frame_submitted(&mut self) -> Result<(), Error> { - self.buffers.submitted() + if let Some(mut pending) = self.pending_fb.take() { + std::mem::swap(&mut pending, &mut self.current_fb); + if self.queued_fb.is_some() { + self.submit()?; + } + } + + Ok(()) + } + + 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().get::>().unwrap().fb; + + let flip = if self.drm.commit_pending() { + self.drm.commit([(fb, self.drm.plane())].iter(), true) + } else { + self.drm.page_flip([(fb, self.drm.plane())].iter(), true) + }; + if flip.is_ok() { + self.pending_fb = Some(slot); + } + flip.map_err(Error::DrmError) } /// Returns the underlying [`crtc`](drm::control::crtc) of this surface @@ -285,99 +325,6 @@ impl Drop for FbHandle { } } -struct Buffers { - drm: Arc>, - _current_fb: Slot>, - pending_fb: Option>>, - queued_fb: Option>>, - next_fb: Option>>, -} - -// TODO: Replace with #[derive(Debug)] once gbm::BufferObject implements debug -impl std::fmt::Debug for Buffers { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("Buffers") - .field("drm", &self.drm) - .finish_non_exhaustive() - } -} - -impl Buffers -where - D: AsRawFd + 'static, -{ - pub fn new(drm: Arc>, slot: Slot>) -> Buffers { - Buffers { - drm, - _current_fb: slot, - pending_fb: None, - queued_fb: None, - next_fb: None, - } - } - - pub fn next( - &mut self, - swapchain: &mut Swapchain, BufferObject<()>>, - ) -> Result { - 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 = self.next_fb.as_ref().unwrap(); - Ok(slot.userdata().get::().unwrap().clone()) - } - - pub fn queue(&mut self) -> Result<(), Error> { - self.queued_fb = self.next_fb.take(); - if self.pending_fb.is_none() && self.queued_fb.is_some() { - self.submit() - } else { - Ok(()) - } - } - - pub fn submitted(&mut self) -> Result<(), Error> { - if self.pending_fb.is_none() { - return Ok(()); - } - self._current_fb = self.pending_fb.take().unwrap(); - if self.queued_fb.is_some() { - self.submit() - } else { - Ok(()) - } - } - - 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().get::>().unwrap().fb; - - let flip = if self.drm.commit_pending() { - self.drm.commit([(fb, self.drm.plane())].iter(), true) - } else { - self.drm.page_flip([(fb, self.drm.plane())].iter(), true) - }; - if flip.is_ok() { - self.pending_fb = Some(slot); - } - flip.map_err(Error::DrmError) - } -} - fn attach_framebuffer(drm: &Arc>, bo: &BufferObject<()>) -> Result, Error> where A: AsRawFd + 'static, From 6c57e27e5e272cd2c5dbef0c9870b3a06f7b245e Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Tue, 23 Nov 2021 17:23:39 +0100 Subject: [PATCH 3/4] swapchain: Support buffer age --- src/backend/allocator/swapchain.rs | 35 +++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/backend/allocator/swapchain.rs b/src/backend/allocator/swapchain.rs index c365fec..204141a 100644 --- a/src/backend/allocator/swapchain.rs +++ b/src/backend/allocator/swapchain.rs @@ -1,6 +1,6 @@ use std::ops::Deref; use std::sync::{ - atomic::{AtomicBool, Ordering}, + atomic::{AtomicBool, AtomicU8, Ordering}, Arc, }; @@ -60,6 +60,7 @@ pub struct Slot(Arc>); struct InternalSlot { buffer: Option, acquired: AtomicBool, + age: AtomicU8, userdata: UserDataMap, } @@ -68,6 +69,11 @@ impl Slot { pub fn userdata(&self) -> &UserDataMap { &self.0.userdata } + + /// Retrieve the age of the buffer + pub fn age(&self) -> u8 { + self.0.age.load(Ordering::SeqCst) + } } impl Default for InternalSlot { @@ -75,6 +81,7 @@ impl Default for InternalSlot { InternalSlot { buffer: None, acquired: AtomicBool::new(false), + age: AtomicU8::new(0), userdata: UserDataMap::new(), } } @@ -144,6 +151,27 @@ where Ok(None) } + /// Mark a given buffer as submitted. + /// + /// This might effect internal data (e.g. buffer age) and may only be called, + /// if the buffer was actually used for display. + /// Buffers can always just be safely discarded by dropping them, but not + /// calling this function may affect performance characteristics + /// (e.g. by not tracking the buffer age). + pub fn submitted(&self, slot: Slot) { + // don't mess up the state, if the user submitted and old buffer, after e.g. a resize + if !self.slots.iter().any(|other| Arc::ptr_eq(&slot.0, other)) { + return; + } + + slot.0.age.store(1, Ordering::SeqCst); + for other_slot in &self.slots { + if !Arc::ptr_eq(other_slot, &slot.0) { + other_slot.age.fetch_add(1, Ordering::SeqCst); + } + } + } + /// Change the dimensions of newly returned buffers. /// /// Already obtained buffers are unaffected and will be cleaned up on drop. @@ -156,4 +184,9 @@ where self.height = height; self.slots = Default::default(); } + + /// Remove all internally cached buffers to e.g. reset age values + pub fn reset_buffers(&mut self) { + self.slots = Default::default(); + } } From a4f2729608b183572e7d41fb536c34d128ac87c3 Mon Sep 17 00:00:00 2001 From: Victor Brekenfeld Date: Tue, 23 Nov 2021 17:28:48 +0100 Subject: [PATCH 4/4] gbm: expose buffer age --- CHANGELOG.md | 1 + anvil/src/udev.rs | 4 ++-- src/backend/drm/surface/gbm.rs | 7 ++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3b1538..728bdbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ - 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 +- `GbmBufferedSurface::next_buffer` now additionally returns the age of the buffer ### Additions diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index fbec837..957a419 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -738,7 +738,7 @@ fn render_surface( return Ok(()); }; - let dmabuf = surface.surface.next_buffer()?; + let (dmabuf, _age) = surface.surface.next_buffer()?; renderer.bind(dmabuf)?; // and draw to our buffer @@ -867,7 +867,7 @@ fn schedule_initial_render( } fn initial_render(surface: &mut RenderSurface, renderer: &mut Gles2Renderer) -> Result<(), SwapBuffersError> { - let dmabuf = surface.next_buffer()?; + let (dmabuf, _age) = surface.next_buffer()?; renderer.bind(dmabuf)?; // Does not matter if we render an empty frame renderer diff --git a/src/backend/drm/surface/gbm.rs b/src/backend/drm/surface/gbm.rs index b6b8be2..3bc5be0 100644 --- a/src/backend/drm/surface/gbm.rs +++ b/src/backend/drm/surface/gbm.rs @@ -167,11 +167,11 @@ where } } - /// Retrieves the next buffer to be rendered into. + /// Retrieves the next buffer to be rendered into and it's age. /// /// *Note*: This function can be called multiple times and /// will return the same buffer until it is queued (see [`GbmBufferedSurface::queue_buffer`]). - pub fn next_buffer(&mut self) -> Result { + pub fn next_buffer(&mut self) -> Result<(Dmabuf, u8), Error> { if self.next_fb.is_none() { let slot = self.swapchain.acquire()?.ok_or(Error::NoFreeSlotsError)?; @@ -189,7 +189,7 @@ where } let slot = self.next_fb.as_ref().unwrap(); - Ok(slot.userdata().get::().unwrap().clone()) + Ok((slot.userdata().get::().unwrap().clone(), slot.age())) } /// Queues the current buffer for rendering. @@ -213,6 +213,7 @@ where pub fn frame_submitted(&mut self) -> Result<(), Error> { if let Some(mut pending) = self.pending_fb.take() { std::mem::swap(&mut pending, &mut self.current_fb); + self.swapchain.submitted(pending); if self.queued_fb.is_some() { self.submit()?; }