x11: emit errors when presentation fails

CHANGELOG.md

@@ -73,6 +73,7 @@
 
 - EGLBufferReader now checks if buffers are alive before using them.
 - LibSeat no longer panics on seat disable event.
+- X11 backend will report an error when trying to present a dmabuf fails.
 
 ### Anvil
 

src/backend/x11/buffer.rs

@@ -29,54 +29,23 @@
 
 use std::sync::atomic::Ordering;
 
-use super::{Window, X11Error};
+use super::{PresentError, Window, X11Error};
 use drm_fourcc::DrmFourcc;
 use nix::fcntl;
-use x11rb::connection::Connection;
+use x11rb::{
-use x11rb::protocol::dri3::ConnectionExt as _;
+    connection::Connection,
-use x11rb::protocol::present::{self, ConnectionExt};
+    protocol::{
-use x11rb::protocol::xproto::PixmapWrapper;
+        dri3::ConnectionExt as _,
-use x11rb::rust_connection::{ConnectionError, ReplyOrIdError};
+        present::{self, ConnectionExt},
-use x11rb::utils::RawFdContainer;
+        xproto::PixmapWrapper,
+    },
+    utils::RawFdContainer,
+};
 
-use crate::backend::allocator::dmabuf::Dmabuf;
+use crate::backend::allocator::{dmabuf::Dmabuf, Buffer};
-use crate::backend::allocator::Buffer;
 
 // Shm can be easily supported in the future using, xcb_shm_create_pixmap.
 
-#[derive(Debug, thiserror::Error)]
-pub enum CreatePixmapError {
-    #[error("An x11 protocol error occured")]
-    Protocol(X11Error),
-
-    #[error("The Dmabuf had too many planes")]
-    TooManyPlanes,
-
-    #[error("Duplicating the file descriptors for the dmabuf handles failed")]
-    DupFailed(String),
-
-    #[error("Buffer had incorrect format, expected: {0}")]
-    IncorrectFormat(DrmFourcc),
-}
-
-impl From<X11Error> for CreatePixmapError {
-    fn from(e: X11Error) -> Self {
-        CreatePixmapError::Protocol(e)
-    }
-}
-
-impl From<ReplyOrIdError> for CreatePixmapError {
-    fn from(e: ReplyOrIdError) -> Self {
-        X11Error::from(e).into()
-    }
-}
-
-impl From<ConnectionError> for CreatePixmapError {
-    fn from(e: ConnectionError) -> Self {
-        X11Error::from(e).into()
-    }
-}
-
 pub trait PixmapWrapperExt<'c, C>
 where
     C: Connection,
@@ -88,7 +57,7 @@ where
         connection: &'c C,
         window: &Window,
         dmabuf: &Dmabuf,
-    ) -> Result<PixmapWrapper<'c, C>, CreatePixmapError>;
+    ) -> Result<PixmapWrapper<'c, C>, X11Error>;
 
     /// Presents the pixmap to the window.
     ///
@@ -108,9 +77,9 @@ where
         connection: &'c C,
         window: &Window,
         dmabuf: &Dmabuf,
-    ) -> Result<PixmapWrapper<'c, C>, CreatePixmapError> {
+    ) -> Result<PixmapWrapper<'c, C>, X11Error> {
         if dmabuf.format().code != window.format() {
-            return Err(CreatePixmapError::IncorrectFormat(window.format()));
+            return Err(PresentError::IncorrectFormat(window.format()).into());
         }
 
         let mut fds = Vec::new();
@@ -121,7 +90,7 @@ where
                 handle,
                 fcntl::FcntlArg::F_DUPFD_CLOEXEC(3), // Set to 3 so the fd cannot become stdin, stdout or stderr
             )
-            .map_err(|e| CreatePixmapError::DupFailed(e.to_string()))?;
+            .map_err(|e| PresentError::DupFailed(e.to_string()))?;
 
             fds.push(RawFdContainer::new(fd))
         }
