x11: Use swapchain and expose age

Refactors the `X11Surface` to use the `Swapchain` internally,
which gives us a buffer age implemenation, which we can then use
to change the `Present` struct of the X11 backend to provide age
values to the user.

This change also merged the `Present` struct into the surface
because submits should now happen explicitly via a new `submit`-call
instead of being submitted implicitly on drop.
This allows the buffer to be discarded if rendering fails.

CHANGELOG.md

@@ -27,6 +27,9 @@
 - 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
+- `Present` was merged into the `X11Surface`
+- `X11Surface::buffer` now additionally returns the age of the buffer
+- `X11Surface` now has an explicit `submit` function
 
 ### Additions
 

anvil/src/x11.rs

@@ -204,122 +204,116 @@ pub fn run_x11(log: Logger) {
         if state.backend_data.render {
             state.backend_data.render = false;
             let backend_data = &mut state.backend_data;
+            let mut renderer = renderer.borrow_mut();
 
-            match backend_data.surface.present() {
+            // We need to borrow everything we want to refer to inside the renderer callback otherwise rustc is unhappy.
-                Ok(present) => {
+            let window_map = state.window_map.borrow();
-                    let mut renderer = renderer.borrow_mut();
+            let (x, y) = state.pointer_location.into();
+            let dnd_icon = &state.dnd_icon;
+            let cursor_status = &state.cursor_status;
+            #[cfg(feature = "debug")]
+            let fps = backend_data.fps.avg().round() as u32;
+            #[cfg(feature = "debug")]
+            let fps_texture = &backend_data.fps_texture;
 
-                    // We need to borrow everything we want to refer to inside the renderer callback otherwise rustc is unhappy.
+            let (buffer, _age) = backend_data.surface.buffer().expect("gbm device was destroyed");
-                    let window_map = state.window_map.borrow();
+            if let Err(err) = renderer.bind(buffer) {
-                    let (x, y) = state.pointer_location.into();
+                error!(log, "Error while binding buffer: {}", err);
-                    let dnd_icon = &state.dnd_icon;
+            }
-                    let cursor_status = &state.cursor_status;
-                    #[cfg(feature = "debug")]
-                    let fps = backend_data.fps.avg().round() as u32;
-                    #[cfg(feature = "debug")]
-                    let fps_texture = &backend_data.fps_texture;
 
-                    if let Err(err) = renderer.bind(present.buffer().expect("gbm device was destroyed")) {
+            // drawing logic
-                        error!(log, "Error while binding buffer: {}", err);
+            match renderer
-                    }
+                // X11 scanout for a Dmabuf is upside down
+                // TODO: Address this issue in renderer.
+                .render(
+                    backend_data.mode.size,
+                    Transform::Flipped180,
+                    |renderer, frame| {
+                        render_layers_and_windows(
+                            renderer,
+                            frame,
+                            &*window_map,
+                            output_geometry,
+                            output_scale,
+                            &log,
+                        )?;
 
-                    // drawing logic
+                        // draw the dnd icon if any
-                    match renderer
+                        {
-                        // X11 scanout for a Dmabuf is upside down
+                            let guard = dnd_icon.lock().unwrap();
-                        // TODO: Address this issue in renderer.
+                            if let Some(ref surface) = *guard {
-                        .render(
+                                if surface.as_ref().is_alive() {
-                            backend_data.mode.size,
+                                    draw_dnd_icon(
-                            Transform::Flipped180,
+                                        renderer,
-                            |renderer, frame| {
+                                        frame,
-                                render_layers_and_windows(
+                                        surface,
+                                        (x as i32, y as i32).into(),
+                                        output_scale,
+                                        &log,
+                                    )?;
+                                }
+                            }
+                        }
+
+                        // draw the cursor as relevant
+                        {
+                            let mut guard = cursor_status.lock().unwrap();
+                            // reset the cursor if the surface is no longer alive
+                            let mut reset = false;
+
+                            if let CursorImageStatus::Image(ref surface) = *guard {
+                                reset = !surface.as_ref().is_alive();
+                            }
+
+                            if reset {
+                                *guard = CursorImageStatus::Default;
+                            }
+
+                            // draw as relevant
+                            if let CursorImageStatus::Image(ref surface) = *guard {
+                                cursor_visible = false;
+                                draw_cursor(
                                     renderer,
                                     frame,
-                                    &*window_map,
+                                    surface,
-                                    output_geometry,
+                                    (x as i32, y as i32).into(),
                                     output_scale,
                                     &log,
                                 )?;
-
+                            } else {
-                                // draw the dnd icon if any
+                                cursor_visible = true;
-                                {
-                                    let guard = dnd_icon.lock().unwrap();
-                                    if let Some(ref surface) = *guard {
-                                        if surface.as_ref().is_alive() {
-                                            draw_dnd_icon(
-                                                renderer,
-                                                frame,
-                                                surface,
-                                                (x as i32, y as i32).into(),
-                                                output_scale,
-                                                &log,
-                                            )?;
-                                        }
-                                    }
-                                }
-
-                                // draw the cursor as relevant
-                                {
-                                    let mut guard = cursor_status.lock().unwrap();
-                                    // reset the cursor if the surface is no longer alive
-                                    let mut reset = false;
-
-                                    if let CursorImageStatus::Image(ref surface) = *guard {
-                                        reset = !surface.as_ref().is_alive();
-                                    }
-
-                                    if reset {
-                                        *guard = CursorImageStatus::Default;
-                                    }
-
-                                    // draw as relevant
-                                    if let CursorImageStatus::Image(ref surface) = *guard {
-                                        cursor_visible = false;
-                                        draw_cursor(
-                                            renderer,
-                                            frame,
-                                            surface,
-                                            (x as i32, y as i32).into(),
-                                            output_scale,
-                                            &log,
-                                        )?;
-                                    } else {
-                                        cursor_visible = true;
-                                    }
-                                }
-
-                                #[cfg(feature = "debug")]
-                                {
-                                    use crate::drawing::draw_fps;
-
-                                    draw_fps(renderer, frame, fps_texture, output_scale as f64, fps)?;
-                                }
-
-                                Ok(())
-                            },
-                        )
-                        .map_err(Into::<SwapBuffersError>::into)
-                        .and_then(|x| x)
-                        .map_err(Into::<SwapBuffersError>::into)
-                    {
-                        Ok(()) => {
-                            // Unbind the buffer and now let the scope end to present.
-                            if let Err(err) = renderer.unbind() {
-                                error!(log, "Error while unbinding buffer: {}", err);
                             }
                         }
 
-                        Err(err) => {
+                        #[cfg(feature = "debug")]
-                            if let SwapBuffersError::ContextLost(err) = err {
+                        {
-                                error!(log, "Critical Rendering Error: {}", err);
+                            use crate::drawing::draw_fps;
-                                state.running.store(false, Ordering::SeqCst);
+
-                            }
+                            draw_fps(renderer, frame, fps_texture, output_scale as f64, fps)?;
                         }
+
+                        Ok(())
+                    },
+                )
+                .map_err(Into::<SwapBuffersError>::into)
+                .and_then(|x| x)
+                .map_err(Into::<SwapBuffersError>::into)
+            {
+                Ok(()) => {
+                    // Unbind the buffer
+                    if let Err(err) = renderer.unbind() {
+                        error!(log, "Error while unbinding buffer: {}", err);
                     }
+
+                    // Submit the buffer
+                    backend_data.surface.submit();
                 }
 
                 Err(err) => {
-                    error!(log, "Failed to allocate buffers to present to window: {}", err);
+                    if let SwapBuffersError::ContextLost(err) = err {
-                    state.running.store(false, Ordering::SeqCst);
+                        error!(log, "Critical Rendering Error: {}", err);
+                        state.running.store(false, Ordering::SeqCst);
+                    }
                 }
             }
 
@@ -328,10 +322,9 @@ pub fn run_x11(log: Logger) {
             window.set_cursor_visible(cursor_visible);
 
             // Send frame events so that client start drawing their next frame
-            state
+            window_map.send_frames(start_time.elapsed().as_millis() as u32);
-                .window_map
+            std::mem::drop(window_map);
-                .borrow()
+
-                .send_frames(start_time.elapsed().as_millis() as u32);
             display.borrow_mut().flush_clients(&mut state);
         }
 

src/backend/x11/error.rs

@@ -134,6 +134,10 @@ pub enum AllocateBuffersError {
     /// Exporting a dmabuf failed.
     #[error("Exporting a dmabuf failed.")]
     ExportDmabuf(#[from] GbmConvertError),
+
+    /// No free slots
+    #[error("No free slots in the swapchain")]
+    NoFreeSlots,
 }
 
 impl From<Errno> for AllocateBuffersError {

src/backend/x11/mod.rs

@@ -76,7 +76,10 @@ mod window_inner;
 use self::{buffer::PixmapWrapperExt, window_inner::WindowInner};
 use crate::{
     backend::{
-        allocator::dmabuf::{AsDmabuf, Dmabuf},
+        allocator::{
+            dmabuf::{AsDmabuf, Dmabuf},
+            Slot, Swapchain,
+        },
         drm::{node::path_to_type, CreateDrmNodeError, DrmNode, NodeType},
         egl::{native::X11DefaultDisplay, EGLDevice, EGLDisplay, Error as EGLError},
         input::{Axis, ButtonState, InputEvent, KeyState},
@@ -85,7 +88,7 @@ use crate::{
 };
 use calloop::{EventSource, Poll, PostAction, Readiness, Token, TokenFactory};
 use drm_fourcc::{DrmFourcc, DrmModifier};
-use gbm::{BufferObject, BufferObjectFlags};
+use gbm::BufferObject;
 use nix::{
     fcntl::{self, OFlag},
     sys::stat::Mode,
@@ -342,12 +345,12 @@ pub struct X11Surface {
     connection: Weak<RustConnection>,
     window: Window,
     resize: Receiver<Size<u16, Logical>>,
-    device: gbm::Device<DrmNode>,
+    swapchain: Swapchain<gbm::Device<DrmNode>, BufferObject<()>>,
     format: DrmFourcc,
     width: u16,
     height: u16,
-    current: BufferObject<Dmabuf>,
+    current: Option<Slot<BufferObject<()>>>,
-    next: BufferObject<Dmabuf>,
+    next: Option<Slot<BufferObject<()>>>,
 }
 
 #[derive(Debug, thiserror::Error)]
@@ -451,28 +454,7 @@ impl X11Surface {
         let window = backend.window();
         let format = window.format().unwrap();
         let size = window.size();
-        let mut current = device
+        let swapchain = Swapchain::new(device, size.w as u32, size.h as u32, format, modifiers);
-            .create_buffer_object_with_modifiers(
-                size.w as u32,
-                size.h as u32,
-                format,
-                modifiers.iter().cloned(),
-            )
-            .map_err(Into::<AllocateBuffersError>::into)?;
-        current
-            .set_userdata(current.export().map_err(Into::<AllocateBuffersError>::into)?)
-            .map_err(Into::<AllocateBuffersError>::into)?;
-
-        let mut next = device
-            .create_buffer_object_with_modifiers(
-                size.w as u32,
-                size.h as u32,
-                format,
-                modifiers.iter().cloned(),
-            )
-            .map_err(Into::<AllocateBuffersError>::into)?;
-        next.set_userdata(next.export().map_err(Into::<AllocateBuffersError>::into)?)
-            .map_err(Into::<AllocateBuffersError>::into)?;
 
         let (sender, recv) = mpsc::channel();
 
@@ -481,19 +463,19 @@ impl X11Surface {
         Ok(X11Surface {
             connection: Arc::downgrade(&backend.connection),
             window,
-            device,
+            swapchain,
             format,
             width: size.w,
             height: size.h,
-            current,
+            current: None,
-            next,
+            next: None,
             resize: recv,
         })
     }
 
     /// Returns a handle to the GBM device used to allocate buffers.
     pub fn device(&self) -> &gbm::Device<DrmNode> {
-        &self.device
+        &self.swapchain.allocator
     }
 
     /// Returns the format of the buffers the surface accepts.
@@ -501,115 +483,79 @@ impl X11Surface {
         self.format
     }
 
-    /// Returns an RAII scoped object which provides the next buffer.
+    /// Returns the next buffer that will be presented to the Window and its age.
     ///
-    /// When the object is dropped, the contents of the buffer are swapped and then presented.
+    /// You may bind this buffer to a renderer to render.
-    pub fn present(&mut self) -> Result<Present<'_>, AllocateBuffersError> {
+    /// This function will return the same buffer until [`submit`](Self::submit) is called
+    /// 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)?;
         }
 
-        Ok(Present { surface: self })
+        if self.next.is_none() {
+            self.next = Some(
+                self.swapchain
+                    .acquire()
+                    .map_err(Into::<AllocateBuffersError>::into)?
+                    .ok_or(AllocateBuffersError::NoFreeSlots)?,
+            );
+        }
+
+        let slot = self.next.as_ref().unwrap();
+        let age = slot.age();
+        match slot.userdata().get::<Dmabuf>() {
+            Some(dmabuf) => Ok((dmabuf.clone(), age)),
+            None => {
+                let dmabuf = slot.export().map_err(Into::<AllocateBuffersError>::into)?;
+                slot.userdata().insert_if_missing(|| dmabuf.clone());
+                Ok((dmabuf, age))
+            }
+        }
     }
 
-    fn resize(&mut self, size: Size<u16, Logical>) -> Result<(), AllocateBuffersError> {
+    /// Consume and submit the buffer to the window.
-        let mut current = self
+    pub fn submit(&mut self) {
-            .device
+        if let Some(connection) = self.connection.upgrade() {
-            .create_buffer_object(
-                size.w as u32,
-                size.h as u32,
-                self.format,
-                BufferObjectFlags::empty(),
-            )
-            .map_err(Into::<AllocateBuffersError>::into)?;
-        current
-            .set_userdata(current.export().map_err(Into::<AllocateBuffersError>::into)?)
-            .map_err(Into::<AllocateBuffersError>::into)?;
-
-        let mut next = self
-            .device
-            .create_buffer_object(
-                size.w as u32,
-                size.h as u32,
-                self.format,
-                BufferObjectFlags::empty(),
-            )
-            .map_err(Into::<AllocateBuffersError>::into)?;
-        next.set_userdata(next.export().map_err(Into::<AllocateBuffersError>::into)?)
-            .map_err(Into::<AllocateBuffersError>::into)?;
-
-        self.width = size.w;
-        self.height = size.h;
-        self.current = current;
-        self.next = next;
-
-        Ok(())
-    }
-}
-
-/// An RAII scope containing the next buffer that will be presented to the window. Presentation
-/// occurs when the `Present` is dropped.
-///
-/// The provided buffer may be bound to a [Renderer](crate::backend::renderer::Renderer) to draw to
-/// the window.
-///
-/// ```rust,ignore
-/// // Instantiate a new present object to start the process of presenting.
-/// let present = surface.present()?;
-///
-/// // Bind the buffer to the renderer in order to render.
-/// renderer.bind(present.buffer())?;
-///
-/// // Rendering here!
-///
-/// // Make sure to unbind the buffer when done.
-/// renderer.unbind()?;
-///
-/// // When the `present` is dropped, what was rendered will be presented to the window.
-/// ```
-#[derive(Debug)]
-pub struct Present<'a> {
-    surface: &'a mut X11Surface,
-}
-
-impl Present<'_> {
-    /// Returns the next buffer that will be presented to the Window.
-    ///
-    /// You may bind this buffer to a renderer to render.
-    pub fn buffer(&self) -> Result<Dmabuf, AllocateBuffersError> {
-        Ok(self
-            .surface
-            .next
-            .userdata()
-            .map(|dmabuf| dmabuf.cloned())
-            .map(Option::unwrap)?)
-    }
-}
-
-impl Drop for Present<'_> {
-    fn drop(&mut self) {
-        let surface = &mut self.surface;
-
-        if let Some(connection) = surface.connection.upgrade() {
             // Swap the buffers
-            mem::swap(&mut surface.next, &mut surface.current);
+            if let Some(mut next) = self.next.take() {
+                if let Some(current) = self.current.as_mut() {
+                    mem::swap(&mut next, current);
+                    self.swapchain.submitted(next);
+                } else {
+                    self.current = Some(next);
+                }
+            }
 
-            match surface.current.userdata().map(Option::unwrap) {
+            if let Ok(pixmap) = PixmapWrapper::with_dmabuf(
-                Ok(dmabuf) => {
+                &*connection,
-                    if let Ok(pixmap) = PixmapWrapper::with_dmabuf(&*connection, &surface.window, dmabuf) {
+                &self.window,
-                        // Now present the current buffer
+                self.current.as_ref().unwrap().userdata().get::<Dmabuf>().unwrap(),
-                        let _ = pixmap.present(&*connection, &surface.window);
+            ) {
-                    }
+                // Now present the current buffer
-                }
+                let _ = pixmap.present(&*connection, &self.window);
-                Err(_err) => {
-                    todo!("Log error")
-                }
             }
 
             // Flush the connection after presenting to the window to ensure we don't run out of buffer space in the X11 connection.
             let _ = connection.flush();
         }
     }
+
+    /// Resets the internal buffers, e.g. to reset age values
+    pub fn reset_buffers(&mut self) {
+        self.swapchain.reset_buffers();
+        self.next = None;
+    }
+
+    fn resize(&mut self, size: Size<u16, Logical>) -> Result<(), AllocateBuffersError> {
+        self.swapchain.resize(size.w as u32, size.h as u32);
+        self.next = None;
+
+        self.width = size.w;
+        self.height = size.h;
+
+        Ok(())
+    }
 }
 
 /// An X11 window.