From b1ecb8ea52b5b4cdea2fc0bd9a72bf2c3358db8f Mon Sep 17 00:00:00 2001 From: i509VCB Date: Sun, 12 Nov 2023 16:58:03 -0600 Subject: [PATCH] wayland(dmabuf): ImportNotifier for asyncrohnous buffer import --- Cargo.toml | 3 +- anvil/src/udev.rs | 13 ++- anvil/src/winit.rs | 13 ++- anvil/src/x11.rs | 12 +-- src/wayland/dmabuf/dispatch.rs | 76 ++++---------- src/wayland/dmabuf/mod.rs | 187 ++++++++++++++++++++++++++------- 6 files changed, 193 insertions(+), 111 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3f4954b2815a..39202f025e80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ drm-ffi = { version = "0.6.0", optional = true } errno = "0.3.5" gbm = { version = "0.13.0", optional = true, default-features = false, features = ["drm-support"] } glow = { version = "0.12", optional = true } +glutin = { version = "0.31.1", optional = true } input = { version = "0.8.2", default-features = false, features=["libinput_1_14"], optional = true } indexmap = "2.0" lazy_static = "1" @@ -88,7 +89,7 @@ backend_drm = ["drm", "drm-ffi"] backend_gbm = ["gbm", "cc", "pkg-config"] backend_gbm_has_fd_for_plane = [] backend_gbm_has_create_with_modifiers2 = [] -backend_egl = ["gl_generator", "libloading"] +backend_egl = ["gl_generator", "libloading", "glutin"] backend_libinput = ["input"] backend_session = [] backend_udev = ["udev", "input/udev"] diff --git a/anvil/src/udev.rs b/anvil/src/udev.rs index ddd330d3a60e..aa47beff2661 100644 --- a/anvil/src/udev.rs +++ b/anvil/src/udev.rs @@ -81,7 +81,7 @@ use smithay::{ wayland::{ compositor, dmabuf::{ - DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportError, + DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportNotifier, }, drm_lease::{ DrmLease, DrmLeaseBuilder, DrmLeaseHandler, DrmLeaseRequest, DrmLeaseState, LeaseRejected, @@ -156,13 +156,16 @@ impl DmabufHandler for AnvilState { &mut self.backend_data.dmabuf_state.as_mut().unwrap().0 } - fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf) -> Result<(), ImportError> { - self.backend_data + fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) { + if self + .backend_data .gpus .single_renderer(&self.backend_data.primary_gpu) .and_then(|mut renderer| renderer.import_dmabuf(&dmabuf, None)) - .map(|_| ()) - .map_err(|_| ImportError::Failed) + .is_err() + { + notifier.failed(); + } } } delegate_dmabuf!(AnvilState); diff --git a/anvil/src/winit.rs b/anvil/src/winit.rs index a75fc316e8f3..2397ee18ec33 100644 --- a/anvil/src/winit.rs +++ b/anvil/src/winit.rs @@ -39,7 +39,7 @@ use smithay::{ wayland::{ compositor, dmabuf::{ - DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportError, + DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportNotifier, }, }, }; @@ -64,13 +64,16 @@ impl DmabufHandler for AnvilState { &mut self.backend_data.dmabuf_state.0 } - fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf) -> Result<(), ImportError> { - self.backend_data + fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) { + if self + .backend_data .backend .renderer() .import_dmabuf(&dmabuf, None) - .map(|_| ()) - .map_err(|_| ImportError::Failed) + .is_err() + { + notifier.failed(); + } } } delegate_dmabuf!(AnvilState); diff --git a/anvil/src/x11.rs b/anvil/src/x11.rs index c485ca57f9e6..cb2b37ff697d 100644 --- a/anvil/src/x11.rs +++ b/anvil/src/x11.rs @@ -44,7 +44,7 @@ use smithay::{ wayland::{ compositor, dmabuf::{ - DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportError, + DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportNotifier, }, }, }; @@ -73,12 +73,10 @@ impl DmabufHandler for AnvilState { &mut self.backend_data.dmabuf_state } - fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf) -> Result<(), ImportError> { - self.backend_data - .renderer - .import_dmabuf(&dmabuf, None) - .map(|_| ()) - .map_err(|_| ImportError::Failed) + fn dmabuf_imported(&mut self, _global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) { + if self.backend_data.renderer.import_dmabuf(&dmabuf, None).is_err() { + notifier.failed(); + } } } delegate_dmabuf!(AnvilState); diff --git a/src/wayland/dmabuf/dispatch.rs b/src/wayland/dmabuf/dispatch.rs index 33a6c9b7415c..4e7ceaf1bfa7 100644 --- a/src/wayland/dmabuf/dispatch.rs +++ b/src/wayland/dmabuf/dispatch.rs @@ -1,7 +1,5 @@ use std::sync::{atomic::AtomicBool, Mutex}; -use tracing::error; - use wayland_protocols::wp::linux_dmabuf::zv1::server::{ zwp_linux_buffer_params_v1, zwp_linux_dmabuf_feedback_v1, zwp_linux_dmabuf_v1, }; @@ -16,7 +14,7 @@ use crate::{ use super::{ DmabufData, DmabufFeedbackData, DmabufGlobal, DmabufGlobalData, DmabufHandler, DmabufParamsData, - DmabufState, ImportError, Modifier, SurfaceDmabufFeedbackState, + DmabufState, Import, ImportNotifier, Modifier, SurfaceDmabufFeedbackState, }; impl Dispatch for DmabufState @@ -224,7 +222,7 @@ where { fn request( state: &mut D, - client: &Client, + _client: &Client, params: &zwp_linux_buffer_params_v1::ZwpLinuxBufferParamsV1, request: zwp_linux_buffer_params_v1::Request, data: &DmabufParamsData, @@ -285,33 +283,15 @@ where // create_dmabuf performs an implicit ensure_unused function call. if let Some(dmabuf) = data.create_dmabuf(params, width, height, format, flags) { if state.dmabuf_state().globals.get(&data.id).is_some() { - match state.dmabuf_imported(&DmabufGlobal { id: data.id }, dmabuf.clone()) { - Ok(_) => { - match client.create_resource::(dh, 1, dmabuf) - { - Ok(buffer) => { - params.created(&buffer); - } - - Err(_) => { - error!("failed to create protocol object for \"create\" request"); - // Failed to import since the buffer protocol object could not be created. - params.failed(); - } - } - } - - Err(ImportError::InvalidFormat) => { - params.post_error( - zwp_linux_buffer_params_v1::Error::InvalidFormat, - "format and plane combination are not valid", - ); - } - - Err(ImportError::Failed) => { - params.failed(); - } - } + let notifier = ImportNotifier { + inner: params.clone(), + display: dh.clone(), + dmabuf: dmabuf.clone(), + import: Import::Falliable, + drop_ignore: false, + }; + + state.dmabuf_imported(&DmabufGlobal { id: data.id }, dmabuf, notifier); } else { // If the dmabuf global was destroyed, we cannot import any buffers. params.failed(); @@ -330,28 +310,18 @@ where // create_dmabuf performs an implicit ensure_unused function call. if let Some(dmabuf) = data.create_dmabuf(params, width, height, format, flags) { if state.dmabuf_state().globals.get(&data.id).is_some() { - match state.dmabuf_imported(&DmabufGlobal { id: data.id }, dmabuf.clone()) { - Ok(_) => { - // Import was successful, initialize the dmabuf data - data_init.init(buffer_id, dmabuf); - } - - Err(ImportError::InvalidFormat) => { - params.post_error( - zwp_linux_buffer_params_v1::Error::InvalidFormat, - "format and plane combination are not valid", - ); - } - - Err(ImportError::Failed) => { - // Buffer import failed. The protocol documentation heavily implies killing the - // client is the right thing to do here. - params.post_error( - zwp_linux_buffer_params_v1::Error::InvalidWlBuffer, - "buffer import failed", - ); - } - } + // The buffer isn't technically valid during data_init, but the client is not allowed to use the buffer until ready. + let buffer = data_init.init(buffer_id, dmabuf.clone()); + + let notifier = ImportNotifier { + inner: params.clone(), + display: dh.clone(), + dmabuf: dmabuf.clone(), + import: Import::Infallible(buffer), + drop_ignore: false, + }; + + state.dmabuf_imported(&DmabufGlobal { id: data.id }, dmabuf, notifier); } else { // Buffer import failed. The protocol documentation heavily implies killing the // client is the right thing to do here. diff --git a/src/wayland/dmabuf/mod.rs b/src/wayland/dmabuf/mod.rs index 5251eb8645bc..b90c634011f0 100644 --- a/src/wayland/dmabuf/mod.rs +++ b/src/wayland/dmabuf/mod.rs @@ -18,7 +18,7 @@ //! ```no_run //! use smithay::{ //! delegate_dmabuf, -//! backend::allocator::dmabuf::Dmabuf, +//! backend::allocator::dmabuf::{Dmabuf}, //! reexports::{ //! wayland_server::protocol::{ //! wl_buffer::WlBuffer, @@ -27,7 +27,7 @@ //! }, //! wayland::{ //! buffer::BufferHandler, -//! dmabuf::{DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportError} +//! dmabuf::{DmabufFeedback, DmabufFeedbackBuilder, DmabufGlobal, DmabufHandler, DmabufState, ImportNotifier} //! }, //! }; //! @@ -53,12 +53,12 @@ //! &mut self.dmabuf_state //! } //! -//! fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf) -> Result<(), ImportError> { +//! fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) { //! // Here you should import the dmabuf into your renderer. //! // -//! // The return value indicates whether import was successful. If the return value is Err, then -//! // the client is told dmabuf import has failed. -//! Ok(()) +//! // The notifier is used to communicate whether import was successful. In this example we +//! // call successful to notify the client import was successful. +//! notifier.successful::(); //! } //! //! fn new_surface_feedback( @@ -145,7 +145,7 @@ //! # reexports::{wayland_server::protocol::wl_buffer::WlBuffer}, //! # wayland::{ //! # buffer::BufferHandler, -//! # dmabuf::{DmabufGlobal, DmabufHandler, DmabufState, ImportError} +//! # dmabuf::{DmabufGlobal, DmabufHandler, DmabufState, ImportNotifier} //! # }, //! # }; //! # pub struct State { @@ -159,9 +159,7 @@ //! # fn dmabuf_state(&mut self) -> &mut DmabufState { //! # &mut self.dmabuf_state //! # } -//! # fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf) -> Result<(), ImportError> { -//! # Ok(()) -//! # } +//! # fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier) {} //! # } //! # delegate_dmabuf!(State); //! # let mut display = wayland_server::Display::::new().unwrap(); @@ -204,12 +202,16 @@ use std::{ use indexmap::IndexSet; use rustix::fs::{seek, SeekFrom}; use wayland_protocols::wp::linux_dmabuf::zv1::server::{ - zwp_linux_buffer_params_v1, zwp_linux_dmabuf_feedback_v1, zwp_linux_dmabuf_v1, + zwp_linux_buffer_params_v1::{self, ZwpLinuxBufferParamsV1}, + zwp_linux_dmabuf_feedback_v1, zwp_linux_dmabuf_v1, }; use wayland_server::{ - backend::GlobalId, - protocol::{wl_buffer, wl_surface::WlSurface}, - Client, DisplayHandle, GlobalDispatch, Resource, WEnum, + backend::{GlobalId, InvalidId}, + protocol::{ + wl_buffer::{self, WlBuffer}, + wl_surface::WlSurface, + }, + Client, Dispatch, DisplayHandle, GlobalDispatch, Resource, WEnum, }; use crate::{ @@ -839,6 +841,133 @@ pub struct DmabufGlobal { id: usize, } +/// An object to allow asynchronous creation of a [`Dmabuf`] backed [`WlBuffer`]. +/// +/// This object is [`Send`] to allow import of a [`Dmabuf`] to take place on another thread if desired. +#[must_use = "This object must be used to notify the client whether dmabuf import succeeded"] +#[derive(Debug)] +pub struct ImportNotifier { + inner: ZwpLinuxBufferParamsV1, + display: DisplayHandle, + dmabuf: Dmabuf, + import: Import, + drop_ignore: bool, +} + +/// Type of dmabuf import. +#[derive(Debug)] +enum Import { + /// The import can fail or create a WlBuffer. + Falliable, + + /// A WlBuffer object has already been created. Failure causes client death. + Infallible(WlBuffer), +} + +impl ImportNotifier { + /// Dmabuf import was successful. + /// + /// This can return [`InvalidId`] if the client the buffer was imported from has died. + pub fn successful(mut self) -> Result + where + D: Dispatch + + Dispatch + + BufferHandler + + DmabufHandler + + 'static, + { + let client = self.inner.client(); + + let result = match self.import { + Import::Falliable => { + if let Some(client) = client { + match client.create_resource::( + &self.display, + 1, + self.dmabuf.clone(), + ) { + Ok(buffer) => { + self.inner.created(&buffer); + Ok(buffer) + } + + Err(err) => { + tracing::error!("failed to create protocol object for \"create\" request"); + Err(err) + } + } + } else { + tracing::error!("client was dead while creating wl_buffer resource"); + self.inner.post_error( + zwp_linux_buffer_params_v1::Error::InvalidWlBuffer, + "create_immed failed and produced an invalid wl_buffer", + ); + Err(InvalidId) + } + } + Import::Infallible(ref buffer) => Ok(buffer.clone()), + }; + + self.drop_ignore = true; + result + } + + /// The buffer being imported is incomplete. + /// + /// This may be the result of too few or too many planes being used when creating a buffer. + pub fn incomplete(mut self) { + self.inner.post_error( + zwp_linux_buffer_params_v1::Error::Incomplete, + "missing or too many planes to create a buffer", + ); + self.drop_ignore = true; + } + + /// The buffer being imported has an invalid width or height. + pub fn invalid_dimensions(mut self) { + self.inner.post_error( + zwp_linux_buffer_params_v1::Error::InvalidDimensions, + "width or height of dmabuf is invalid", + ); + self.drop_ignore = true; + } + + /// Import failed due to an invalid format and plane combination. + /// + /// This is always a client error and will result in the client being killed. + pub fn invalid_format(mut self) { + self.inner.post_error( + zwp_linux_buffer_params_v1::Error::InvalidFormat, + "format and plane combination are not valid", + ); + self.drop_ignore = true; + } + + /// Import failed for an implementation dependent reason. + pub fn failed(mut self) { + if matches!(self.import, Import::Falliable) { + self.inner.failed(); + } else { + self.inner.post_error( + zwp_linux_buffer_params_v1::Error::InvalidWlBuffer, + "create_immed failed and produced an invalid wl_buffer", + ); + } + self.drop_ignore = true; + } +} + +impl Drop for ImportNotifier { + fn drop(&mut self) { + if !self.drop_ignore { + tracing::warn!( + "Compositor bug: Server ignored ImportNotifier for {:?}", + self.inner + ); + } + } +} + /// Handler trait for [`Dmabuf`] import from the compositor. pub trait DmabufHandler: BufferHandler { /// Returns a mutable reference to the [`DmabufState`] delegate type. @@ -849,13 +978,8 @@ pub trait DmabufHandler: BufferHandler { /// The `global` indicates which [`DmabufGlobal`] the buffer was imported to. You should import the dmabuf /// into your renderer to ensure the dmabuf may be used later when rendering. /// - /// The return value of this function indicates whether dmabuf import is successful. The renderer is - /// responsible for determining whether the format and plane combinations are valid and should return - /// [`ImportError::InvalidFormat`] if the format and planes are not correct. - /// - /// If the import fails due to an implementation specific reason, then [`ImportError::Failed`] should be - /// returned. - fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf) -> Result<(), ImportError>; + /// Whether dmabuf import succeded is notified through the [`ImportNotifier`] object provided in this function. + fn dmabuf_imported(&mut self, global: &DmabufGlobal, dmabuf: Dmabuf, notifier: ImportNotifier); /// This function allows to override the default [`DmabufFeedback`] for a surface /// @@ -873,23 +997,6 @@ pub trait DmabufHandler: BufferHandler { } } -/// Error that may occur when importing a [`Dmabuf`]. -#[derive(Debug, thiserror::Error)] -pub enum ImportError { - /// Buffer import failed for a renderer implementation specific reason. - /// - /// Depending on the request sent by the client, this error may notify the client that buffer import - /// failed or will kill the client. - #[error("buffer import failed")] - Failed, - - /// The format and plane combination is not valid. - /// - /// This specific error will kill the client providing the dmabuf. - #[error("format and plane combination is not valid")] - InvalidFormat, -} - /// Gets the contents of a [`Dmabuf`] backed [`WlBuffer`]. /// /// If the buffer is managed by the dmabuf handler, the [`Dmabuf`] is returned. @@ -940,7 +1047,7 @@ impl DmabufParamsData { /// Emits a protocol error if the params have already been used to create a dmabuf. /// /// This returns true if the protocol object has not been used. - fn ensure_unused(&self, params: &zwp_linux_buffer_params_v1::ZwpLinuxBufferParamsV1) -> bool { + fn ensure_unused(&self, params: &ZwpLinuxBufferParamsV1) -> bool { if !self.used.load(Ordering::Relaxed) { return true; } @@ -961,7 +1068,7 @@ impl DmabufParamsData { /// A return value of [`None`] indicates buffer import has failed and the client has been killed. fn create_dmabuf( &self, - params: &zwp_linux_buffer_params_v1::ZwpLinuxBufferParamsV1, + params: &ZwpLinuxBufferParamsV1, width: i32, height: i32, format: u32,