@@ -129,7 +98,7 @@ where
         // We need dri3 >= 1.2 in order to use the enhanced dri3_pixmap_from_buffers function.
         let xid = if window.0.extensions.dri3 >= Some((1, 2)) {
             if dmabuf.num_planes() > 4 {
-                return Err(CreatePixmapError::TooManyPlanes);
+                return Err(PresentError::TooManyPlanes.into());
             }
 
             let xid = connection.generate_id()?;
@@ -165,7 +134,7 @@ where
         } else {
             // Old codepath can only create a pixmap using one plane from a dmabuf.
             if dmabuf.num_planes() != 1 {
-                return Err(CreatePixmapError::TooManyPlanes);
+                return Err(PresentError::TooManyPlanes.into());
             }
 
             let xid = connection.generate_id()?;

src/backend/x11/error.rs

@@ -6,24 +6,26 @@ use x11rb::rust_connection::{ConnectError, ConnectionError, ReplyError, ReplyOrI
 
 use crate::backend::{allocator::gbm::GbmConvertError, drm::CreateDrmNodeError};
 
+use super::PresentError;
+
 /// An error emitted by the X11 backend during setup.
 #[derive(Debug, thiserror::Error)]
 pub enum X11Error {
     /// Connecting to the X server failed.
     #[error("Connecting to the X server failed")]
-    ConnectionFailed(ConnectError),
+    ConnectionFailed(#[from] ConnectError),
 
     /// A required X11 extension was not present or has the right version.
     #[error("{0}")]
-    MissingExtension(MissingExtensionError),
+    MissingExtension(#[from] MissingExtensionError),
 
     /// Some protocol error occurred during setup.
     #[error("Some protocol error occurred during setup")]
-    Protocol(ReplyOrIdError),
+    Protocol(#[from] ReplyOrIdError),
 
     /// Creating the window failed.
     #[error("Creating the window failed")]
-    CreateWindow(CreateWindowError),
+    CreateWindow(#[from] CreateWindowError),
 
     /// An X11 surface already exists for this window.
     #[error("An X11 surface already exists for this window")]
@@ -42,13 +44,11 @@ pub enum X11Error {
 
     /// Failed to allocate buffers needed to present to the window.
     #[error("Failed to allocate buffers needed to present to the window")]
-    Allocation(AllocateBuffersError),
+    Allocation(#[from] AllocateBuffersError),
-}
 
-impl From<ConnectError> for X11Error {
+    /// Error while presenting to a window.
-    fn from(err: ConnectError) -> Self {
+    #[error(transparent)]
-        Self::ConnectionFailed(err)
+    Present(#[from] PresentError),
-    }
 }
 
 impl From<ReplyError> for X11Error {
@@ -63,12 +63,6 @@ impl From<ConnectionError> for X11Error {
     }
 }
 
-impl From<ReplyOrIdError> for X11Error {
-    fn from(err: ReplyOrIdError) -> Self {
-        Self::Protocol(err)
-    }
-}
-
 /// An error that occurs when a required X11 extension is not present.
 #[derive(Debug, thiserror::Error)]
 pub enum MissingExtensionError {
@@ -99,12 +93,6 @@ pub enum MissingExtensionError {
     },
 }
 
-impl From<MissingExtensionError> for X11Error {
-    fn from(err: MissingExtensionError) -> Self {
-        Self::MissingExtension(err)
-    }
-}
-
 /// An error which may occur when creating an X11 window.
 #[derive(Debug, thiserror::Error)]
 pub enum CreateWindowError {
@@ -117,12 +105,6 @@ pub enum CreateWindowError {
     NoVisual,
 }
 
-impl From<CreateWindowError> for X11Error {
-    fn from(err: CreateWindowError) -> Self {
-        Self::CreateWindow(err)
-    }
-}
-
 /// An error which may occur when allocating buffers for presentation to the window.
 #[derive(Debug, thiserror::Error)]
 pub enum AllocateBuffersError {
@@ -165,9 +147,3 @@ impl From<CreateDrmNodeError> for AllocateBuffersError {
         }
     }
 }
-
-impl From<AllocateBuffersError> for X11Error {
-    fn from(err: AllocateBuffersError) -> Self {
-        Self::Allocation(err)
-    }
-}

src/backend/x11/surface.rs

@@ -19,6 +19,24 @@ use crate::{
     utils::{Logical, Size},
 };
 
+use super::X11Error;
+
+/// An error that may occur when presenting.
+#[derive(Debug, thiserror::Error)]
+pub enum PresentError {
+    /// The dmabuf being presented has too many planes.
+    #[error("The Dmabuf had too many planes")]
+    TooManyPlanes,
+
+    /// Duplicating the dmabuf handles failed.
+    #[error("Duplicating the file descriptors for the dmabuf handles failed")]
+    DupFailed(String),
+
+    /// The format dmabuf presented does not match the format of the window.
+    #[error("Buffer had incorrect format, expected: {0}")]
+    IncorrectFormat(DrmFourcc),
+}
+
 /// An X11 surface which uses GBM to allocate and present buffers.
 #[derive(Debug)]
 pub struct X11Surface {
@@ -67,7 +85,7 @@ impl X11Surface {
     /// or [`reset_buffers`](Self::reset_buffer) is used to reset the buffers.
     pub fn buffer(&mut self) -> Result<(Dmabuf, u8), AllocateBuffersError> {
         if let Some(new_size) = self.resize.try_iter().last() {
-            self.resize(new_size)?;
+            self.resize(new_size);
         }
 
         if self.buffer.is_none() {
@@ -92,7 +110,7 @@ impl X11Surface {
     }
 
     /// Consume and submit the buffer to the window.
-    pub fn submit(&mut self) -> Result<(), AllocateBuffersError> {
+    pub fn submit(&mut self) -> Result<(), X11Error> {
         if let Some(connection) = self.connection.upgrade() {
             // Get a new buffer
             let mut next = self
@@ -107,15 +125,14 @@ impl X11Surface {
             }
 
             let window = self.window().ok_or(AllocateBuffersError::WindowDestroyed)?;
-
+            let pixmap = PixmapWrapper::with_dmabuf(
-            if let Ok(pixmap) = PixmapWrapper::with_dmabuf(
                 &*connection,
                 window.as_ref(),
                 next.userdata().get::<Dmabuf>().unwrap(),
-            ) {
+            )?;
+
             // Now present the current buffer
-                let _ = pixmap.present(&*connection, window.as_ref());
+            let _ = pixmap.present(&*connection, window.as_ref())?;
-            }
             self.swapchain.submitted(next);
 
             // Flush the connection after presenting to the window to ensure we don't run out of buffer space in the X11 connection.
@@ -130,13 +147,11 @@ impl X11Surface {
         self.buffer = None;
     }
 
-    fn resize(&mut self, size: Size<u16, Logical>) -> Result<(), AllocateBuffersError> {
+    fn resize(&mut self, size: Size<u16, Logical>) {
         self.swapchain.resize(size.w as u32, size.h as u32);
         self.buffer = None;
 
         self.width = size.w;
         self.height = size.h;
-
-        Ok(())
     }
 }