m: Don't use the borrowing trick under X11 (#86)

Cargo.toml

@@ -12,6 +12,10 @@ categories = ["game-development", "graphics", "gui", "multimedia", "rendering"]
 exclude = ["examples"]
 rust-version = "1.64.0"
 
+[[bench]]
+name = "buffer_mut"
+harness = false
+
 [features]
 default = ["x11", "wayland", "wayland-dlopen"]
 wayland = ["wayland-backend", "wayland-client", "memmap2", "nix", "fastrand"]
@@ -62,6 +66,7 @@ redox_syscall = "0.3"
 cfg_aliases = "0.1.1"
 
 [dev-dependencies]
+criterion = { version = "0.4.0", default-features = false, features = ["cargo_bench_support"] }
 instant = "0.1.12"
 winit = "0.28.1"
 

benches/buffer_mut.rs

@@ -0,0 +1,65 @@
+use criterion::{criterion_group, criterion_main, Criterion};
+
+fn buffer_mut(c: &mut Criterion) {
+    #[cfg(any(target_arch = "wasm32", target_arch = "wasm64"))]
+    {
+        // Do nothing.
+        let _ = c;
+    }
+
+    #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))]
+    {
+        use criterion::black_box;
+        use softbuffer::{Context, Surface};
+        use std::num::NonZeroU32;
+        use winit::platform::run_return::EventLoopExtRunReturn;
+
+        let mut evl = winit::event_loop::EventLoop::new();
+        let window = winit::window::WindowBuilder::new()
+            .with_visible(false)
+            .build(&evl)
+            .unwrap();
+
+        evl.run_return(move |ev, elwt, control_flow| {
+            control_flow.set_poll();
+
+            if let winit::event::Event::RedrawEventsCleared = ev {
+                control_flow.set_exit();
+
+                let mut surface = unsafe {
+                    let context = Context::new(elwt).unwrap();
+                    Surface::new(&context, &window).unwrap()
+                };
+
+                let size = window.inner_size();
+                surface
+                    .resize(
+                        NonZeroU32::new(size.width).unwrap(),
+                        NonZeroU32::new(size.height).unwrap(),
+                    )
+                    .unwrap();
+
+                c.bench_function("buffer_mut()", |b| {
+                    b.iter(|| {
+                        for _ in 0..500 {
+                            black_box(surface.buffer_mut().unwrap());
+                        }
+                    });
+                });
+
+                c.bench_function("pixels_mut()", |b| {
+                    let mut buffer = surface.buffer_mut().unwrap();
+                    b.iter(|| {
+                        for _ in 0..500 {
+                            let x: &mut [u32] = &mut buffer;
+                            black_box(x);
+                        }
+                    });
+                });
+            }
+        });
+    }
+}
+
+criterion_group!(benches, buffer_mut);
+criterion_main!(benches);

src/x11.rs

@@ -6,11 +6,11 @@
 #![allow(clippy::uninlined_format_args)]
 
 use crate::error::SwResultExt;
-use crate::{util, SoftBufferError};
+use crate::SoftBufferError;
 use nix::libc::{shmat, shmctl, shmdt, shmget, IPC_PRIVATE, IPC_RMID};
 use raw_window_handle::{XcbDisplayHandle, XcbWindowHandle, XlibDisplayHandle, XlibWindowHandle};
 use std::ptr::{null_mut, NonNull};
-use std::{fmt, io, mem, num::NonZeroU32, rc::Rc};
+use std::{fmt, io, mem, num::NonZeroU32, rc::Rc, slice};
 
 use x11_dl::xlib::Display;
 use x11_dl::xlib_xcb::Xlib_xcb;
@@ -34,8 +34,6 @@ impl X11DisplayImpl {
     pub(crate) unsafe fn from_xlib(
         display_handle: XlibDisplayHandle,
     ) -> Result<X11DisplayImpl, SoftBufferError> {
-        // TODO: We should cache the shared libraries.
-
         // Try to open the XlibXCB shared library.
         let lib_xcb = Xlib_xcb::open().swbuf_err("Failed to open XlibXCB shared library")?;
 
@@ -273,34 +271,32 @@ impl X11Impl {
     pub(crate) fn buffer_mut(&mut self) -> Result<BufferImpl, SoftBufferError> {
         log::trace!("buffer_mut: window={:X}", self.window);
 
-        Ok(BufferImpl(util::BorrowStack::new(self, |surface| {
+        // Finish waiting on the previous `shm::PutImage` request, if any.
-            let buffer = surface
+        self.buffer.finish_wait(&self.display.connection)?;
-                .buffer
-                .buffer_mut(&surface.display.connection)
-                .swbuf_err("Failed to get mutable X11 buffer")?;
 
-            // Crop it down to the window size.
+        // We can now safely call `buffer_mut` on the buffer.
-            Ok(&mut buffer[..total_len(surface.width, surface.height) / 4])
+        Ok(BufferImpl(self))
-        })?))
     }
 }
 
-pub struct BufferImpl<'a>(util::BorrowStack<'a, X11Impl, [u32]>);
+pub struct BufferImpl<'a>(&'a mut X11Impl);
 
 impl<'a> BufferImpl<'a> {
     #[inline]
     pub fn pixels(&self) -> &[u32] {
-        self.0.member()
+        // SAFETY: We called `finish_wait` on the buffer, so it is safe to call `buffer()`.
+        unsafe { self.0.buffer.buffer() }
     }
 
     #[inline]
     pub fn pixels_mut(&mut self) -> &mut [u32] {
-        self.0.member_mut()
+        // SAFETY: We called `finish_wait` on the buffer, so it is safe to call `buffer_mut`.
+        unsafe { self.0.buffer.buffer_mut() }
     }
 
     /// Push the buffer to the window.
     pub fn present(self) -> Result<(), SoftBufferError> {
-        let imp = self.0.into_container();
+        let imp = self.0;
 
         log::trace!("present: window={:X}", imp.window);
 
@@ -329,39 +325,37 @@ impl<'a> BufferImpl<'a> {
 
             Buffer::Shm(ref mut shm) => {
                 // If the X server is still processing the last image, wait for it to finish.
-                shm.finish_wait(&imp.display.connection)
+                // SAFETY: We know that we called finish_wait() before this.
-                    .and_then(|()| {
+                // Put the image into the window.
-                        // Put the image into the window.
+                if let Some((_, segment_id)) = shm.seg {
-                        if let Some((_, segment_id)) = shm.seg {
+                    imp.display
-                            imp.display
+                        .connection
-                                .connection
+                        .shm_put_image(
-                                .shm_put_image(
+                            imp.window,
-                                    imp.window,
+                            imp.gc,
-                                    imp.gc,
+                            imp.width,
-                                    imp.width,
+                            imp.height,
-                                    imp.height,
+                            0,
-                                    0,
+                            0,
-                                    0,
+                            imp.width,
-                                    imp.width,
+                            imp.height,
-                                    imp.height,
+                            0,
-                                    0,
+                            0,
-                                    0,
+                            imp.depth,
-                                    imp.depth,
+                            xproto::ImageFormat::Z_PIXMAP.into(),
-                                    xproto::ImageFormat::Z_PIXMAP.into(),
+                            false,
-                                    false,
+                            segment_id,
-                                    segment_id,
+                            0,
-                                    0,
+                        )
-                                )
+                        .push_err()
-                                .push_err()
+                        .map(|c| c.ignore_error())
-                                .map(|c| c.ignore_error())
+                        .and_then(|()| {
-                        } else {
+                            // Send a short request to act as a notification for when the X server is done processing the image.
-                            Ok(())
+                            shm.begin_wait(&imp.display.connection)
-                        }
+                        })
-                    })
+                } else {
-                    .and_then(|()| {
+                    Ok(())
-                        // Send a short request to act as a notification for when the X server is done processing the image.
+                }
-                        shm.begin_wait(&imp.display.connection)
-                    })
             }
         };
 
@@ -386,11 +380,39 @@ impl Buffer {
         }
     }
 
-    /// Get a mutable reference to the buffer.
+    /// Finish waiting for an ongoing `shm::PutImage` request, if there is one.
-    fn buffer_mut(&mut self, conn: &impl Connection) -> Result<&mut [u32], PushBufferError> {
+    fn finish_wait(&mut self, conn: &impl Connection) -> Result<(), SoftBufferError> {
+        if let Buffer::Shm(ref mut shm) = self {
+            shm.finish_wait(conn)
+                .swbuf_err("Failed to wait for X11 buffer")?;
+        }
+
+        Ok(())
+    }
+
+    /// Get a reference to the buffer.
+    ///
+    /// # Safety
+    ///
+    /// `finish_wait()` must be called in between `shm::PutImage` requests and this function.
+    #[inline]
+    unsafe fn buffer(&self) -> &[u32] {
         match self {
-            Buffer::Shm(ref mut shm) => shm.as_mut(conn),
+            Buffer::Shm(ref shm) => unsafe { shm.as_ref() },
-            Buffer::Wire(wire) => Ok(wire),
+            Buffer::Wire(wire) => wire,
+        }
+    }
+
+    /// Get a mutable reference to the buffer.
+    ///
+    /// # Safety
+    ///
+    /// `finish_wait()` must be called in between `shm::PutImage` requests and this function.
+    #[inline]
+    unsafe fn buffer_mut(&mut self) -> &mut [u32] {
+        match self {
+            Buffer::Shm(ref mut shm) => unsafe { shm.as_mut() },
+            Buffer::Wire(wire) => wire,
         }
     }
 }
@@ -420,20 +442,40 @@ impl ShmBuffer {
         Ok(())
     }
 
-    /// Get the SHM buffer as a mutable reference.
+    /// Get the SHM buffer as a reference.
-    fn as_mut(&mut self, conn: &impl Connection) -> Result<&mut [u32], PushBufferError> {
+    ///
-        // Make sure that, if we're waiting for the X server to finish processing the last image,
+    /// # Safety
-        // that we finish the wait.
+    ///
-        self.finish_wait(conn)?;
+    /// `finish_wait()` must be called before this function is.
-
+    #[inline]
-        match self.seg.as_mut() {
+    unsafe fn as_ref(&self) -> &[u32] {
+        match self.seg.as_ref() {
             Some((seg, _)) => {
                 // SAFETY: No other code should be able to access the segment.
-                Ok(bytemuck::cast_slice_mut(unsafe { seg.as_mut() }))
+                bytemuck::cast_slice(unsafe { seg.as_ref() })
             }
             None => {
                 // Nothing has been allocated yet.
-                Ok(&mut [])
+                &[]
+            }
+        }
+    }
+
+    /// Get the SHM buffer as a mutable reference.
+    ///
+    /// # Safety
+    ///
+    /// `finish_wait()` must be called before this function is.
+    #[inline]
+    unsafe fn as_mut(&mut self) -> &mut [u32] {
+        match self.seg.as_mut() {
+            Some((seg, _)) => {
+                // SAFETY: No other code should be able to access the segment.
+                bytemuck::cast_slice_mut(unsafe { seg.as_mut() })
+            }
+            None => {
+                // Nothing has been allocated yet.
+                &mut []
             }
         }
     }
@@ -514,13 +556,22 @@ impl ShmSegment {
         }
     }
 
+    /// Get this shared memory segment as a reference.
+    ///
+    /// # Safety
+    ///
+    /// One must ensure that no other processes are writing to this memory.
+    unsafe fn as_ref(&self) -> &[i8] {
+        unsafe { slice::from_raw_parts(self.ptr.as_ptr(), self.size) }
+    }
+
     /// Get this shared memory segment as a mutable reference.
     ///
     /// # Safety
     ///
     /// One must ensure that no other processes are reading from or writing to this memory.
     unsafe fn as_mut(&mut self) -> &mut [i8] {
-        unsafe { std::slice::from_raw_parts_mut(self.ptr.as_ptr(), self.size) }
+        unsafe { slice::from_raw_parts_mut(self.ptr.as_ptr(), self.size) }
     }
 
     /// Get the size of this shared memory segment.