From c12cba8658b8a0bff9bd87e0d4af423f609516eb Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Tue, 10 Jan 2023 15:32:26 -0800 Subject: [PATCH 01/24] wayland: Use `mmap` This is based on the API that will be used for no-copy presentation. But wraps it in `set_buffer`. This also fixes the wayland buffer code to set `self.width` and `self.height` on resize, and set the length of the shared memory file when the buffer is created. --- Cargo.toml | 3 +- src/wayland/buffer.rs | 45 ++++++++++++++++++----- src/wayland/mod.rs | 85 +++++++++++++++++++++++++++---------------- 3 files changed, 91 insertions(+), 42 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 27cfe35e..f0053dae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ rust-version = "1.60.0" [features] default = ["x11", "wayland", "wayland-dlopen"] -wayland = ["wayland-backend", "wayland-client", "nix", "fastrand"] +wayland = ["wayland-backend", "wayland-client", "memmap2", "nix", "fastrand"] wayland-dlopen = ["wayland-sys/dlopen"] x11 = ["bytemuck", "nix", "x11rb", "x11-dl"] @@ -25,6 +25,7 @@ thiserror = "1.0.30" [target.'cfg(all(unix, not(any(target_vendor = "apple", target_os = "android", target_os = "redox"))))'.dependencies] bytemuck = { version = "1.12.3", optional = true } +memmap2 = { version = "0.5.8", optional = true } nix = { version = "0.26.1", optional = true } wayland-backend = { version = "0.1.0", features = ["client_system"], optional = true } wayland-client = { version = "0.30.0", optional = true } diff --git a/src/wayland/buffer.rs b/src/wayland/buffer.rs index c6535a18..5517312c 100644 --- a/src/wayland/buffer.rs +++ b/src/wayland/buffer.rs @@ -1,7 +1,9 @@ +use memmap2::MmapMut; use std::{ ffi::CStr, fs::File, - os::unix::prelude::{AsRawFd, FileExt, FromRawFd}, + os::unix::prelude::{AsRawFd, FromRawFd}, + slice, sync::{ atomic::{AtomicBool, Ordering}, Arc, @@ -69,9 +71,19 @@ fn create_memfile() -> File { panic!("Failed to generate non-existant shm name") } +// Round size to use for pool for given dimentions, rounding up to power of 2 +fn get_pool_size(width: i32, height: i32) -> i32 { + ((width * height * 4) as u32).next_power_of_two() as i32 +} + +unsafe fn map_file(file: &File) -> MmapMut { + unsafe { MmapMut::map_mut(file.as_raw_fd()).expect("Failed to map shared memory") } +} + pub(super) struct WaylandBuffer { qh: QueueHandle, tempfile: File, + map: MmapMut, pool: wl_shm_pool::WlShmPool, pool_size: i32, buffer: wl_buffer::WlBuffer, @@ -82,8 +94,15 @@ pub(super) struct WaylandBuffer { impl WaylandBuffer { pub fn new(shm: &wl_shm::WlShm, width: i32, height: i32, qh: &QueueHandle) -> Self { + // Calculate size to use for shm pool + let pool_size = get_pool_size(width, height); + + // Create an `mmap` shared memory let tempfile = create_memfile(); - let pool_size = width * height * 4; + let _ = tempfile.set_len(pool_size as u64); + let map = unsafe { map_file(&tempfile) }; + + // Create wayland shm pool and buffer let pool = shm.create_pool(tempfile.as_raw_fd(), pool_size, qh, ()); let released = Arc::new(AtomicBool::new(true)); let buffer = pool.create_buffer( @@ -95,8 +114,10 @@ impl WaylandBuffer { qh, released.clone(), ); + Self { qh: qh.clone(), + map, tempfile, pool, pool_size, @@ -119,6 +140,7 @@ impl WaylandBuffer { let _ = self.tempfile.set_len(size as u64); self.pool.resize(size); self.pool_size = size; + self.map = unsafe { map_file(&self.tempfile) }; } // Create buffer with correct size @@ -131,15 +153,10 @@ impl WaylandBuffer { &self.qh, self.released.clone(), ); - } - } - pub fn write(&self, buffer: &[u32]) { - let buffer = - unsafe { std::slice::from_raw_parts(buffer.as_ptr() as *const u8, buffer.len() * 4) }; - self.tempfile - .write_all_at(buffer, 0) - .expect("Failed to write buffer to temporary file."); + self.width = width; + self.height = height; + } } pub fn attach(&self, surface: &wl_surface::WlSurface) { @@ -150,6 +167,14 @@ impl WaylandBuffer { pub fn released(&self) -> bool { self.released.load(Ordering::SeqCst) } + + fn len(&self) -> usize { + self.width as usize * self.height as usize + } + + pub unsafe fn mapped_mut(&mut self) -> &mut [u32] { + unsafe { slice::from_raw_parts_mut(self.map.as_mut_ptr() as *mut u32, self.len()) } + } } impl Drop for WaylandBuffer { diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index 4f96c4a0..bbe92311 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -47,6 +47,8 @@ pub struct WaylandImpl { display: Arc, surface: wl_surface::WlSurface, buffers: Option<(WaylandBuffer, WaylandBuffer)>, + width: i32, + height: i32, } impl WaylandImpl { @@ -72,31 +74,44 @@ impl WaylandImpl { display, surface, buffers: Default::default(), + width: 0, + height: 0, }) } - fn buffer(&mut self, width: i32, height: i32) -> &WaylandBuffer { - self.buffers = Some(if let Some((front, mut back)) = self.buffers.take() { - // Swap buffers; block if back buffer not released yet + // Allocate front and back buffer + fn alloc_buffers(&mut self) { + self.buffers = Some(( + WaylandBuffer::new(&self.display.shm, self.width, self.height, &self.display.qh), + WaylandBuffer::new(&self.display.shm, self.width, self.height, &self.display.qh), + )); + } + + fn resize(&mut self, width: u32, height: u32) { + self.width = width as i32; + self.height = height as i32; + } + + fn buffer_mut(&mut self) -> &mut [u32] { + if let Some((_front, back)) = &mut self.buffers { + // Block if back buffer not released yet if !back.released() { let mut event_queue = self.display.event_queue.lock().unwrap(); while !back.released() { event_queue.blocking_dispatch(&mut State).unwrap(); } } - back.resize(width, height); - (back, front) + + // Resize, if buffer isn't large enough + back.resize(self.width, self.height); } else { - // Allocate front and back buffer - ( - WaylandBuffer::new(&self.display.shm, width, height, &self.display.qh), - WaylandBuffer::new(&self.display.shm, width, height, &self.display.qh), - ) - }); - &self.buffers.as_ref().unwrap().0 + self.alloc_buffers(); + }; + + unsafe { self.buffers.as_mut().unwrap().1.mapped_mut() } } - pub(super) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { + fn present(&mut self) { let _ = self .display .event_queue @@ -104,29 +119,37 @@ impl WaylandImpl { .unwrap() .dispatch_pending(&mut State); - let surface = self.surface.clone(); - let wayland_buffer = self.buffer(width.into(), height.into()); - wayland_buffer.write(buffer); - wayland_buffer.attach(&surface); - - // FIXME: Proper damaging mechanism. - // - // In order to propagate changes on compositors which track damage, for now damage the entire surface. - if self.surface.version() < 4 { - // FIXME: Accommodate scale factor since wl_surface::damage is in terms of surface coordinates while - // wl_surface::damage_buffer is in buffer coordinates. + if let Some((front, back)) = &mut self.buffers { + // Swap front and back buffer + std::mem::swap(front, back); + + front.attach(&self.surface); + + // FIXME: Proper damaging mechanism. // - // i32::MAX is a valid damage box (most compositors interpret the damage box as "the entire surface") - self.surface.damage(0, 0, i32::MAX, i32::MAX); - } else { - // Introduced in version 4, it is an error to use this request in version 3 or lower. - self.surface - .damage_buffer(0, 0, width as i32, height as i32); + // In order to propagate changes on compositors which track damage, for now damage the entire surface. + if self.surface.version() < 4 { + // FIXME: Accommodate scale factor since wl_surface::damage is in terms of surface coordinates while + // wl_surface::damage_buffer is in buffer coordinates. + // + // i32::MAX is a valid damage box (most compositors interpret the damage box as "the entire surface") + self.surface.damage(0, 0, i32::MAX, i32::MAX); + } else { + // Introduced in version 4, it is an error to use this request in version 3 or lower. + self.surface.damage_buffer(0, 0, self.width, self.height); + } + + self.surface.commit(); } - self.surface.commit(); let _ = self.display.event_queue.lock().unwrap().flush(); } + + pub unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { + self.resize(width.into(), height.into()); + self.buffer_mut().copy_from_slice(buffer); + self.present(); + } } impl Dispatch for State { From 85772750f88a63ebae7e23e8637176ffa646bab9 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Tue, 10 Jan 2023 16:04:15 -0800 Subject: [PATCH 02/24] WIP no-copy in public API Currently only for `wayland` and the `winit` example. --- examples/winit.rs | 24 ++++++++++++------------ src/lib.rs | 39 +++++++++++++++++++++++++++++++++++++++ src/wayland/mod.rs | 6 +++--- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/examples/winit.rs b/examples/winit.rs index 3ecb33c8..be8e4343 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -32,21 +32,21 @@ fn main() { let size = window.inner_size(); (size.width, size.height) }; - let buffer = (0..((width * height) as usize)) - .map(|index| { - let y = index / (width as usize); - let x = index % (width as usize); - let red = x % 255; - let green = y % 255; - let blue = (x * y) % 255; - let color = blue | (green << 8) | (red << 16); + surface.resize(width, height); - color as u32 - }) - .collect::>(); + let buffer = surface.buffer_mut(); + for index in 0..(width * height) { + let y = index as u32 / width; + let x = index as u32 % width; + let red = x % 255; + let green = y % 255; + let blue = (x * y) % 255; - surface.set_buffer(&buffer, width as u16, height as u16); + buffer[index as usize] = blue | (green << 8) | (red << 16); + } + + surface.present(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/src/lib.rs b/src/lib.rs index 384aee4f..ba1c04c8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,6 +71,33 @@ macro_rules! make_dispatch { } impl SurfaceDispatch { + pub fn resize(&mut self, width: u32, height: u32) { + match self { + $( + $(#[$attr])* + Self::$name(inner) => inner.resize(width, height), + )* + } + } + + pub fn buffer_mut(&mut self) -> &mut [u32] { + match self { + $( + $(#[$attr])* + Self::$name(inner) => inner.buffer_mut(), + )* + } + } + + pub fn present(&mut self) { + match self { + $( + $(#[$attr])* + Self::$name(inner) => inner.present(), + )* + } + } + unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { match self { $( @@ -235,6 +262,18 @@ impl Surface { }) } + pub fn resize(&mut self, width: u32, height: u32) { + self.surface_impl.resize(width, height); + } + + pub fn buffer_mut(&mut self) -> &mut [u32] { + self.surface_impl.buffer_mut() + } + + pub fn present(&mut self) { + self.surface_impl.present(); + } + /// Shows the given buffer with the given width and height on the window corresponding to this /// graphics context. Panics if buffer.len() ≠ width*height. If the size of the buffer does /// not match the size of the window, the buffer is drawn in the upper-left corner of the window. diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index bbe92311..28462032 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -87,12 +87,12 @@ impl WaylandImpl { )); } - fn resize(&mut self, width: u32, height: u32) { + pub fn resize(&mut self, width: u32, height: u32) { self.width = width as i32; self.height = height as i32; } - fn buffer_mut(&mut self) -> &mut [u32] { + pub fn buffer_mut(&mut self) -> &mut [u32] { if let Some((_front, back)) = &mut self.buffers { // Block if back buffer not released yet if !back.released() { @@ -111,7 +111,7 @@ impl WaylandImpl { unsafe { self.buffers.as_mut().unwrap().1.mapped_mut() } } - fn present(&mut self) { + pub fn present(&mut self) { let _ = self .display .event_queue From 7a5f61db3e2b7069ef3fe215d43c5d1cd319c86b Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Tue, 10 Jan 2023 18:09:39 -0800 Subject: [PATCH 03/24] win32: Use owned buffer and `Gdi:BitBlt` --- src/win32.rs | 221 ++++++++++++++++++++++++++++++++++----------------- 1 file changed, 150 insertions(+), 71 deletions(-) diff --git a/src/win32.rs b/src/win32.rs index 3390bf7c..6e318bac 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -7,28 +7,124 @@ use raw_window_handle::Win32WindowHandle; use std::io; use std::mem; -use std::os::raw::c_int; +use std::ptr; +use std::slice; use windows_sys::Win32::Foundation::HWND; -use windows_sys::Win32::Graphics::Gdi::{ - GetDC, StretchDIBits, ValidateRect, BITMAPINFOHEADER, BI_BITFIELDS, DIB_RGB_COLORS, HDC, - RGBQUAD, SRCCOPY, +use windows_sys::Win32::Graphics::Gdi; + +const ZERO_QUAD: Gdi::RGBQUAD = Gdi::RGBQUAD { + rgbBlue: 0, + rgbGreen: 0, + rgbRed: 0, + rgbReserved: 0, }; +struct Buffer { + dc: Gdi::HDC, + bitmap: Gdi::HBITMAP, + pixels: *mut u32, + width: i32, + height: i32, +} + +impl Drop for Buffer { + fn drop(&mut self) { + unsafe { + Gdi::DeleteDC(self.dc); + Gdi::DeleteObject(self.bitmap); + } + } +} + +impl Buffer { + fn new(window_dc: Gdi::HDC, width: i32, height: i32) -> Self { + let dc = unsafe { Gdi::CreateCompatibleDC(window_dc) }; + assert!(dc != 0); + + // Create a new bitmap info struct. + let bitmap_info = BitmapInfo { + bmi_header: Gdi::BITMAPINFOHEADER { + biSize: mem::size_of::() as u32, + biWidth: width, + biHeight: -height, + biPlanes: 1, + biBitCount: 32, + biCompression: Gdi::BI_BITFIELDS, + biSizeImage: 0, + biXPelsPerMeter: 0, + biYPelsPerMeter: 0, + biClrUsed: 0, + biClrImportant: 0, + }, + bmi_colors: [ + Gdi::RGBQUAD { + rgbRed: 0xff, + ..ZERO_QUAD + }, + Gdi::RGBQUAD { + rgbGreen: 0xff, + ..ZERO_QUAD + }, + Gdi::RGBQUAD { + rgbBlue: 0xff, + ..ZERO_QUAD + }, + ], + }; + + // XXX alignment? + // XXX better to use CreateFileMapping, and pass hSection? + // XXX test return value? + let mut pixels: *mut u32 = ptr::null_mut(); + let bitmap = unsafe { + Gdi::CreateDIBSection( + dc, + &bitmap_info as *const BitmapInfo as *const _, + Gdi::DIB_RGB_COLORS, + &mut pixels as *mut *mut u32 as _, + 0, + 0, + ) + }; + assert!(bitmap != 0); + + unsafe { + Gdi::SelectObject(dc, bitmap); + } + + Self { + dc, + bitmap, + width, + height, + pixels, + } + } + + fn pixels_mut(&mut self) -> &mut [u32] { + unsafe { + slice::from_raw_parts_mut(self.pixels, self.width as usize * self.height as usize * 4) + } + } +} + /// The handle to a window for software buffering. pub struct Win32Impl { /// The window handle. window: HWND, /// The device context for the window. - dc: HDC, + dc: Gdi::HDC, + + buffer: Option, } /// The Win32-compatible bitmap information. #[repr(C)] struct BitmapInfo { - pub bmi_header: BITMAPINFOHEADER, - pub bmi_colors: [RGBQUAD; 3], + bmi_header: Gdi::BITMAPINFOHEADER, + bmi_colors: [Gdi::RGBQUAD; 3], } impl Win32Impl { @@ -46,7 +142,7 @@ impl Win32Impl { // Get the handle to the device context. // SAFETY: We have confirmed that the window handle is valid. let hwnd = handle.hwnd as HWND; - let dc = unsafe { GetDC(hwnd) }; + let dc = unsafe { Gdi::GetDC(hwnd) }; // GetDC returns null if there is a platform error. if dc == 0 { @@ -56,72 +152,55 @@ impl Win32Impl { )); } - Ok(Self { dc, window: hwnd }) + Ok(Self { + dc, + window: hwnd, + buffer: None, + }) } - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - // Create a new bitmap info struct. - let bmi_header = BITMAPINFOHEADER { - biSize: mem::size_of::() as u32, - biWidth: width as i32, - biHeight: -(height as i32), - biPlanes: 1, - biBitCount: 32, - biCompression: BI_BITFIELDS, - biSizeImage: 0, - biXPelsPerMeter: 0, - biYPelsPerMeter: 0, - biClrUsed: 0, - biClrImportant: 0, - }; - let zero_quad = RGBQUAD { - rgbBlue: 0, - rgbGreen: 0, - rgbRed: 0, - rgbReserved: 0, - }; - let bmi_colors = [ - RGBQUAD { - rgbRed: 0xff, - ..zero_quad - }, - RGBQUAD { - rgbGreen: 0xff, - ..zero_quad - }, - RGBQUAD { - rgbBlue: 0xff, - ..zero_quad - }, - ]; - let bitmap_info = BitmapInfo { - bmi_header, - bmi_colors, - }; + pub fn resize(&mut self, width: u32, height: u32) { + if let Some(buffer) = self.buffer.as_ref() { + if buffer.width == width as i32 && buffer.height == height as i32 { + return; + } + } - // Stretch the bitmap onto the window. - // SAFETY: - // - The bitmap information is valid. - // - The buffer is a valid pointer to image data. - unsafe { - StretchDIBits( - self.dc, - 0, - 0, - width as c_int, - height as c_int, - 0, - 0, - width as c_int, - height as c_int, - buffer.as_ptr().cast(), - &bitmap_info as *const BitmapInfo as *const _, - DIB_RGB_COLORS, - SRCCOPY, - ) - }; + self.buffer = if width != 0 && height != 0 { + Some(Buffer::new(self.dc, width as i32, height as i32)) + } else { + None + } + } + + pub fn buffer_mut(&mut self) -> &mut [u32] { + self.buffer.as_mut().map_or(&mut [], Buffer::pixels_mut) + } + + pub fn present(&mut self) { + if let Some(buffer) = &self.buffer { + unsafe { + Gdi::BitBlt( + self.dc, + 0, + 0, + buffer.width, + buffer.height, + buffer.dc, + 0, + 0, + Gdi::SRCCOPY, + ); + + // Validate the window. + Gdi::ValidateRect(self.window, ptr::null_mut()); + } + } + } - // Validate the window. - unsafe { ValidateRect(self.window, std::ptr::null_mut()) }; + pub unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { + self.resize(width.into(), height.into()); + self.buffer_mut().copy_from_slice(buffer); + self.present(); } } From de2f37d244db88a5062b7e881420e7ac2145f12e Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 11 Jan 2023 09:13:39 -0800 Subject: [PATCH 04/24] Implement X11 owned buffer --- src/win32.rs | 10 +- src/x11.rs | 339 ++++++++++++++++++++++++++++++++------------------- 2 files changed, 221 insertions(+), 128 deletions(-) diff --git a/src/win32.rs b/src/win32.rs index 6e318bac..87f7c3b0 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -7,7 +7,7 @@ use raw_window_handle::Win32WindowHandle; use std::io; use std::mem; -use std::ptr; +use std::ptr::{self, NonNull}; use std::slice; use windows_sys::Win32::Foundation::HWND; @@ -23,7 +23,7 @@ const ZERO_QUAD: Gdi::RGBQUAD = Gdi::RGBQUAD { struct Buffer { dc: Gdi::HDC, bitmap: Gdi::HBITMAP, - pixels: *mut u32, + pixels: NonNull, width: i32, height: i32, } @@ -88,6 +88,7 @@ impl Buffer { ) }; assert!(bitmap != 0); + let pixels = NonNull::new(pixels).unwrap(); unsafe { Gdi::SelectObject(dc, bitmap); @@ -104,7 +105,10 @@ impl Buffer { fn pixels_mut(&mut self) -> &mut [u32] { unsafe { - slice::from_raw_parts_mut(self.pixels, self.width as usize * self.height as usize * 4) + slice::from_raw_parts_mut( + self.pixels.as_ptr(), + self.width as usize * self.height as usize * 4, + ) } } } diff --git a/src/x11.rs b/src/x11.rs index 6a9cbaab..d8ae9c12 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -9,7 +9,7 @@ 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, sync::Arc}; +use std::{fmt, io, mem, sync::Arc}; use x11_dl::xlib::Display; use x11_dl::xlib_xcb::Xlib_xcb; @@ -80,6 +80,9 @@ impl X11DisplayImpl { }; let is_shm_available = is_shm_available(&connection); + if !is_shm_available { + log::warn!("SHM extension is not available. Performance may be poor."); + } Ok(Self { connection, @@ -102,11 +105,26 @@ pub struct X11Impl { /// The depth (bits per pixel) of the drawing context. depth: u8, - /// Information about SHM, if it is available. - shm: Option, + /// The buffer we draw to. + buffer: Buffer, + + /// The current buffer width. + width: u16, + + /// The current buffer height. + height: u16, } -struct ShmInfo { +/// The buffer that is being drawn to. +enum Buffer { + /// A buffer implemented using shared memory to prevent unnecessary copying. + Shm(ShmBuffer), + + /// A normal buffer that we send over the wire. + Wire(Vec), +} + +struct ShmBuffer { /// The shared memory segment, paired with its ID. seg: Option<(ShmSegment, shm::Seg)>, @@ -152,6 +170,8 @@ impl X11Impl { window_handle: XcbWindowHandle, display: Arc, ) -> Result { + log::trace!("new: window_handle={:X}", window_handle.window,); + // Check that the handle is valid. if window_handle.window == 0 { return Err(SoftBufferError::IncompleteWindowHandle); @@ -187,18 +207,15 @@ impl X11Impl { .swbuf_err("Failed to get geometry reply")?; // See if SHM is available. - let shm_info = { - let present = display.is_shm_available; - - if present { - // SHM is available. - Some(ShmInfo { - seg: None, - done_processing: None, - }) - } else { - None - } + let buffer = if display.is_shm_available { + // SHM is available. + Buffer::Shm(ShmBuffer { + seg: None, + done_processing: None, + }) + } else { + // SHM is not available. + Buffer::Wire(Vec::new()) }; Ok(Self { @@ -206,119 +223,161 @@ impl X11Impl { window, gc, depth: geometry_reply.depth, - shm: shm_info, + buffer, + width: 0, + height: 0, }) } - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - // Draw the image to the buffer. - let result = unsafe { self.set_buffer_shm(buffer, width, height) }.and_then(|had_shm| { - if had_shm { - Ok(()) - } else { - log::debug!("Falling back to non-SHM method"); - self.set_buffer_fallback(buffer, width, height) + /// Resize the internal buffer to the given width and height. + pub(crate) fn resize(&mut self, width: u32, height: u32) { + log::trace!("resize: window={:X}, size={}x{}", self.window, width, height); + + // Width and height should fit in u16. + let width: u16 = width.try_into().expect("Width too large"); + let height: u16 = height.try_into().expect("Height too large"); + + if width == self.width && height == self.height { + // Nothing to do. + return; + } + + match self.buffer.resize(&self.display.connection, width, height) { + Ok(()) => { + // We successfully resized the buffer. + self.width = width; + self.height = height; } - }); - if let Err(e) = result { - log::error!("Failed to draw image to window: {}", e); + Err(e) => { + log::error!("Failed to resize window: {}", e); + } } } - /// Put the given buffer into the window using the SHM method. - /// - /// Returns `false` if SHM is not available. - /// - /// # Safety - /// - /// The buffer's length must be `width * height`. - unsafe fn set_buffer_shm( - &mut self, - buffer: &[u32], - width: u16, - height: u16, - ) -> Result { - let shm_info = match self.shm { - Some(ref mut info) => info, - None => return Ok(false), - }; + /// Get a mutable reference to the buffer. + pub(crate) fn buffer_mut(&mut self) -> &mut [u32] { + log::trace!("buffer_mut: window={:X}", self.window); - // If the X server is still processing the last image, wait for it to finish. - shm_info.finish_wait(&self.display.connection)?; + let buffer = self + .buffer + .buffer_mut(&self.display.connection) + .expect("Failed to get buffer"); - // Get the SHM segment to use. - let necessary_size = (width as usize) * (height as usize) * 4; - let (segment, segment_id) = shm_info.segment(&self.display.connection, necessary_size)?; + // Crop it down to the window size. + &mut buffer[..total_len(self.width, self.height) / 4] + } - // Copy the buffer into the segment. - // SAFETY: The buffer is properly sized and we've ensured that the X server isn't reading from it. - unsafe { - segment.copy(buffer); + /// Push the buffer to the window. + pub(crate) fn present(&mut self) { + log::trace!("present: window={:X}", self.window); + + let result = match self.buffer { + Buffer::Wire(ref wire) => { + // This is a suboptimal strategy, raise a stink in the debug logs. + log::debug!("Falling back to non-SHM method for window drawing."); + + self.display + .connection + .put_image( + xproto::ImageFormat::Z_PIXMAP, + self.window, + self.gc, + self.width, + self.height, + 0, + 0, + 0, + self.depth, + bytemuck::cast_slice(wire), + ) + .map(|c| c.ignore_error()) + .push_err() + } + + Buffer::Shm(ref mut shm) => { + // If the X server is still processing the last image, wait for it to finish. + shm.finish_wait(&self.display.connection) + .and_then(|()| { + // Put the image into the window. + if let Some((_, segment_id)) = shm.seg { + self.display + .connection + .shm_put_image( + self.window, + self.gc, + self.width, + self.height, + 0, + 0, + self.width, + self.height, + 0, + 0, + self.depth, + xproto::ImageFormat::Z_PIXMAP.into(), + false, + segment_id, + 0, + ) + .push_err() + .map(|c| c.ignore_error()) + } else { + Ok(()) + } + }) + .and_then(|()| { + // Send a short request to act as a notification for when the X server is done processing the image. + shm.begin_wait(&self.display.connection) + }) + } + }; + + if let Err(e) = result { + log::error!("Failed to draw image to window: {}", e); } + } - // Put the image into the window. - self.display - .connection - .shm_put_image( - self.window, - self.gc, - width, - height, - 0, - 0, - width, - height, - 0, - 0, - self.depth, - xproto::ImageFormat::Z_PIXMAP.into(), - false, - segment_id, - 0, - )? - .ignore_error(); - - // Send a short request to act as a notification for when the X server is done processing the image. - shm_info.begin_wait(&self.display.connection)?; - - Ok(true) + pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { + self.resize(width.into(), height.into()); + self.buffer_mut().copy_from_slice(buffer); + self.present(); } +} - /// Put the given buffer into the window using the fallback wire transfer method. - fn set_buffer_fallback( +impl Buffer { + /// Resize the buffer to the given size. + fn resize( &mut self, - buffer: &[u32], + conn: &impl Connection, width: u16, height: u16, ) -> Result<(), PushBufferError> { - self.display - .connection - .put_image( - xproto::ImageFormat::Z_PIXMAP, - self.window, - self.gc, - width, - height, - 0, - 0, - 0, - self.depth, - bytemuck::cast_slice(buffer), - )? - .ignore_error(); + match self { + Buffer::Shm(ref mut shm) => shm.alloc_segment(conn, total_len(width, height)), + Buffer::Wire(wire) => { + wire.resize(total_len(width, height), 0); + Ok(()) + } + } + } - Ok(()) + /// Get a mutable reference to the buffer. + fn buffer_mut(&mut self, conn: &impl Connection) -> Result<&mut [u32], PushBufferError> { + match self { + Buffer::Shm(ref mut shm) => shm.as_mut(conn), + Buffer::Wire(wire) => Ok(wire), + } } } -impl ShmInfo { +impl ShmBuffer { /// Allocate a new `ShmSegment` of the given size. - fn segment( + fn alloc_segment( &mut self, conn: &impl Connection, size: usize, - ) -> Result<(&mut ShmSegment, shm::Seg), PushBufferError> { + ) -> Result<(), PushBufferError> { // Round the size up to the next power of two to prevent frequent reallocations. let size = size.next_power_of_two(); @@ -334,12 +393,26 @@ impl ShmInfo { self.associate(conn, new_seg)?; } - // Get the segment and ID. - Ok(self - .seg - .as_mut() - .map(|(ref mut seg, id)| (seg, *id)) - .unwrap()) + + Ok(()) + } + + /// Get the SHM buffer as a mutable 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, + // that we finish the wait. + self.finish_wait(conn)?; + + match self.seg.as_mut() { + Some((seg, _)) => { + // SAFETY: No other code should be able to access the segment. + Ok(bytemuck::cast_slice_mut(unsafe { seg.as_mut() })) + } + None => { + // Nothing has been allocated yet. + Ok(&mut []) + } + } } /// Associate an SHM segment with the server. @@ -354,6 +427,9 @@ impl ShmInfo { // Take out the old one and detach it. if let Some((old_seg, old_id)) = self.seg.replace((seg, new_id)) { + // Wait for the old segment to finish processing. + self.finish_wait(conn)?; + conn.shm_detach(old_id)?.ignore_error(); // Drop the old segment. @@ -415,23 +491,13 @@ impl ShmSegment { } } - /// Copy data into this shared memory segment. + /// Get this shared memory segment as a mutable reference. /// /// # Safety /// - /// This function assumes that the size of `self`'s buffer is larger than or equal to `data.len()`. - /// In addition, no other processes should be reading from or writing to this memory. - unsafe fn copy(&mut self, data: &[T]) { - debug_assert!(data.len() * std::mem::size_of::() <= self.size,); - let incoming_data = bytemuck::cast_slice::<_, u8>(data); - - unsafe { - std::ptr::copy_nonoverlapping( - incoming_data.as_ptr(), - self.ptr.as_ptr() as *mut u8, - incoming_data.len(), - ) - } + /// 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) } } /// Get the size of this shared memory segment. @@ -460,7 +526,7 @@ impl Drop for ShmSegment { impl Drop for X11Impl { fn drop(&mut self) { // If we used SHM, make sure it's detached from the server. - if let Some(mut shm) = self.shm.take() { + if let Buffer::Shm(mut shm) = mem::replace(&mut self.buffer, Buffer::Wire(Vec::new())) { // If we were in the middle of processing a buffer, wait for it to finish. shm.finish_wait(&self.display.connection).ok(); @@ -563,11 +629,11 @@ impl From for PushBufferError { } /// Convenient wrapper to cast errors into SoftBufferError. -trait ResultExt { +trait SwResultExt { fn swbuf_err(self, msg: impl Into) -> Result; } -impl ResultExt for Result { +impl SwResultExt for Result { fn swbuf_err(self, msg: impl Into) -> Result { self.map_err(|e| { SoftBufferError::PlatformError(Some(msg.into()), Some(Box::new(LibraryError(e)))) @@ -575,6 +641,17 @@ impl ResultExt for Result { } } +/// Convenient wrapper to cast errors into PushBufferError. +trait PushResultExt { + fn push_err(self) -> Result; +} + +impl> PushResultExt for Result { + fn push_err(self) -> Result { + self.map_err(Into::into) + } +} + /// A wrapper around a library error. /// /// This prevents `x11-dl` and `x11rb` from becoming public dependencies, since users cannot downcast @@ -594,3 +671,15 @@ impl fmt::Display for LibraryError { } impl std::error::Error for LibraryError {} + +/// Get the length that a slice needs to be to hold a buffer of the given dimensions. +#[inline(always)] +fn total_len(width: u16, height: u16) -> usize { + let width: usize = width.into(); + let height: usize = height.into(); + + width + .checked_mul(height) + .and_then(|len| len.checked_mul(4)) + .unwrap_or_else(|| panic!("Dimensions are too large: ({} x {})", width, height)) +} From ce5952890d011f733a9d575ce780be78de3cff91 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 11 Jan 2023 09:15:55 -0800 Subject: [PATCH 05/24] fmt --- src/x11.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/x11.rs b/src/x11.rs index d8ae9c12..c9fa5667 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -231,7 +231,12 @@ impl X11Impl { /// Resize the internal buffer to the given width and height. pub(crate) fn resize(&mut self, width: u32, height: u32) { - log::trace!("resize: window={:X}, size={}x{}", self.window, width, height); + log::trace!( + "resize: window={:X}, size={}x{}", + self.window, + width, + height + ); // Width and height should fit in u16. let width: u16 = width.try_into().expect("Width too large"); @@ -393,7 +398,6 @@ impl ShmBuffer { self.associate(conn, new_seg)?; } - Ok(()) } From 579656aabea53534bb69bd090fb6689f065cb8ad Mon Sep 17 00:00:00 2001 From: jtnunley Date: Wed, 11 Jan 2023 10:15:06 -0800 Subject: [PATCH 06/24] Add owned buffer for web --- src/web.rs | 65 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 6 deletions(-) diff --git a/src/web.rs b/src/web.rs index 28667873..1665fd34 100644 --- a/src/web.rs +++ b/src/web.rs @@ -1,3 +1,7 @@ +//! Implementation of software buffering for web targets. + +#![allow(clippy::uninlined_format_args)] + use raw_window_handle::WebWindowHandle; use wasm_bindgen::Clamped; use wasm_bindgen::JsCast; @@ -6,10 +10,20 @@ use web_sys::HtmlCanvasElement; use web_sys::ImageData; use crate::SoftBufferError; +use std::convert::TryInto; pub struct WebImpl { + /// The handle to the canvas that we're drawing to. canvas: HtmlCanvasElement, + + /// The 2D rendering context for the canvas. ctx: CanvasRenderingContext2d, + + /// The buffer that we're drawing to. + buffer: Vec, + + /// The current width of the canvas. + width: u32, } impl WebImpl { @@ -57,14 +71,32 @@ impl WebImpl { .dyn_into() .expect("`getContext(\"2d\") didn't return a `CanvasRenderingContext2d`"); - Ok(Self { canvas, ctx }) + Ok(Self { + canvas, + ctx, + buffer: Vec::new(), + width: 0, + }) } - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - self.canvas.set_width(width.into()); - self.canvas.set_height(height.into()); + /// Resize the canvas to the given dimensions. + pub(crate) fn resize(&mut self, width: u32, height: u32) { + self.buffer.resize(total_len(width, height), 0); + self.canvas.set_width(width); + self.canvas.set_height(height); + self.width = width; + } - let bitmap: Vec<_> = buffer + /// Get a pointer to the mutable buffer. + pub(crate) fn buffer_mut(&mut self) -> &mut [u32] { + &mut self.buffer + } + + /// Push the buffer to the canvas. + pub(crate) fn present(&mut self) { + // Create a bitmap from the buffer. + let bitmap: Vec<_> = self + .buffer .iter() .copied() .flat_map(|pixel| [(pixel >> 16) as u8, (pixel >> 8) as u8, pixel as u8, 255]) @@ -72,9 +104,30 @@ impl WebImpl { // This should only throw an error if the buffer we pass's size is incorrect, which is checked in the outer `set_buffer` call. let image_data = - ImageData::new_with_u8_clamped_array(Clamped(&bitmap), width.into()).unwrap(); + ImageData::new_with_u8_clamped_array(Clamped(&bitmap), self.width).unwrap(); // This can only throw an error if `data` is detached, which is impossible. self.ctx.put_image_data(&image_data, 0.0, 0.0).unwrap(); } + + pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { + self.resize(width.into(), height.into()); + self.buffer.copy_from_slice(buffer); + self.present(); + } +} + +#[inline(always)] +fn total_len(width: u32, height: u32) -> usize { + // Convert width and height to `usize`, then multiply. + width + .try_into() + .ok() + .and_then(|w: usize| height.try_into().ok().and_then(|h| w.checked_mul(h))) + .unwrap_or_else(|| { + panic!( + "Overflow when calculating total length of buffer: {}x{}", + width, height + ); + }) } From 256ff9eb6f7695088d57c4d88f2ab3b8df6619a7 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 13 Jan 2023 15:37:36 -0800 Subject: [PATCH 07/24] cg: Port to owned buffer Still probably not the most efficient implementation. --- Cargo.toml | 1 + src/cg.rs | 89 ++++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 64 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f0053dae..2ff41732 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ version = "0.42.0" features = ["Win32_Graphics_Gdi", "Win32_UI_WindowsAndMessaging", "Win32_Foundation"] [target.'cfg(target_os = "macos")'.dependencies] +bytemuck = { version = "1.12.3", features = ["extern_crate_alloc"] } cocoa = "0.24.0" core-graphics = "0.22.3" foreign-types = "0.3.0" diff --git a/src/cg.rs b/src/cg.rs index 29103720..fd190519 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -14,8 +14,20 @@ use foreign_types::ForeignType; use std::sync::Arc; +struct Buffer(Vec); + +impl AsRef<[u8]> for Buffer { + fn as_ref(&self) -> &[u8] { + bytemuck::cast_slice(&self.0) + } +} + pub struct CGImpl { layer: CALayer, + color_space: CGColorSpace, + buffer: Option>, + width: u32, + height: u32, } impl CGImpl { @@ -34,36 +46,61 @@ impl CGImpl { view.addSubview_(subview); // retains subview (+1) = 2 let _: () = msg_send![subview, release]; // releases subview (-1) = 1 } - Ok(Self { layer }) + let color_space = CGColorSpace::create_device_rgb(); + Ok(Self { + layer, + color_space, + width: 0, + height: 0, + buffer: None, + }) } - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - let color_space = CGColorSpace::create_device_rgb(); - let data = - unsafe { std::slice::from_raw_parts(buffer.as_ptr() as *const u8, buffer.len() * 4) } - .to_vec(); - let data_provider = CGDataProvider::from_buffer(Arc::new(data)); - let image = CGImage::new( - width as usize, - height as usize, - 8, - 32, - (width * 4) as usize, - &color_space, - kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, - &data_provider, - false, - kCGRenderingIntentDefault, - ); + pub fn resize(&mut self, width: u32, height: u32) { + self.width = width; + self.height = height; + } - // The CALayer has a default action associated with a change in the layer contents, causing - // a quarter second fade transition to happen every time a new buffer is applied. This can - // be mitigated by wrapping the operation in a transaction and disabling all actions. - transaction::begin(); - transaction::set_disable_actions(true); + pub fn buffer_mut(&mut self) -> &mut [u32] { + if self.buffer.is_none() { + self.buffer = Some(Vec::new()); + } + let buffer = self.buffer.as_mut().unwrap(); + buffer.resize(self.width as usize * self.height as usize * 4, 0); + buffer.as_mut() + } + + pub fn present(&mut self) { + if let Some(buffer) = self.buffer.take() { + let data_provider = CGDataProvider::from_buffer(Arc::new(Buffer(buffer))); + let image = CGImage::new( + self.width as usize, + self.height as usize, + 8, + 32, + (self.width * 4) as usize, + &self.color_space, + kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, + &data_provider, + false, + kCGRenderingIntentDefault, + ); + + // The CALayer has a default action associated with a change in the layer contents, causing + // a quarter second fade transition to happen every time a new buffer is applied. This can + // be mitigated by wrapping the operation in a transaction and disabling all actions. + transaction::begin(); + transaction::set_disable_actions(true); - unsafe { self.layer.set_contents(image.as_ptr() as id) }; + unsafe { self.layer.set_contents(image.as_ptr() as id) }; - transaction::commit(); + transaction::commit(); + } + } + + pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { + self.resize(width.into(), height.into()); + self.buffer_mut().copy_from_slice(buffer); + self.present(); } } From 8c3275ccc94b5e629d837322c2758dfdfb9c7cbc Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 13 Jan 2023 17:09:26 -0800 Subject: [PATCH 08/24] win32: Fix length of buffer slice --- src/win32.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/win32.rs b/src/win32.rs index 87f7c3b0..28add5f2 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -107,7 +107,7 @@ impl Buffer { unsafe { slice::from_raw_parts_mut( self.pixels.as_ptr(), - self.width as usize * self.height as usize * 4, + self.width as usize * self.height as usize, ) } } From b16e00e98a425877586e3066845bc4f40a51d628 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 13 Jan 2023 17:26:27 -0800 Subject: [PATCH 09/24] Port more examples to owned buffers --- examples/libxcb.rs | 13 ++++++------- examples/rectangle.rs | 18 +++++++----------- examples/winit.rs | 4 ++-- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/examples/libxcb.rs b/examples/libxcb.rs index 4498b005..e0fa8100 100644 --- a/examples/libxcb.rs +++ b/examples/libxcb.rs @@ -12,6 +12,8 @@ mod example { xcb_ffi::XCBConnection, }; + const RED: u32 = 255 << 16; + pub(crate) fn run() { // Create a new XCB connection let (conn, screen) = XCBConnection::connect(None).expect("Failed to connect to X server"); @@ -96,13 +98,10 @@ mod example { match event { Event::Expose(_) => { // Draw a width x height red rectangle. - let red = 255 << 16; - let source = std::iter::repeat(red) - .take((width as usize * height as usize) as _) - .collect::>(); - - // Draw the buffer. - surface.set_buffer(&source, width, height); + surface.resize(width.into(), height.into()); + let buffer = surface.buffer_mut(); + buffer.fill(RED); + surface.present(); } Event::ConfigureNotify(configure_notify) => { width = configure_notify.width; diff --git a/examples/rectangle.rs b/examples/rectangle.rs index 17b8a201..21dc79ee 100644 --- a/examples/rectangle.rs +++ b/examples/rectangle.rs @@ -43,7 +43,6 @@ fn main() { let context = unsafe { softbuffer::Context::new(&window) }.unwrap(); let mut surface = unsafe { softbuffer::Surface::new(&context, &window) }.unwrap(); - let mut buffer = Vec::new(); let mut flag = false; event_loop.run(move |event, _, control_flow| { @@ -54,19 +53,16 @@ fn main() { // Grab the window's client area dimensions let (width, height) = { let size = window.inner_size(); - (size.width as usize, size.height as usize) + (size.width, size.height) }; - // Resize the off-screen buffer if the window size has changed - if buffer.len() != width * height { - buffer.resize(width * height, 0); - } + // Resize surface if needed + surface.resize(width, height); - // Draw something in the offscreen buffer - redraw(&mut buffer, width, height, flag); - - // Blit the offscreen buffer to the window's client area - surface.set_buffer(&buffer, width as u16, height as u16); + // Draw something in the window + let buffer = surface.buffer_mut(); + redraw(buffer, width as usize, height as usize, flag); + surface.present(); } Event::WindowEvent { diff --git a/examples/winit.rs b/examples/winit.rs index be8e4343..1a725587 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -37,8 +37,8 @@ fn main() { let buffer = surface.buffer_mut(); for index in 0..(width * height) { - let y = index as u32 / width; - let x = index as u32 % width; + let y = index / width; + let x = index % width; let red = x % 255; let green = y % 255; let blue = (x * y) % 255; From 5e8a09da88285c99148ab4e6e5b8b1b4464a8e57 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 18 Jan 2023 13:40:10 -0800 Subject: [PATCH 10/24] orbital: Implement new API (but with copy) --- src/orbital.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/orbital.rs b/src/orbital.rs index 9d48126a..af403e44 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -42,14 +42,37 @@ impl Drop for OrbitalMap { pub struct OrbitalImpl { handle: OrbitalWindowHandle, + width: u32, + height: u32, + buffer: Vec, } impl OrbitalImpl { pub fn new(handle: OrbitalWindowHandle) -> Result { - Ok(Self { handle }) + Ok(Self { + handle, + width: 0, + height: 0, + buffer: Vec::new(), + }) } - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width_u16: u16, height_u16: u16) { + pub fn resize(&mut self, width: u32, height: u32) { + self.width = width; + self.height = height; + } + + pub fn buffer_mut(&mut self) -> &mut [u32] { + self.buffer + .resize(self.width as usize * self.height as usize, 0); + &mut self.buffer + } + + pub fn present(&mut self) { + unsafe { self.set_buffer(&self.buffer, self.width as u16, self.height as u16) }; + } + + pub unsafe fn set_buffer(&self, buffer: &[u32], width_u16: u16, height_u16: u16) { let window_fd = self.handle.window as usize; // Read the current width and size From 1c08169fc3c0f997199bf1b8251f656466a8bf63 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 18 Jan 2023 13:40:10 -0800 Subject: [PATCH 11/24] cg: Allocate correct buffer size --- src/cg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cg.rs b/src/cg.rs index 70dc98ed..c4eadc19 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -68,7 +68,7 @@ impl CGImpl { self.buffer = Some(Vec::new()); } let buffer = self.buffer.as_mut().unwrap(); - buffer.resize(self.width as usize * self.height as usize * 4, 0); + buffer.resize(self.width as usize * self.height as usize, 0); buffer.as_mut() } From b963c438c765118d8b8056bd104de1d702b4caf1 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 18 Jan 2023 14:18:19 -0800 Subject: [PATCH 12/24] examples: Use owned buffer API for `fruit` and `winit_wrong_sized_buffer` --- examples/fruit.rs | 25 ++++++++++++++----------- examples/winit_wrong_sized_buffer.rs | 24 ++++++++++++------------ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/examples/fruit.rs b/examples/fruit.rs index b64ae474..b0006e18 100644 --- a/examples/fruit.rs +++ b/examples/fruit.rs @@ -6,16 +6,6 @@ use winit::window::WindowBuilder; fn main() { //see fruit.jpg.license for the license of fruit.jpg let fruit = image::load_from_memory(include_bytes!("fruit.jpg")).unwrap(); - let buffer = fruit - .pixels() - .map(|(_x, _y, pixel)| { - let red = pixel.0[0] as u32; - let green = pixel.0[1] as u32; - let blue = pixel.0[2] as u32; - - blue | (green << 8) | (red << 16) - }) - .collect::>(); let event_loop = EventLoop::new(); let window = WindowBuilder::new() @@ -45,7 +35,20 @@ fn main() { match event { Event::RedrawRequested(window_id) if window_id == window.id() => { - surface.set_buffer(&buffer, fruit.width() as u16, fruit.height() as u16); + surface.resize(fruit.width(), fruit.height()); + + let buffer = surface.buffer_mut(); + let width = fruit.width() as usize; + for (x, y, pixel) in fruit.pixels() { + let red = pixel.0[0] as u32; + let green = pixel.0[1] as u32; + let blue = pixel.0[2] as u32; + + let color = blue | (green << 8) | (red << 16); + buffer[y as usize * width + x as usize] = color; + } + + surface.present(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/winit_wrong_sized_buffer.rs b/examples/winit_wrong_sized_buffer.rs index 120e5bc0..aeaf9272 100644 --- a/examples/winit_wrong_sized_buffer.rs +++ b/examples/winit_wrong_sized_buffer.rs @@ -31,21 +31,21 @@ fn main() { match event { Event::RedrawRequested(window_id) if window_id == window.id() => { - let buffer = (0..(BUFFER_WIDTH * BUFFER_HEIGHT)) - .map(|index| { - let y = index / BUFFER_WIDTH; - let x = index % BUFFER_WIDTH; - let red = x % 255; - let green = y % 255; - let blue = (x * y) % 255; + surface.resize(BUFFER_WIDTH as u32, BUFFER_HEIGHT as u32); - let color = blue | (green << 8) | (red << 16); + let buffer = surface.buffer_mut(); + for y in 0..BUFFER_HEIGHT { + for x in 0..BUFFER_WIDTH { + let red = x as u32 % 255; + let green = y as u32 % 255; + let blue = (x as u32 * y as u32) % 255; - color as u32 - }) - .collect::>(); + let color = blue | (green << 8) | (red << 16); + buffer[y * BUFFER_WIDTH + x] = color; + } + } - surface.set_buffer(&buffer, BUFFER_WIDTH as u16, BUFFER_HEIGHT as u16); + surface.present(); } Event::WindowEvent { event: WindowEvent::CloseRequested, From a922a392b6258e99175888d6b48d06695a8364d5 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 18 Jan 2023 17:10:04 -0800 Subject: [PATCH 13/24] Remove `set_buffer`, and move its documentation to the new methods --- README.md | 23 ++++++++++---------- examples/animation.rs | 7 +++++-- src/cg.rs | 6 ------ src/lib.rs | 49 +++++++++++++++---------------------------- src/orbital.rs | 8 +++---- src/wayland/mod.rs | 6 ------ src/web.rs | 8 +------ src/win32.rs | 6 ------ src/x11.rs | 6 ------ 9 files changed, 38 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index f7844918..c5639785 100644 --- a/README.md +++ b/README.md @@ -72,21 +72,20 @@ fn main() { let size = window.inner_size(); (size.width, size.height) }; - let buffer = (0..((width * height) as usize)) - .map(|index| { - let y = index / (width as usize); - let x = index % (width as usize); - let red = x % 255; - let green = y % 255; - let blue = (x * y) % 255; + surface.resize(width, height); - let color = blue | (green << 8) | (red << 16); + let buffer = surface.buffer_mut(); + for index in 0..(width * height) { + let y = index / width; + let x = index % width; + let red = x % 255; + let green = y % 255; + let blue = (x * y) % 255; - color as u32 - }) - .collect::>(); + buffer[index as usize] = blue | (green << 8) | (red << 16); + } - surface.set_buffer(&buffer, width as u16, height as u16); + surface.present(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/animation.rs b/examples/animation.rs index 7fcc7402..ae3f8092 100644 --- a/examples/animation.rs +++ b/examples/animation.rs @@ -47,8 +47,11 @@ fn main() { frames = pre_render_frames(width as usize, height as usize); }; - let buffer = &frames[((elapsed * 60.0).round() as usize).clamp(0, 59)]; - surface.set_buffer(buffer.as_slice(), width as u16, height as u16); + let frame = &frames[((elapsed * 60.0).round() as usize).clamp(0, 59)]; + + surface.resize(width, height); + surface.buffer_mut().copy_from_slice(frame); + surface.present(); } Event::MainEventsCleared => { window.request_redraw(); diff --git a/src/cg.rs b/src/cg.rs index c4eadc19..16a47812 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -103,12 +103,6 @@ impl CGImpl { transaction::commit(); } } - - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - self.resize(width.into(), height.into()); - self.buffer_mut().copy_from_slice(buffer); - self.present(); - } } impl Drop for CGImpl { diff --git a/src/lib.rs b/src/lib.rs index e81b3c7e..cad84d76 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -97,15 +97,6 @@ macro_rules! make_dispatch { )* } } - - unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - match self { - $( - $(#[$attr])* - Self::$name(inner) => unsafe { inner.set_buffer(buffer, width, height) }, - )* - } - } } }; } @@ -262,23 +253,19 @@ impl Surface { }) } + /// Set the size of the buffer that will be returned by [`Surface::buffer_mut`]. + /// + /// If the size of the buffer does not match the size of the window, the buffer is drawn + /// in the upper-left corner of the window. It is recommended in most production use cases + /// to have the buffer fill the entire window. Use your windowing library to find the size + /// of the window. pub fn resize(&mut self, width: u32, height: u32) { self.surface_impl.resize(width, height); } - pub fn buffer_mut(&mut self) -> &mut [u32] { - self.surface_impl.buffer_mut() - } - - pub fn present(&mut self) { - self.surface_impl.present(); - } - - /// Shows the given buffer with the given width and height on the window corresponding to this - /// graphics context. Panics if buffer.len() ≠ width*height. If the size of the buffer does - /// not match the size of the window, the buffer is drawn in the upper-left corner of the window. - /// It is recommended in most production use cases to have the buffer fill the entire window. - /// Use your windowing library to find the size of the window. + /// Return a mutable slice to the buffer that the next frame should be rendered into. The size + /// be set with [`Surface::resize`] first. The initial contents of the buffer may be zeroed, or + /// may contain a previous frame. /// /// The format of the buffer is as follows. There is one u32 in the buffer for each pixel in /// the area to draw. The first entry is the upper-left most pixel. The second is one to the right @@ -297,10 +284,15 @@ impl Surface { /// R: Red channel /// G: Green channel /// B: Blue channel + pub fn buffer_mut(&mut self) -> &mut [u32] { + self.surface_impl.buffer_mut() + } + + /// Presents buffer returned by [`Surface::buffer_mut`] to the window. /// /// # Platform dependent behavior /// - /// This section of the documentation details how some platforms may behave when [`set_buffer`](Surface::set_buffer) + /// This section of the documentation details how some platforms may behave when [`present`](Surface::present) /// is called. /// /// ## Wayland @@ -311,15 +303,8 @@ impl Surface { /// /// If the caller wishes to synchronize other surface/window changes, such requests must be sent to the /// Wayland compositor before calling this function. - #[inline] - pub fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - if (width as usize) * (height as usize) != buffer.len() { - panic!("The size of the passed buffer is not the correct size. Its length must be exactly width*height."); - } - - unsafe { - self.surface_impl.set_buffer(buffer, width, height); - } + pub fn present(&mut self) { + self.surface_impl.present(); } } diff --git a/src/orbital.rs b/src/orbital.rs index af403e44..4db8c8f5 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -69,10 +69,10 @@ impl OrbitalImpl { } pub fn present(&mut self) { - unsafe { self.set_buffer(&self.buffer, self.width as u16, self.height as u16) }; + self.set_buffer(&self.buffer, self.width, self.height); } - pub unsafe fn set_buffer(&self, buffer: &[u32], width_u16: u16, height_u16: u16) { + fn set_buffer(&self, buffer: &[u32], width_u32: u32, height_u32: u32) { let window_fd = self.handle.window as usize; // Read the current width and size @@ -107,8 +107,8 @@ impl OrbitalImpl { }; // Copy each line, cropping to fit - let width = width_u16 as usize; - let height = height_u16 as usize; + let width = width_u32 as usize; + let height = height_u32 as usize; let min_width = cmp::min(width, window_width); let min_height = cmp::min(height, window_height); for y in 0..min_height { diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index 28462032..cfb64e55 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -144,12 +144,6 @@ impl WaylandImpl { let _ = self.display.event_queue.lock().unwrap().flush(); } - - pub unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - self.resize(width.into(), height.into()); - self.buffer_mut().copy_from_slice(buffer); - self.present(); - } } impl Dispatch for State { diff --git a/src/web.rs b/src/web.rs index 9b820268..e51fc93a 100644 --- a/src/web.rs +++ b/src/web.rs @@ -118,19 +118,13 @@ impl WebImpl { .flat_map(|pixel| [(pixel >> 16) as u8, (pixel >> 8) as u8, pixel as u8, 255]) .collect(); - // This should only throw an error if the buffer we pass's size is incorrect, which is checked in the outer `set_buffer` call. + // This should only throw an error if the buffer we pass's size is incorrect. let image_data = ImageData::new_with_u8_clamped_array(Clamped(&bitmap), self.width).unwrap(); // This can only throw an error if `data` is detached, which is impossible. self.ctx.put_image_data(&image_data, 0.0, 0.0).unwrap(); } - - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - self.resize(width.into(), height.into()); - self.buffer.copy_from_slice(buffer); - self.present(); - } } #[inline(always)] diff --git a/src/win32.rs b/src/win32.rs index 28add5f2..0e7f5906 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -201,10 +201,4 @@ impl Win32Impl { } } } - - pub unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - self.resize(width.into(), height.into()); - self.buffer_mut().copy_from_slice(buffer); - self.present(); - } } diff --git a/src/x11.rs b/src/x11.rs index c9fa5667..afde9121 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -342,12 +342,6 @@ impl X11Impl { log::error!("Failed to draw image to window: {}", e); } } - - pub(crate) unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) { - self.resize(width.into(), height.into()); - self.buffer_mut().copy_from_slice(buffer); - self.present(); - } } impl Buffer { From 1bb74ddd636c4a5c75558eeda01530ce2f762565 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 19 Jan 2023 12:20:00 -0800 Subject: [PATCH 14/24] Make `Context`/`Surface` consistently `!Send`/`!Sync` --- src/lib.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index cad84d76..092fd803 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,6 +21,7 @@ mod x11; mod error; +use std::marker::PhantomData; #[cfg(any(wayland_platform, x11_platform))] use std::sync::Arc; @@ -35,6 +36,7 @@ use raw_window_handle::{ pub struct Context { /// The inner static dispatch object. context_impl: ContextDispatch, + _marker: PhantomData<*mut ()>, } /// A macro for creating the enum used to statically dispatch to the platform-specific implementation. @@ -167,6 +169,7 @@ impl Context { Ok(Self { context_impl: imple, + _marker: PhantomData, }) } } @@ -174,6 +177,7 @@ impl Context { pub struct Surface { /// This is boxed so that `Surface` is the same size on every platform. surface_impl: Box, + _marker: PhantomData<*mut ()>, } impl Surface { @@ -250,6 +254,7 @@ impl Surface { Ok(Self { surface_impl: Box::new(imple), + _marker: PhantomData, }) } From 34a2796ed15a94091657e384c730344cad5b8a14 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 19 Jan 2023 16:03:22 -0800 Subject: [PATCH 15/24] Don't use thread-safe types --- src/lib.rs | 14 ++++++-------- src/wayland/mod.rs | 17 ++++++++--------- src/x11.rs | 8 ++++---- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 092fd803..d78abc71 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,7 +23,7 @@ mod error; use std::marker::PhantomData; #[cfg(any(wayland_platform, x11_platform))] -use std::sync::Arc; +use std::rc::Rc; pub use error::SoftBufferError; @@ -105,9 +105,9 @@ macro_rules! make_dispatch { make_dispatch! { #[cfg(x11_platform)] - X11(Arc, x11::X11Impl), + X11(Rc, x11::X11Impl), #[cfg(wayland_platform)] - Wayland(std::sync::Arc, wayland::WaylandImpl), + Wayland(Rc, wayland::WaylandImpl), #[cfg(target_os = "windows")] Win32((), win32::Win32Impl), #[cfg(target_os = "macos")] @@ -137,17 +137,15 @@ impl Context { let imple: ContextDispatch = match raw_display_handle { #[cfg(x11_platform)] RawDisplayHandle::Xlib(xlib_handle) => unsafe { - ContextDispatch::X11(Arc::new(x11::X11DisplayImpl::from_xlib(xlib_handle)?)) + ContextDispatch::X11(Rc::new(x11::X11DisplayImpl::from_xlib(xlib_handle)?)) }, #[cfg(x11_platform)] RawDisplayHandle::Xcb(xcb_handle) => unsafe { - ContextDispatch::X11(Arc::new(x11::X11DisplayImpl::from_xcb(xcb_handle)?)) + ContextDispatch::X11(Rc::new(x11::X11DisplayImpl::from_xcb(xcb_handle)?)) }, #[cfg(wayland_platform)] RawDisplayHandle::Wayland(wayland_handle) => unsafe { - ContextDispatch::Wayland(Arc::new(wayland::WaylandDisplayImpl::new( - wayland_handle, - )?)) + ContextDispatch::Wayland(Rc::new(wayland::WaylandDisplayImpl::new(wayland_handle)?)) }, #[cfg(target_os = "windows")] RawDisplayHandle::Windows(_) => ContextDispatch::Win32(()), diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index cfb64e55..e7e90bfb 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -1,6 +1,6 @@ use crate::{error::unwrap, SoftBufferError}; use raw_window_handle::{WaylandDisplayHandle, WaylandWindowHandle}; -use std::sync::{Arc, Mutex}; +use std::{cell::RefCell, rc::Rc}; use wayland_client::{ backend::{Backend, ObjectId}, globals::{registry_queue_init, GlobalListContents}, @@ -15,7 +15,7 @@ struct State; pub struct WaylandDisplayImpl { conn: Connection, - event_queue: Mutex>, + event_queue: RefCell>, qh: QueueHandle, shm: wl_shm::WlShm, } @@ -36,7 +36,7 @@ impl WaylandDisplayImpl { )?; Ok(Self { conn, - event_queue: Mutex::new(event_queue), + event_queue: RefCell::new(event_queue), qh, shm, }) @@ -44,7 +44,7 @@ impl WaylandDisplayImpl { } pub struct WaylandImpl { - display: Arc, + display: Rc, surface: wl_surface::WlSurface, buffers: Option<(WaylandBuffer, WaylandBuffer)>, width: i32, @@ -54,7 +54,7 @@ pub struct WaylandImpl { impl WaylandImpl { pub unsafe fn new( window_handle: WaylandWindowHandle, - display: Arc, + display: Rc, ) -> Result { // SAFETY: Ensured by user let surface_id = unwrap( @@ -96,7 +96,7 @@ impl WaylandImpl { if let Some((_front, back)) = &mut self.buffers { // Block if back buffer not released yet if !back.released() { - let mut event_queue = self.display.event_queue.lock().unwrap(); + let mut event_queue = self.display.event_queue.borrow_mut(); while !back.released() { event_queue.blocking_dispatch(&mut State).unwrap(); } @@ -115,8 +115,7 @@ impl WaylandImpl { let _ = self .display .event_queue - .lock() - .unwrap() + .borrow_mut() .dispatch_pending(&mut State); if let Some((front, back)) = &mut self.buffers { @@ -142,7 +141,7 @@ impl WaylandImpl { self.surface.commit(); } - let _ = self.display.event_queue.lock().unwrap().flush(); + let _ = self.display.event_queue.borrow_mut().flush(); } } diff --git a/src/x11.rs b/src/x11.rs index afde9121..53252f5f 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -9,7 +9,7 @@ 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, sync::Arc}; +use std::{fmt, io, mem, rc::Rc}; use x11_dl::xlib::Display; use x11_dl::xlib_xcb::Xlib_xcb; @@ -94,7 +94,7 @@ impl X11DisplayImpl { /// The handle to an X11 drawing context. pub struct X11Impl { /// X display this window belongs to. - display: Arc, + display: Rc, /// The window to draw to. window: xproto::Window, @@ -151,7 +151,7 @@ impl X11Impl { /// The `XlibWindowHandle` and `XlibDisplayHandle` must be valid. pub unsafe fn from_xlib( window_handle: XlibWindowHandle, - display: Arc, + display: Rc, ) -> Result { let mut xcb_window_handle = XcbWindowHandle::empty(); xcb_window_handle.window = window_handle.window as _; @@ -168,7 +168,7 @@ impl X11Impl { /// The `XcbWindowHandle` and `XcbDisplayHandle` must be valid. pub(crate) unsafe fn from_xcb( window_handle: XcbWindowHandle, - display: Arc, + display: Rc, ) -> Result { log::trace!("new: window_handle={:X}", window_handle.window,); From 82ca6be5224a9c634a3bdd63dd4fa0953d49368b Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 20 Jan 2023 13:35:20 -0800 Subject: [PATCH 16/24] Make `Surface::present()` return a `Result` --- src/cg.rs | 4 +++- src/lib.rs | 6 +++--- src/orbital.rs | 3 ++- src/wayland/mod.rs | 4 +++- src/web.rs | 4 +++- src/win32.rs | 4 +++- src/x11.rs | 11 +++++++---- 7 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/cg.rs b/src/cg.rs index 16a47812..c4f455f3 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -72,7 +72,7 @@ impl CGImpl { buffer.as_mut() } - pub fn present(&mut self) { + pub fn present(&mut self) -> Result<(), SoftBufferError> { if let Some(buffer) = self.buffer.take() { let data_provider = CGDataProvider::from_buffer(Arc::new(Buffer(buffer))); let image = CGImage::new( @@ -102,6 +102,8 @@ impl CGImpl { transaction::commit(); } + + Ok(()) } } diff --git a/src/lib.rs b/src/lib.rs index d78abc71..9160ec6e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -91,7 +91,7 @@ macro_rules! make_dispatch { } } - pub fn present(&mut self) { + pub fn present(&mut self) -> Result<(), SoftBufferError> { match self { $( $(#[$attr])* @@ -306,8 +306,8 @@ impl Surface { /// /// If the caller wishes to synchronize other surface/window changes, such requests must be sent to the /// Wayland compositor before calling this function. - pub fn present(&mut self) { - self.surface_impl.present(); + pub fn present(&mut self) -> Result<(), SoftBufferError> { + self.surface_impl.present() } } diff --git a/src/orbital.rs b/src/orbital.rs index 4db8c8f5..1e6e3ea9 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -68,8 +68,9 @@ impl OrbitalImpl { &mut self.buffer } - pub fn present(&mut self) { + pub fn present(&mut self) -> Result<(), SoftBufferError> { self.set_buffer(&self.buffer, self.width, self.height); + Ok(()) } fn set_buffer(&self, buffer: &[u32], width_u32: u32, height_u32: u32) { diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index e7e90bfb..89047cd8 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -111,7 +111,7 @@ impl WaylandImpl { unsafe { self.buffers.as_mut().unwrap().1.mapped_mut() } } - pub fn present(&mut self) { + pub fn present(&mut self) -> Result<(), SoftBufferError> { let _ = self .display .event_queue @@ -142,6 +142,8 @@ impl WaylandImpl { } let _ = self.display.event_queue.borrow_mut().flush(); + + Ok(()) } } diff --git a/src/web.rs b/src/web.rs index e51fc93a..51c7e996 100644 --- a/src/web.rs +++ b/src/web.rs @@ -109,7 +109,7 @@ impl WebImpl { } /// Push the buffer to the canvas. - pub(crate) fn present(&mut self) { + pub(crate) fn present(&mut self) -> Result<(), SoftBufferError> { // Create a bitmap from the buffer. let bitmap: Vec<_> = self .buffer @@ -124,6 +124,8 @@ impl WebImpl { // This can only throw an error if `data` is detached, which is impossible. self.ctx.put_image_data(&image_data, 0.0, 0.0).unwrap(); + + Ok(()) } } diff --git a/src/win32.rs b/src/win32.rs index 0e7f5906..ab83532b 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -181,7 +181,7 @@ impl Win32Impl { self.buffer.as_mut().map_or(&mut [], Buffer::pixels_mut) } - pub fn present(&mut self) { + pub fn present(&mut self) -> Result<(), SoftBufferError> { if let Some(buffer) = &self.buffer { unsafe { Gdi::BitBlt( @@ -200,5 +200,7 @@ impl Win32Impl { Gdi::ValidateRect(self.window, ptr::null_mut()); } } + + Ok(()) } } diff --git a/src/x11.rs b/src/x11.rs index 53252f5f..3cbde14f 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -274,7 +274,7 @@ impl X11Impl { } /// Push the buffer to the window. - pub(crate) fn present(&mut self) { + pub(crate) fn present(&mut self) -> Result<(), SoftBufferError> { log::trace!("present: window={:X}", self.window); let result = match self.buffer { @@ -338,9 +338,12 @@ impl X11Impl { } }; - if let Err(e) = result { - log::error!("Failed to draw image to window: {}", e); - } + result.map_err(|err| { + SoftBufferError::PlatformError( + Some("Failed to draw image to window".to_string()), + Some(Box::new(err)), + ) + }) } } From bb8b1e508dc2dceb9a2936718e740dee907b35b4 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 25 Jan 2023 09:39:21 -0800 Subject: [PATCH 17/24] examples: Unwrap present errors --- examples/animation.rs | 2 +- examples/fruit.rs | 2 +- examples/libxcb.rs | 2 +- examples/rectangle.rs | 2 +- examples/winit.rs | 2 +- examples/winit_wrong_sized_buffer.rs | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/animation.rs b/examples/animation.rs index ae3f8092..52217855 100644 --- a/examples/animation.rs +++ b/examples/animation.rs @@ -51,7 +51,7 @@ fn main() { surface.resize(width, height); surface.buffer_mut().copy_from_slice(frame); - surface.present(); + surface.present().unwrap(); } Event::MainEventsCleared => { window.request_redraw(); diff --git a/examples/fruit.rs b/examples/fruit.rs index b0006e18..fa86a728 100644 --- a/examples/fruit.rs +++ b/examples/fruit.rs @@ -48,7 +48,7 @@ fn main() { buffer[y as usize * width + x as usize] = color; } - surface.present(); + surface.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/libxcb.rs b/examples/libxcb.rs index e0fa8100..3ebdc627 100644 --- a/examples/libxcb.rs +++ b/examples/libxcb.rs @@ -101,7 +101,7 @@ mod example { surface.resize(width.into(), height.into()); let buffer = surface.buffer_mut(); buffer.fill(RED); - surface.present(); + surface.present().unwrap(); } Event::ConfigureNotify(configure_notify) => { width = configure_notify.width; diff --git a/examples/rectangle.rs b/examples/rectangle.rs index 21dc79ee..1ea4e245 100644 --- a/examples/rectangle.rs +++ b/examples/rectangle.rs @@ -62,7 +62,7 @@ fn main() { // Draw something in the window let buffer = surface.buffer_mut(); redraw(buffer, width as usize, height as usize, flag); - surface.present(); + surface.present().unwrap(); } Event::WindowEvent { diff --git a/examples/winit.rs b/examples/winit.rs index 1a725587..587acd9b 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -46,7 +46,7 @@ fn main() { buffer[index as usize] = blue | (green << 8) | (red << 16); } - surface.present(); + surface.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/winit_wrong_sized_buffer.rs b/examples/winit_wrong_sized_buffer.rs index aeaf9272..4eca36be 100644 --- a/examples/winit_wrong_sized_buffer.rs +++ b/examples/winit_wrong_sized_buffer.rs @@ -45,7 +45,7 @@ fn main() { } } - surface.present(); + surface.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, From 527e1706e69c58a676d87b68c111d2593a584e85 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Wed, 25 Jan 2023 13:41:57 -0800 Subject: [PATCH 18/24] Make `resize` and `buffer_mut` return `Result`s --- README.md | 6 +-- examples/animation.rs | 4 +- examples/fruit.rs | 4 +- examples/libxcb.rs | 4 +- examples/rectangle.rs | 4 +- examples/winit.rs | 4 +- examples/winit_wrong_sized_buffer.rs | 6 ++- src/cg.rs | 7 +-- src/error.rs | 3 ++ src/lib.rs | 10 ++-- src/orbital.rs | 7 +-- src/wayland/mod.rs | 67 ++++++++++++++++-------- src/web.rs | 7 +-- src/win32.rs | 78 ++++++++++++++++------------ src/x11.rs | 50 +++++++++++------- 15 files changed, 158 insertions(+), 103 deletions(-) diff --git a/README.md b/README.md index c5639785..b435844e 100644 --- a/README.md +++ b/README.md @@ -72,9 +72,9 @@ fn main() { let size = window.inner_size(); (size.width, size.height) }; - surface.resize(width, height); + surface.resize(width, height).unwrap(); - let buffer = surface.buffer_mut(); + let buffer = surface.buffer_mut().unwrap(); for index in 0..(width * height) { let y = index / width; let x = index % width; @@ -85,7 +85,7 @@ fn main() { buffer[index as usize] = blue | (green << 8) | (red << 16); } - surface.present(); + surface.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/animation.rs b/examples/animation.rs index 52217855..26a3e9cb 100644 --- a/examples/animation.rs +++ b/examples/animation.rs @@ -49,8 +49,8 @@ fn main() { let frame = &frames[((elapsed * 60.0).round() as usize).clamp(0, 59)]; - surface.resize(width, height); - surface.buffer_mut().copy_from_slice(frame); + surface.resize(width, height).unwrap(); + surface.buffer_mut().unwrap().copy_from_slice(frame); surface.present().unwrap(); } Event::MainEventsCleared => { diff --git a/examples/fruit.rs b/examples/fruit.rs index fa86a728..9231804b 100644 --- a/examples/fruit.rs +++ b/examples/fruit.rs @@ -35,9 +35,9 @@ fn main() { match event { Event::RedrawRequested(window_id) if window_id == window.id() => { - surface.resize(fruit.width(), fruit.height()); + surface.resize(fruit.width(), fruit.height()).unwrap(); - let buffer = surface.buffer_mut(); + let buffer = surface.buffer_mut().unwrap(); let width = fruit.width() as usize; for (x, y, pixel) in fruit.pixels() { let red = pixel.0[0] as u32; diff --git a/examples/libxcb.rs b/examples/libxcb.rs index 3ebdc627..5091e104 100644 --- a/examples/libxcb.rs +++ b/examples/libxcb.rs @@ -98,8 +98,8 @@ mod example { match event { Event::Expose(_) => { // Draw a width x height red rectangle. - surface.resize(width.into(), height.into()); - let buffer = surface.buffer_mut(); + surface.resize(width.into(), height.into()).unwrap(); + let buffer = surface.buffer_mut().unwrap(); buffer.fill(RED); surface.present().unwrap(); } diff --git a/examples/rectangle.rs b/examples/rectangle.rs index 1ea4e245..8b3caffd 100644 --- a/examples/rectangle.rs +++ b/examples/rectangle.rs @@ -57,10 +57,10 @@ fn main() { }; // Resize surface if needed - surface.resize(width, height); + surface.resize(width, height).unwrap(); // Draw something in the window - let buffer = surface.buffer_mut(); + let buffer = surface.buffer_mut().unwrap(); redraw(buffer, width as usize, height as usize, flag); surface.present().unwrap(); } diff --git a/examples/winit.rs b/examples/winit.rs index 587acd9b..e6666fd7 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -33,9 +33,9 @@ fn main() { (size.width, size.height) }; - surface.resize(width, height); + surface.resize(width, height).unwrap(); - let buffer = surface.buffer_mut(); + let buffer = surface.buffer_mut().unwrap(); for index in 0..(width * height) { let y = index / width; let x = index % width; diff --git a/examples/winit_wrong_sized_buffer.rs b/examples/winit_wrong_sized_buffer.rs index 4eca36be..5132126a 100644 --- a/examples/winit_wrong_sized_buffer.rs +++ b/examples/winit_wrong_sized_buffer.rs @@ -31,9 +31,11 @@ fn main() { match event { Event::RedrawRequested(window_id) if window_id == window.id() => { - surface.resize(BUFFER_WIDTH as u32, BUFFER_HEIGHT as u32); + surface + .resize(BUFFER_WIDTH as u32, BUFFER_HEIGHT as u32) + .unwrap(); - let buffer = surface.buffer_mut(); + let buffer = surface.buffer_mut().unwrap(); for y in 0..BUFFER_HEIGHT { for x in 0..BUFFER_WIDTH { let red = x as u32 % 255; diff --git a/src/cg.rs b/src/cg.rs index c4f455f3..401f17f8 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -58,18 +58,19 @@ impl CGImpl { }) } - pub fn resize(&mut self, width: u32, height: u32) { + pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { self.width = width; self.height = height; + Ok(()) } - pub fn buffer_mut(&mut self) -> &mut [u32] { + pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { if self.buffer.is_none() { self.buffer = Some(Vec::new()); } let buffer = self.buffer.as_mut().unwrap(); buffer.resize(self.width as usize * self.height as usize, 0); - buffer.as_mut() + Ok(buffer.as_mut()) } pub fn present(&mut self) -> Result<(), SoftBufferError> { diff --git a/src/error.rs b/src/error.rs index 01ea1028..65b975d5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -27,6 +27,9 @@ pub enum SoftBufferError { #[error("The provided display handle is null.")] IncompleteDisplayHandle, + #[error("Surface size {width}x{height} out of range for backend.")] + SizeOutOfRange { width: u32, height: u32 }, + #[error("Platform error")] PlatformError(Option, Option>), } diff --git a/src/lib.rs b/src/lib.rs index 9160ec6e..f9ead25f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -73,7 +73,7 @@ macro_rules! make_dispatch { } impl SurfaceDispatch { - pub fn resize(&mut self, width: u32, height: u32) { + pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { match self { $( $(#[$attr])* @@ -82,7 +82,7 @@ macro_rules! make_dispatch { } } - pub fn buffer_mut(&mut self) -> &mut [u32] { + pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { match self { $( $(#[$attr])* @@ -262,8 +262,8 @@ impl Surface { /// in the upper-left corner of the window. It is recommended in most production use cases /// to have the buffer fill the entire window. Use your windowing library to find the size /// of the window. - pub fn resize(&mut self, width: u32, height: u32) { - self.surface_impl.resize(width, height); + pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + self.surface_impl.resize(width, height) } /// Return a mutable slice to the buffer that the next frame should be rendered into. The size @@ -287,7 +287,7 @@ impl Surface { /// R: Red channel /// G: Green channel /// B: Blue channel - pub fn buffer_mut(&mut self) -> &mut [u32] { + pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { self.surface_impl.buffer_mut() } diff --git a/src/orbital.rs b/src/orbital.rs index 1e6e3ea9..37e45881 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -57,15 +57,16 @@ impl OrbitalImpl { }) } - pub fn resize(&mut self, width: u32, height: u32) { + pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { self.width = width; self.height = height; + Ok(()) } - pub fn buffer_mut(&mut self) -> &mut [u32] { + pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { self.buffer .resize(self.width as usize * self.height as usize, 0); - &mut self.buffer + Ok(&mut self.buffer) } pub fn present(&mut self) -> Result<(), SoftBufferError> { diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index 89047cd8..081a3ed2 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -1,6 +1,6 @@ use crate::{error::unwrap, SoftBufferError}; use raw_window_handle::{WaylandDisplayHandle, WaylandWindowHandle}; -use std::{cell::RefCell, rc::Rc}; +use std::{cell::RefCell, num::NonZeroI32, rc::Rc}; use wayland_client::{ backend::{Backend, ObjectId}, globals::{registry_queue_init, GlobalListContents}, @@ -47,8 +47,7 @@ pub struct WaylandImpl { display: Rc, surface: wl_surface::WlSurface, buffers: Option<(WaylandBuffer, WaylandBuffer)>, - width: i32, - height: i32, + size: Option<(NonZeroI32, NonZeroI32)>, } impl WaylandImpl { @@ -74,44 +73,69 @@ impl WaylandImpl { display, surface, buffers: Default::default(), - width: 0, - height: 0, + size: None, }) } - // Allocate front and back buffer - fn alloc_buffers(&mut self) { - self.buffers = Some(( - WaylandBuffer::new(&self.display.shm, self.width, self.height, &self.display.qh), - WaylandBuffer::new(&self.display.shm, self.width, self.height, &self.display.qh), - )); + pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + self.size = Some( + (|| { + let width = NonZeroI32::new(i32::try_from(width).ok()?)?; + let height = NonZeroI32::new(i32::try_from(height).ok()?)?; + Some((width, height)) + })() + .ok_or(SoftBufferError::SizeOutOfRange { width, height })?, + ); + Ok(()) } - pub fn resize(&mut self, width: u32, height: u32) { - self.width = width as i32; - self.height = height as i32; - } + pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { + let (width, height) = self + .size + .expect("Must set size of surface before calling `buffer_mut()`"); - pub fn buffer_mut(&mut self) -> &mut [u32] { if let Some((_front, back)) = &mut self.buffers { // Block if back buffer not released yet if !back.released() { let mut event_queue = self.display.event_queue.borrow_mut(); while !back.released() { - event_queue.blocking_dispatch(&mut State).unwrap(); + event_queue.blocking_dispatch(&mut State).map_err(|err| { + SoftBufferError::PlatformError( + Some("Wayland dispatch failure".to_string()), + Some(Box::new(err)), + ) + })?; } } // Resize, if buffer isn't large enough - back.resize(self.width, self.height); + back.resize(width.into(), height.into()); } else { - self.alloc_buffers(); + // Allocate front and back buffer + self.buffers = Some(( + WaylandBuffer::new( + &self.display.shm, + width.into(), + height.into(), + &self.display.qh, + ), + WaylandBuffer::new( + &self.display.shm, + width.into(), + height.into(), + &self.display.qh, + ), + )); }; - unsafe { self.buffers.as_mut().unwrap().1.mapped_mut() } + Ok(unsafe { self.buffers.as_mut().unwrap().1.mapped_mut() }) } pub fn present(&mut self) -> Result<(), SoftBufferError> { + let (width, height) = self + .size + .expect("Must set size of surface before calling `present()`"); + let _ = self .display .event_queue @@ -135,7 +159,8 @@ impl WaylandImpl { self.surface.damage(0, 0, i32::MAX, i32::MAX); } else { // Introduced in version 4, it is an error to use this request in version 3 or lower. - self.surface.damage_buffer(0, 0, self.width, self.height); + self.surface + .damage_buffer(0, 0, width.into(), height.into()); } self.surface.commit(); diff --git a/src/web.rs b/src/web.rs index 51c7e996..98ceca03 100644 --- a/src/web.rs +++ b/src/web.rs @@ -96,16 +96,17 @@ impl WebImpl { } /// Resize the canvas to the given dimensions. - pub(crate) fn resize(&mut self, width: u32, height: u32) { + pub(crate) fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { self.buffer.resize(total_len(width, height), 0); self.canvas.set_width(width); self.canvas.set_height(height); self.width = width; + Ok(()) } /// Get a pointer to the mutable buffer. - pub(crate) fn buffer_mut(&mut self) -> &mut [u32] { - &mut self.buffer + pub(crate) fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { + Ok(&mut self.buffer) } /// Push the buffer to the canvas. diff --git a/src/win32.rs b/src/win32.rs index ab83532b..88a581f6 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -7,6 +7,7 @@ use raw_window_handle::Win32WindowHandle; use std::io; use std::mem; +use std::num::NonZeroI32; use std::ptr::{self, NonNull}; use std::slice; @@ -24,8 +25,8 @@ struct Buffer { dc: Gdi::HDC, bitmap: Gdi::HBITMAP, pixels: NonNull, - width: i32, - height: i32, + width: NonZeroI32, + height: NonZeroI32, } impl Drop for Buffer { @@ -38,7 +39,7 @@ impl Drop for Buffer { } impl Buffer { - fn new(window_dc: Gdi::HDC, width: i32, height: i32) -> Self { + fn new(window_dc: Gdi::HDC, width: NonZeroI32, height: NonZeroI32) -> Self { let dc = unsafe { Gdi::CreateCompatibleDC(window_dc) }; assert!(dc != 0); @@ -46,8 +47,8 @@ impl Buffer { let bitmap_info = BitmapInfo { bmi_header: Gdi::BITMAPINFOHEADER { biSize: mem::size_of::() as u32, - biWidth: width, - biHeight: -height, + biWidth: width.into(), + biHeight: -i32::from(height), biPlanes: 1, biBitCount: 32, biCompression: Gdi::BI_BITFIELDS, @@ -107,7 +108,7 @@ impl Buffer { unsafe { slice::from_raw_parts_mut( self.pixels.as_ptr(), - self.width as usize * self.height as usize, + i32::from(self.width) as usize * i32::from(self.height) as usize, ) } } @@ -163,42 +164,53 @@ impl Win32Impl { }) } - pub fn resize(&mut self, width: u32, height: u32) { + pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + let (width, height) = (|| { + let width = NonZeroI32::new(i32::try_from(width).ok()?)?; + let height = NonZeroI32::new(i32::try_from(height).ok()?)?; + Some((width, height)) + })() + .ok_or(SoftBufferError::SizeOutOfRange { width, height })?; + if let Some(buffer) = self.buffer.as_ref() { - if buffer.width == width as i32 && buffer.height == height as i32 { - return; + if buffer.width == width && buffer.height == height { + return Ok(()); } } - self.buffer = if width != 0 && height != 0 { - Some(Buffer::new(self.dc, width as i32, height as i32)) - } else { - None - } + self.buffer = Some(Buffer::new(self.dc, width, height)); + + Ok(()) } - pub fn buffer_mut(&mut self) -> &mut [u32] { - self.buffer.as_mut().map_or(&mut [], Buffer::pixels_mut) + pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { + Ok(self + .buffer + .as_mut() + .expect("Must set size of surface before calling `buffer_mut()`") + .pixels_mut()) } pub fn present(&mut self) -> Result<(), SoftBufferError> { - if let Some(buffer) = &self.buffer { - unsafe { - Gdi::BitBlt( - self.dc, - 0, - 0, - buffer.width, - buffer.height, - buffer.dc, - 0, - 0, - Gdi::SRCCOPY, - ); - - // Validate the window. - Gdi::ValidateRect(self.window, ptr::null_mut()); - } + let buffer = self + .buffer + .as_ref() + .expect("Must set size of surface before calling `present()`"); + unsafe { + Gdi::BitBlt( + self.dc, + 0, + 0, + buffer.width.into(), + buffer.height.into(), + buffer.dc, + 0, + 0, + Gdi::SRCCOPY, + ); + + // Validate the window. + Gdi::ValidateRect(self.window, ptr::null_mut()); } Ok(()) diff --git a/src/x11.rs b/src/x11.rs index 3cbde14f..53428700 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -230,7 +230,7 @@ impl X11Impl { } /// Resize the internal buffer to the given width and height. - pub(crate) fn resize(&mut self, width: u32, height: u32) { + pub(crate) fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { log::trace!( "resize: window={:X}, size={}x{}", self.window, @@ -239,38 +239,48 @@ impl X11Impl { ); // Width and height should fit in u16. - let width: u16 = width.try_into().expect("Width too large"); - let height: u16 = height.try_into().expect("Height too large"); + let width: u16 = width + .try_into() + .or(Err(SoftBufferError::SizeOutOfRange { width, height }))?; + let height: u16 = height.try_into().or(Err(SoftBufferError::SizeOutOfRange { + width: width.into(), + height, + }))?; + + if width != self.width || height != self.height { + self.buffer + .resize(&self.display.connection, width, height) + .map_err(|err| { + SoftBufferError::PlatformError( + Some("Failed to resize X11 buffer".to_string()), + Some(Box::new(err)), + ) + })?; - if width == self.width && height == self.height { - // Nothing to do. - return; + // We successfully resized the buffer. + self.width = width; + self.height = height; } - match self.buffer.resize(&self.display.connection, width, height) { - Ok(()) => { - // We successfully resized the buffer. - self.width = width; - self.height = height; - } - - Err(e) => { - log::error!("Failed to resize window: {}", e); - } - } + Ok(()) } /// Get a mutable reference to the buffer. - pub(crate) fn buffer_mut(&mut self) -> &mut [u32] { + pub(crate) fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { log::trace!("buffer_mut: window={:X}", self.window); let buffer = self .buffer .buffer_mut(&self.display.connection) - .expect("Failed to get buffer"); + .map_err(|err| { + SoftBufferError::PlatformError( + Some("Failed to get mutable X11 buffer".to_string()), + Some(Box::new(err)), + ) + })?; // Crop it down to the window size. - &mut buffer[..total_len(self.width, self.height) / 4] + Ok(&mut buffer[..total_len(self.width, self.height) / 4]) } /// Push the buffer to the window. From e38809e3918c75aaf850ee6df757cbb3a208c0e4 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 26 Jan 2023 11:38:02 -0800 Subject: [PATCH 19/24] Separate `Buffer` type, with a `present` method This is similar to the API used in `winit`. It makes it impossible to present without calling `buffer_mut`, and allows `Buffer` to contain custom data and have custom drop behavior if `present` happens to not be called. This adds a utility type called `BorrowStack` to allow `Buffer` to contain a reference to the surface and also a slice that may borrow from it. There is a miri test to check this conforms to soundness rules. The Orbital implementation here uses a no-copy method if the buffer has the same size as the window, and a separate vec and copy otherwise. --- .github/workflows/ci.yml | 11 +++ README.md | 4 +- examples/animation.rs | 5 +- examples/fruit.rs | 4 +- examples/libxcb.rs | 4 +- examples/rectangle.rs | 6 +- examples/winit.rs | 4 +- examples/winit_wrong_sized_buffer.rs | 5 +- src/cg.rs | 88 ++++++++++-------- src/lib.rs | 84 +++++++++++++++--- src/orbital.rs | 128 +++++++++++++++++++-------- src/util.rs | 63 +++++++++++++ src/wayland/mod.rs | 43 ++++++--- src/web.rs | 25 ++++-- src/win32.rs | 40 ++++++--- src/x11.rs | 82 ++++++++++------- 16 files changed, 426 insertions(+), 170 deletions(-) create mode 100644 src/util.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54c01371..64019134 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,17 @@ jobs: - name: Check Formatting run: cargo +stable fmt --all -- --check + miri-tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: hecrj/setup-rust-action@v1 + with: + rust-version: nightly + components: miri + - name: Run tests with miri + run: cargo +nightly miri test + tests: name: Tests strategy: diff --git a/README.md b/README.md index b435844e..e96e4026 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ fn main() { }; surface.resize(width, height).unwrap(); - let buffer = surface.buffer_mut().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); for index in 0..(width * height) { let y = index / width; let x = index % width; @@ -85,7 +85,7 @@ fn main() { buffer[index as usize] = blue | (green << 8) | (red << 16); } - surface.present().unwrap(); + buffer.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/animation.rs b/examples/animation.rs index 26a3e9cb..ddb9e1d0 100644 --- a/examples/animation.rs +++ b/examples/animation.rs @@ -50,8 +50,9 @@ fn main() { let frame = &frames[((elapsed * 60.0).round() as usize).clamp(0, 59)]; surface.resize(width, height).unwrap(); - surface.buffer_mut().unwrap().copy_from_slice(frame); - surface.present().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); + buffer.copy_from_slice(frame); + buffer.present().unwrap(); } Event::MainEventsCleared => { window.request_redraw(); diff --git a/examples/fruit.rs b/examples/fruit.rs index 9231804b..7c19928c 100644 --- a/examples/fruit.rs +++ b/examples/fruit.rs @@ -37,7 +37,7 @@ fn main() { Event::RedrawRequested(window_id) if window_id == window.id() => { surface.resize(fruit.width(), fruit.height()).unwrap(); - let buffer = surface.buffer_mut().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); let width = fruit.width() as usize; for (x, y, pixel) in fruit.pixels() { let red = pixel.0[0] as u32; @@ -48,7 +48,7 @@ fn main() { buffer[y as usize * width + x as usize] = color; } - surface.present().unwrap(); + buffer.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/libxcb.rs b/examples/libxcb.rs index 5091e104..54e74b1e 100644 --- a/examples/libxcb.rs +++ b/examples/libxcb.rs @@ -99,9 +99,9 @@ mod example { Event::Expose(_) => { // Draw a width x height red rectangle. surface.resize(width.into(), height.into()).unwrap(); - let buffer = surface.buffer_mut().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); buffer.fill(RED); - surface.present().unwrap(); + buffer.present().unwrap(); } Event::ConfigureNotify(configure_notify) => { width = configure_notify.width; diff --git a/examples/rectangle.rs b/examples/rectangle.rs index 8b3caffd..a8d1d6f5 100644 --- a/examples/rectangle.rs +++ b/examples/rectangle.rs @@ -60,9 +60,9 @@ fn main() { surface.resize(width, height).unwrap(); // Draw something in the window - let buffer = surface.buffer_mut().unwrap(); - redraw(buffer, width as usize, height as usize, flag); - surface.present().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); + redraw(&mut buffer, width as usize, height as usize, flag); + buffer.present().unwrap(); } Event::WindowEvent { diff --git a/examples/winit.rs b/examples/winit.rs index e6666fd7..48b9d481 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -35,7 +35,7 @@ fn main() { surface.resize(width, height).unwrap(); - let buffer = surface.buffer_mut().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); for index in 0..(width * height) { let y = index / width; let x = index % width; @@ -46,7 +46,7 @@ fn main() { buffer[index as usize] = blue | (green << 8) | (red << 16); } - surface.present().unwrap(); + buffer.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/examples/winit_wrong_sized_buffer.rs b/examples/winit_wrong_sized_buffer.rs index 5132126a..017361bd 100644 --- a/examples/winit_wrong_sized_buffer.rs +++ b/examples/winit_wrong_sized_buffer.rs @@ -35,7 +35,7 @@ fn main() { .resize(BUFFER_WIDTH as u32, BUFFER_HEIGHT as u32) .unwrap(); - let buffer = surface.buffer_mut().unwrap(); + let mut buffer = surface.buffer_mut().unwrap(); for y in 0..BUFFER_HEIGHT { for x in 0..BUFFER_WIDTH { let red = x as u32 % 255; @@ -46,8 +46,7 @@ fn main() { buffer[y * BUFFER_WIDTH + x] = color; } } - - surface.present().unwrap(); + buffer.present().unwrap(); } Event::WindowEvent { event: WindowEvent::CloseRequested, diff --git a/src/cg.rs b/src/cg.rs index 401f17f8..349ee806 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -26,7 +26,6 @@ pub struct CGImpl { layer: CALayer, window: id, color_space: CGColorSpace, - buffer: Option>, width: u32, height: u32, } @@ -54,7 +53,6 @@ impl CGImpl { color_space, width: 0, height: 0, - buffer: None, }) } @@ -64,45 +62,57 @@ impl CGImpl { Ok(()) } - pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { - if self.buffer.is_none() { - self.buffer = Some(Vec::new()); - } - let buffer = self.buffer.as_mut().unwrap(); - buffer.resize(self.width as usize * self.height as usize, 0); - Ok(buffer.as_mut()) + pub fn buffer_mut(&mut self) -> Result { + Ok(BufferImpl { + buffer: vec![0; self.width as usize * self.height as usize], + imp: self, + }) } +} - pub fn present(&mut self) -> Result<(), SoftBufferError> { - if let Some(buffer) = self.buffer.take() { - let data_provider = CGDataProvider::from_buffer(Arc::new(Buffer(buffer))); - let image = CGImage::new( - self.width as usize, - self.height as usize, - 8, - 32, - (self.width * 4) as usize, - &self.color_space, - kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, - &data_provider, - false, - kCGRenderingIntentDefault, - ); - - // The CALayer has a default action associated with a change in the layer contents, causing - // a quarter second fade transition to happen every time a new buffer is applied. This can - // be mitigated by wrapping the operation in a transaction and disabling all actions. - transaction::begin(); - transaction::set_disable_actions(true); - - unsafe { - self.layer - .set_contents_scale(self.window.backingScaleFactor()); - self.layer.set_contents(image.as_ptr() as id); - }; - - transaction::commit(); - } +pub struct BufferImpl<'a> { + imp: &'a mut CGImpl, + buffer: Vec, +} + +impl<'a> BufferImpl<'a> { + pub fn pixels(&self) -> &[u32] { + &self.buffer + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + &mut self.buffer + } + + pub fn present(self) -> Result<(), SoftBufferError> { + let data_provider = CGDataProvider::from_buffer(Arc::new(Buffer(self.buffer))); + let image = CGImage::new( + self.imp.width as usize, + self.imp.height as usize, + 8, + 32, + (self.imp.width * 4) as usize, + &self.imp.color_space, + kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst, + &data_provider, + false, + kCGRenderingIntentDefault, + ); + + // The CALayer has a default action associated with a change in the layer contents, causing + // a quarter second fade transition to happen every time a new buffer is applied. This can + // be mitigated by wrapping the operation in a transaction and disabling all actions. + transaction::begin(); + transaction::set_disable_actions(true); + + unsafe { + self.imp + .layer + .set_contents_scale(self.imp.window.backingScaleFactor()); + self.imp.layer.set_contents(image.as_ptr() as id); + }; + + transaction::commit(); Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index f9ead25f..42104022 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,8 +20,10 @@ mod win32; mod x11; mod error; +mod util; use std::marker::PhantomData; +use std::ops; #[cfg(any(wayland_platform, x11_platform))] use std::rc::Rc; @@ -44,7 +46,7 @@ macro_rules! make_dispatch { ( $( $(#[$attr:meta])* - $name: ident ($context_inner: ty, $surface_inner : ty), + $name: ident ($context_inner: ty, $surface_inner: ty, $buffer_inner: ty), )* ) => { enum ContextDispatch { @@ -82,16 +84,43 @@ macro_rules! make_dispatch { } } - pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { + pub fn buffer_mut(&mut self) -> Result { match self { $( $(#[$attr])* - Self::$name(inner) => inner.buffer_mut(), + Self::$name(inner) => Ok(BufferDispatch::$name(inner.buffer_mut()?)), + )* + } + } + } + + enum BufferDispatch<'a> { + $( + $(#[$attr])* + $name($buffer_inner), + )* + } + + impl<'a> BufferDispatch<'a> { + pub fn pixels(&self) -> &[u32] { + match self { + $( + $(#[$attr])* + Self::$name(inner) => inner.pixels(), + )* + } + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + match self { + $( + $(#[$attr])* + Self::$name(inner) => inner.pixels_mut(), )* } } - pub fn present(&mut self) -> Result<(), SoftBufferError> { + pub fn present(self) -> Result<(), SoftBufferError> { match self { $( $(#[$attr])* @@ -103,19 +132,22 @@ macro_rules! make_dispatch { }; } +// XXX empty enum with generic bound is invalid? +// XXX deref + make_dispatch! { #[cfg(x11_platform)] - X11(Rc, x11::X11Impl), + X11(Rc, x11::X11Impl, x11::BufferImpl<'a>), #[cfg(wayland_platform)] - Wayland(Rc, wayland::WaylandImpl), + Wayland(Rc, wayland::WaylandImpl, wayland::BufferImpl<'a>), #[cfg(target_os = "windows")] - Win32((), win32::Win32Impl), + Win32((), win32::Win32Impl, win32::BufferImpl<'a>), #[cfg(target_os = "macos")] - CG((), cg::CGImpl), + CG((), cg::CGImpl, cg::BufferImpl<'a>), #[cfg(target_arch = "wasm32")] - Web(web::WebDisplayImpl, web::WebImpl), + Web(web::WebDisplayImpl, web::WebImpl, web::BufferImpl<'a>), #[cfg(target_os = "redox")] - Orbital((), orbital::OrbitalImpl), + Orbital((), orbital::OrbitalImpl, orbital::BufferImpl<'a>), } impl Context { @@ -287,10 +319,20 @@ impl Surface { /// R: Red channel /// G: Green channel /// B: Blue channel - pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { - self.surface_impl.buffer_mut() + pub fn buffer_mut(&mut self) -> Result { + Ok(Buffer { + buffer_impl: self.surface_impl.buffer_mut()?, + _marker: PhantomData, + }) } +} +pub struct Buffer<'a> { + buffer_impl: BufferDispatch<'a>, + _marker: PhantomData<*mut ()>, +} + +impl<'a> Buffer<'a> { /// Presents buffer returned by [`Surface::buffer_mut`] to the window. /// /// # Platform dependent behavior @@ -306,8 +348,22 @@ impl Surface { /// /// If the caller wishes to synchronize other surface/window changes, such requests must be sent to the /// Wayland compositor before calling this function. - pub fn present(&mut self) -> Result<(), SoftBufferError> { - self.surface_impl.present() + pub fn present(self) -> Result<(), SoftBufferError> { + self.buffer_impl.present() + } +} + +impl<'a> ops::Deref for Buffer<'a> { + type Target = [u32]; + + fn deref(&self) -> &[u32] { + self.buffer_impl.pixels() + } +} + +impl<'a> ops::DerefMut for Buffer<'a> { + fn deref_mut(&mut self) -> &mut [u32] { + self.buffer_impl.pixels_mut() } } diff --git a/src/orbital.rs b/src/orbital.rs index 37e45881..cfaabb6e 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -6,6 +6,7 @@ use crate::SoftBufferError; struct OrbitalMap { address: usize, size: usize, + size_unaligned: usize, } impl OrbitalMap { @@ -27,7 +28,19 @@ impl OrbitalMap { )? }; - Ok(Self { address, size }) + Ok(Self { + address, + size, + size_unaligned, + }) + } + + unsafe fn data(&self) -> &[u32] { + unsafe { slice::from_raw_parts(self.address as *const u32, self.size_unaligned / 4) } + } + + unsafe fn data_mut(&self) -> &mut [u32] { + unsafe { slice::from_raw_parts_mut(self.address as *mut u32, self.size_unaligned / 4) } } } @@ -44,7 +57,6 @@ pub struct OrbitalImpl { handle: OrbitalWindowHandle, width: u32, height: u32, - buffer: Vec, } impl OrbitalImpl { @@ -53,7 +65,6 @@ impl OrbitalImpl { handle, width: 0, height: 0, - buffer: Vec::new(), }) } @@ -63,50 +74,56 @@ impl OrbitalImpl { Ok(()) } - pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { - self.buffer - .resize(self.width as usize * self.height as usize, 0); - Ok(&mut self.buffer) + fn window_fd(&self) -> usize { + self.handle.window as usize } - pub fn present(&mut self) -> Result<(), SoftBufferError> { - self.set_buffer(&self.buffer, self.width, self.height); - Ok(()) + // Read the current width and size + fn window_size(&self) -> (usize, usize) { + let mut window_width = 0; + let mut window_height = 0; + + let mut buf: [u8; 4096] = [0; 4096]; + let count = syscall::fpath(self.window_fd(), &mut buf).unwrap(); + let path = str::from_utf8(&buf[..count]).unwrap(); + // orbital:/x/y/w/h/t + let mut parts = path.split('/').skip(3); + if let Some(w) = parts.next() { + window_width = w.parse::().unwrap_or(0); + } + if let Some(h) = parts.next() { + window_height = h.parse::().unwrap_or(0); + } + + (window_width, window_height) } - fn set_buffer(&self, buffer: &[u32], width_u32: u32, height_u32: u32) { - let window_fd = self.handle.window as usize; + pub fn buffer_mut(&mut self) -> Result { + let (window_width, window_height) = self.window_size(); + let pixels = if self.width as usize == window_width && self.height as usize == window_height + { + Pixels::Mapping( + unsafe { OrbitalMap::new(self.window_fd(), window_width * window_height * 4) } + .expect("failed to map orbital window"), + ) + } else { + Pixels::Buffer(vec![0; self.width as usize * self.height as usize]) + }; + Ok(BufferImpl { imp: self, pixels }) + } + fn set_buffer(&self, buffer: &[u32], width_u32: u32, height_u32: u32) { // Read the current width and size - let mut window_width = 0; - let mut window_height = 0; - { - let mut buf: [u8; 4096] = [0; 4096]; - let count = syscall::fpath(window_fd, &mut buf).unwrap(); - let path = str::from_utf8(&buf[..count]).unwrap(); - // orbital:/x/y/w/h/t - let mut parts = path.split('/').skip(3); - if let Some(w) = parts.next() { - window_width = w.parse::().unwrap_or(0); - } - if let Some(h) = parts.next() { - window_height = h.parse::().unwrap_or(0); - } - } + let (window_width, window_height) = self.window_size(); { // Map window buffer let window_map = - unsafe { OrbitalMap::new(window_fd, window_width * window_height * 4) } + unsafe { OrbitalMap::new(self.window_fd(), window_width * window_height * 4) } .expect("failed to map orbital window"); // Window buffer is u32 color data in 0xAABBGGRR format - let window_data = unsafe { - slice::from_raw_parts_mut( - window_map.address as *mut u32, - window_width * window_height, - ) - }; + let window_data = unsafe { window_map.data_mut() }; // Copy each line, cropping to fit let width = width_u32 as usize; @@ -124,6 +141,47 @@ impl OrbitalImpl { } // Tell orbital to show the latest window data - syscall::fsync(window_fd).expect("failed to sync orbital window"); + syscall::fsync(self.window_fd()).expect("failed to sync orbital window"); + } +} + +enum Pixels { + Mapping(OrbitalMap), + Buffer(Vec), +} + +pub struct BufferImpl<'a> { + imp: &'a mut OrbitalImpl, + pixels: Pixels, +} + +impl<'a> BufferImpl<'a> { + pub fn pixels(&self) -> &[u32] { + match &self.pixels { + Pixels::Mapping(mapping) => unsafe { mapping.data() }, + Pixels::Buffer(buffer) => buffer, + } + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + match &mut self.pixels { + Pixels::Mapping(mapping) => unsafe { mapping.data_mut() }, + Pixels::Buffer(buffer) => buffer, + } + } + + pub fn present(self) -> Result<(), SoftBufferError> { + match self.pixels { + Pixels::Mapping(mapping) => { + drop(mapping); + syscall::fsync(self.imp.window_fd()).expect("failed to sync orbital window"); + } + Pixels::Buffer(buffer) => { + self.imp + .set_buffer(&buffer, self.imp.width, self.imp.height); + } + } + + Ok(()) } } diff --git a/src/util.rs b/src/util.rs new file mode 100644 index 00000000..cce639ef --- /dev/null +++ b/src/util.rs @@ -0,0 +1,63 @@ +// Not needed on all platforms +#![allow(dead_code)] + +use crate::SoftBufferError; + +/// Takes a mutable reference to a container and a function deriving a +/// reference into it, and stores both, making it possible to get back the +/// reference to the container once the other reference is no longer needed. +/// +/// This should be consistent with stacked borrow rules, and miri seems to +/// accept it at least in simple cases. +pub struct BorrowStack<'a, T: 'static + ?Sized, U: 'static + ?Sized> { + container: *mut T, + member: *mut U, + _phantom: std::marker::PhantomData<&'a mut T>, +} + +impl<'a, T: 'static + ?Sized, U: 'static + ?Sized> BorrowStack<'a, T, U> { + pub fn new(container: &'a mut T, f: F) -> Result + where + F: for<'b> FnOnce(&'b mut T) -> Result<&'b mut U, SoftBufferError>, + { + let container = container as *mut T; + let member = f(unsafe { &mut *container })? as *mut U; + Ok(Self { + container, + member, + _phantom: std::marker::PhantomData, + }) + } + + pub fn member(&self) -> &U { + unsafe { &*self.member } + } + + pub fn member_mut(&mut self) -> &mut U { + unsafe { &mut *self.member } + } + + pub fn into_container(self) -> &'a mut T { + // SAFETY: Since we consume self and no longer reference member, this + // mutable reference is unique. + unsafe { &mut *self.container } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_borrowstack_slice_int() { + fn f(mut stack: BorrowStack<[u32], u32>) { + assert_eq!(*stack.member(), 3); + *stack.member_mut() = 42; + assert_eq!(stack.into_container(), &[1, 2, 42, 4, 5]); + } + + let mut v = vec![1, 2, 3, 4, 5]; + f(BorrowStack::new(v.as_mut(), |v: &mut [u32]| Ok(&mut v[2])).unwrap()); + assert_eq!(&v, &[1, 2, 42, 4, 5]); + } +} diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index 081a3ed2..3ceff90a 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -1,4 +1,4 @@ -use crate::{error::unwrap, SoftBufferError}; +use crate::{error::unwrap, util, SoftBufferError}; use raw_window_handle::{WaylandDisplayHandle, WaylandWindowHandle}; use std::{cell::RefCell, num::NonZeroI32, rc::Rc}; use wayland_client::{ @@ -89,7 +89,7 @@ impl WaylandImpl { Ok(()) } - pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { + pub fn buffer_mut(&mut self) -> Result { let (width, height) = self .size .expect("Must set size of surface before calling `buffer_mut()`"); @@ -128,45 +128,60 @@ impl WaylandImpl { )); }; - Ok(unsafe { self.buffers.as_mut().unwrap().1.mapped_mut() }) + Ok(BufferImpl(util::BorrowStack::new(self, |buffer| { + Ok(unsafe { buffer.buffers.as_mut().unwrap().1.mapped_mut() }) + })?)) } +} - pub fn present(&mut self) -> Result<(), SoftBufferError> { - let (width, height) = self +pub struct BufferImpl<'a>(util::BorrowStack<'a, WaylandImpl, [u32]>); + +impl<'a> BufferImpl<'a> { + pub fn pixels(&self) -> &[u32] { + self.0.member() + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + self.0.member_mut() + } + + pub fn present(self) -> Result<(), SoftBufferError> { + let imp = self.0.into_container(); + + let (width, height) = imp .size .expect("Must set size of surface before calling `present()`"); - let _ = self + let _ = imp .display .event_queue .borrow_mut() .dispatch_pending(&mut State); - if let Some((front, back)) = &mut self.buffers { + if let Some((front, back)) = &mut imp.buffers { // Swap front and back buffer std::mem::swap(front, back); - front.attach(&self.surface); + front.attach(&imp.surface); // FIXME: Proper damaging mechanism. // // In order to propagate changes on compositors which track damage, for now damage the entire surface. - if self.surface.version() < 4 { + if imp.surface.version() < 4 { // FIXME: Accommodate scale factor since wl_surface::damage is in terms of surface coordinates while // wl_surface::damage_buffer is in buffer coordinates. // // i32::MAX is a valid damage box (most compositors interpret the damage box as "the entire surface") - self.surface.damage(0, 0, i32::MAX, i32::MAX); + imp.surface.damage(0, 0, i32::MAX, i32::MAX); } else { // Introduced in version 4, it is an error to use this request in version 3 or lower. - self.surface - .damage_buffer(0, 0, width.into(), height.into()); + imp.surface.damage_buffer(0, 0, width.into(), height.into()); } - self.surface.commit(); + imp.surface.commit(); } - let _ = self.display.event_queue.borrow_mut().flush(); + let _ = imp.display.event_queue.borrow_mut().flush(); Ok(()) } diff --git a/src/web.rs b/src/web.rs index 98ceca03..a3c9068f 100644 --- a/src/web.rs +++ b/src/web.rs @@ -105,14 +105,29 @@ impl WebImpl { } /// Get a pointer to the mutable buffer. - pub(crate) fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { - Ok(&mut self.buffer) + pub(crate) fn buffer_mut(&mut self) -> Result { + Ok(BufferImpl { imp: self }) + } +} + +pub struct BufferImpl<'a> { + imp: &'a mut WebImpl, +} + +impl<'a> BufferImpl<'a> { + pub fn pixels(&self) -> &[u32] { + &self.imp.buffer + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + &mut self.imp.buffer } /// Push the buffer to the canvas. - pub(crate) fn present(&mut self) -> Result<(), SoftBufferError> { + pub fn present(self) -> Result<(), SoftBufferError> { // Create a bitmap from the buffer. let bitmap: Vec<_> = self + .imp .buffer .iter() .copied() @@ -121,10 +136,10 @@ impl WebImpl { // This should only throw an error if the buffer we pass's size is incorrect. let image_data = - ImageData::new_with_u8_clamped_array(Clamped(&bitmap), self.width).unwrap(); + ImageData::new_with_u8_clamped_array(Clamped(&bitmap), self.imp.width).unwrap(); // This can only throw an error if `data` is detached, which is impossible. - self.ctx.put_image_data(&image_data, 0.0, 0.0).unwrap(); + self.imp.ctx.put_image_data(&image_data, 0.0, 0.0).unwrap(); Ok(()) } diff --git a/src/win32.rs b/src/win32.rs index 88a581f6..8d0b67b7 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -2,7 +2,7 @@ //! //! This module converts the input buffer into a bitmap and then stretches it to the window. -use crate::SoftBufferError; +use crate::{util, SoftBufferError}; use raw_window_handle::Win32WindowHandle; use std::io; @@ -183,22 +183,34 @@ impl Win32Impl { Ok(()) } - pub fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { - Ok(self - .buffer - .as_mut() - .expect("Must set size of surface before calling `buffer_mut()`") - .pixels_mut()) + pub fn buffer_mut(&mut self) -> Result { + Ok(BufferImpl(util::BorrowStack::new(self, |surface| { + Ok(surface + .buffer + .as_mut() + .expect("Must set size of surface before calling `buffer_mut()`") + .pixels_mut()) + })?)) + } +} + +pub struct BufferImpl<'a>(util::BorrowStack<'a, Win32Impl, [u32]>); + +impl<'a> BufferImpl<'a> { + pub fn pixels(&self) -> &[u32] { + self.0.member() + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + self.0.member_mut() } - pub fn present(&mut self) -> Result<(), SoftBufferError> { - let buffer = self - .buffer - .as_ref() - .expect("Must set size of surface before calling `present()`"); + pub fn present(self) -> Result<(), SoftBufferError> { + let imp = self.0.into_container(); + let buffer = imp.buffer.as_ref().unwrap(); unsafe { Gdi::BitBlt( - self.dc, + imp.dc, 0, 0, buffer.width.into(), @@ -210,7 +222,7 @@ impl Win32Impl { ); // Validate the window. - Gdi::ValidateRect(self.window, ptr::null_mut()); + Gdi::ValidateRect(imp.window, ptr::null_mut()); } Ok(()) diff --git a/src/x11.rs b/src/x11.rs index 53428700..49ae59c0 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -5,7 +5,7 @@ #![allow(clippy::uninlined_format_args)] -use crate::SoftBufferError; +use crate::{util, 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}; @@ -266,44 +266,60 @@ impl X11Impl { } /// Get a mutable reference to the buffer. - pub(crate) fn buffer_mut(&mut self) -> Result<&mut [u32], SoftBufferError> { + pub(crate) fn buffer_mut(&mut self) -> Result { log::trace!("buffer_mut: window={:X}", self.window); - let buffer = self - .buffer - .buffer_mut(&self.display.connection) - .map_err(|err| { - SoftBufferError::PlatformError( - Some("Failed to get mutable X11 buffer".to_string()), - Some(Box::new(err)), - ) - })?; - - // Crop it down to the window size. - Ok(&mut buffer[..total_len(self.width, self.height) / 4]) + Ok(BufferImpl(util::BorrowStack::new(self, |surface| { + let buffer = surface + .buffer + .buffer_mut(&surface.display.connection) + .map_err(|err| { + SoftBufferError::PlatformError( + Some("Failed to get mutable X11 buffer".to_string()), + Some(Box::new(err)), + ) + })?; + + // Crop it down to the window size. + Ok(&mut buffer[..total_len(surface.width, surface.height) / 4]) + })?)) + } +} + +pub struct BufferImpl<'a>(util::BorrowStack<'a, X11Impl, [u32]>); + +impl<'a> BufferImpl<'a> { + pub fn pixels(&self) -> &[u32] { + self.0.member() + } + + pub fn pixels_mut(&mut self) -> &mut [u32] { + self.0.member_mut() } /// Push the buffer to the window. - pub(crate) fn present(&mut self) -> Result<(), SoftBufferError> { - log::trace!("present: window={:X}", self.window); + pub fn present(self) -> Result<(), SoftBufferError> { + let imp = self.0.into_container(); + + log::trace!("present: window={:X}", imp.window); - let result = match self.buffer { + let result = match imp.buffer { Buffer::Wire(ref wire) => { // This is a suboptimal strategy, raise a stink in the debug logs. log::debug!("Falling back to non-SHM method for window drawing."); - self.display + imp.display .connection .put_image( xproto::ImageFormat::Z_PIXMAP, - self.window, - self.gc, - self.width, - self.height, + imp.window, + imp.gc, + imp.width, + imp.height, 0, 0, 0, - self.depth, + imp.depth, bytemuck::cast_slice(wire), ) .map(|c| c.ignore_error()) @@ -312,24 +328,24 @@ impl X11Impl { Buffer::Shm(ref mut shm) => { // If the X server is still processing the last image, wait for it to finish. - shm.finish_wait(&self.display.connection) + shm.finish_wait(&imp.display.connection) .and_then(|()| { // Put the image into the window. if let Some((_, segment_id)) = shm.seg { - self.display + imp.display .connection .shm_put_image( - self.window, - self.gc, - self.width, - self.height, + imp.window, + imp.gc, + imp.width, + imp.height, 0, 0, - self.width, - self.height, + imp.width, + imp.height, 0, 0, - self.depth, + imp.depth, xproto::ImageFormat::Z_PIXMAP.into(), false, segment_id, @@ -343,7 +359,7 @@ impl X11Impl { }) .and_then(|()| { // Send a short request to act as a notification for when the X server is done processing the image. - shm.begin_wait(&self.display.connection) + shm.begin_wait(&imp.display.connection) }) } }; From a4931bf3f31be9ad32813542d23aae1c06195d0d Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 23 Feb 2023 09:49:09 -0800 Subject: [PATCH 20/24] x11: Use `swbuf_err` instead of `map_err` --- src/x11.rs | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/src/x11.rs b/src/x11.rs index 49ae59c0..594e9e5d 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -250,12 +250,7 @@ impl X11Impl { if width != self.width || height != self.height { self.buffer .resize(&self.display.connection, width, height) - .map_err(|err| { - SoftBufferError::PlatformError( - Some("Failed to resize X11 buffer".to_string()), - Some(Box::new(err)), - ) - })?; + .swbuf_err("Failed to resize X11 buffer")?; // We successfully resized the buffer. self.width = width; @@ -273,12 +268,7 @@ impl X11Impl { let buffer = surface .buffer .buffer_mut(&surface.display.connection) - .map_err(|err| { - SoftBufferError::PlatformError( - Some("Failed to get mutable X11 buffer".to_string()), - Some(Box::new(err)), - ) - })?; + .swbuf_err("Failed to get mutable X11 buffer")?; // Crop it down to the window size. Ok(&mut buffer[..total_len(surface.width, surface.height) / 4]) @@ -364,12 +354,7 @@ impl<'a> BufferImpl<'a> { } }; - result.map_err(|err| { - SoftBufferError::PlatformError( - Some("Failed to draw image to window".to_string()), - Some(Box::new(err)), - ) - }) + result.swbuf_err("Failed to draw image to window") } } From 109e88428577f250b9f4822a9cad0ccf92358d62 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 23 Feb 2023 09:52:26 -0800 Subject: [PATCH 21/24] Make `deref` and `deref_mut` as `#[inline]` We also need to mark the functions these call. Otherwise these things can't be inlined across crate boundaries, without LTO. --- src/cg.rs | 2 ++ src/lib.rs | 5 ++++- src/orbital.rs | 2 ++ src/wayland/mod.rs | 2 ++ src/win32.rs | 2 ++ src/x11.rs | 2 ++ 6 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cg.rs b/src/cg.rs index 349ee806..110848ba 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -76,10 +76,12 @@ pub struct BufferImpl<'a> { } impl<'a> BufferImpl<'a> { + #[inline] pub fn pixels(&self) -> &[u32] { &self.buffer } + #[inline] pub fn pixels_mut(&mut self) -> &mut [u32] { &mut self.buffer } diff --git a/src/lib.rs b/src/lib.rs index 42104022..77c3473f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -102,6 +102,7 @@ macro_rules! make_dispatch { } impl<'a> BufferDispatch<'a> { + #[inline] pub fn pixels(&self) -> &[u32] { match self { $( @@ -111,6 +112,7 @@ macro_rules! make_dispatch { } } + #[inline] pub fn pixels_mut(&mut self) -> &mut [u32] { match self { $( @@ -133,7 +135,6 @@ macro_rules! make_dispatch { } // XXX empty enum with generic bound is invalid? -// XXX deref make_dispatch! { #[cfg(x11_platform)] @@ -356,12 +357,14 @@ impl<'a> Buffer<'a> { impl<'a> ops::Deref for Buffer<'a> { type Target = [u32]; + #[inline] fn deref(&self) -> &[u32] { self.buffer_impl.pixels() } } impl<'a> ops::DerefMut for Buffer<'a> { + #[inline] fn deref_mut(&mut self) -> &mut [u32] { self.buffer_impl.pixels_mut() } diff --git a/src/orbital.rs b/src/orbital.rs index cfaabb6e..35b79bd4 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -156,6 +156,7 @@ pub struct BufferImpl<'a> { } impl<'a> BufferImpl<'a> { + #[inline] pub fn pixels(&self) -> &[u32] { match &self.pixels { Pixels::Mapping(mapping) => unsafe { mapping.data() }, @@ -163,6 +164,7 @@ impl<'a> BufferImpl<'a> { } } + #[inline] pub fn pixels_mut(&mut self) -> &mut [u32] { match &mut self.pixels { Pixels::Mapping(mapping) => unsafe { mapping.data_mut() }, diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index 3ceff90a..9814b750 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -137,10 +137,12 @@ impl WaylandImpl { pub struct BufferImpl<'a>(util::BorrowStack<'a, WaylandImpl, [u32]>); impl<'a> BufferImpl<'a> { + #[inline] pub fn pixels(&self) -> &[u32] { self.0.member() } + #[inline] pub fn pixels_mut(&mut self) -> &mut [u32] { self.0.member_mut() } diff --git a/src/win32.rs b/src/win32.rs index 8d0b67b7..f1b6dba6 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -197,10 +197,12 @@ impl Win32Impl { pub struct BufferImpl<'a>(util::BorrowStack<'a, Win32Impl, [u32]>); impl<'a> BufferImpl<'a> { + #[inline] pub fn pixels(&self) -> &[u32] { self.0.member() } + #[inline] pub fn pixels_mut(&mut self) -> &mut [u32] { self.0.member_mut() } diff --git a/src/x11.rs b/src/x11.rs index 594e9e5d..cf8e2b24 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -279,10 +279,12 @@ impl X11Impl { pub struct BufferImpl<'a>(util::BorrowStack<'a, X11Impl, [u32]>); impl<'a> BufferImpl<'a> { + #[inline] pub fn pixels(&self) -> &[u32] { self.0.member() } + #[inline] pub fn pixels_mut(&mut self) -> &mut [u32] { self.0.member_mut() } From 8d35f0e704d591d92cea44f41f439e896a750ea4 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Thu, 23 Feb 2023 10:28:57 -0800 Subject: [PATCH 22/24] Update documentation of public API --- src/error.rs | 1 + src/lib.rs | 57 +++++++++++++++++++++++++++++----------------------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/src/error.rs b/src/error.rs index 65b975d5..03ec6d3e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -3,6 +3,7 @@ use std::error::Error; use thiserror::Error; #[derive(Error, Debug)] +#[allow(missing_docs)] // TODO #[non_exhaustive] pub enum SoftBufferError { #[error( diff --git a/src/lib.rs b/src/lib.rs index 77c3473f..c9621587 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ #![doc = include_str!("../README.md")] #![deny(unsafe_op_in_unsafe_fn)] +#![warn(missing_docs)] #[cfg(target_os = "macos")] #[macro_use] @@ -205,6 +206,7 @@ impl Context { } } +/// A surface for drawing to a window with software buffers. pub struct Surface { /// This is boxed so that `Surface` is the same size on every platform. surface_impl: Box, @@ -212,7 +214,7 @@ pub struct Surface { } impl Surface { - /// Creates a new instance of this struct, using the provided window and display. + /// Creates a new surface for the context for the provided window. /// /// # Safety /// @@ -225,7 +227,7 @@ impl Surface { unsafe { Self::from_raw(context, window.raw_window_handle()) } } - /// Creates a new instance of this struct, using the provided raw window and display handles + /// Creates a new surface for the context for the provided raw window handle. /// /// # Safety /// @@ -299,27 +301,9 @@ impl Surface { self.surface_impl.resize(width, height) } - /// Return a mutable slice to the buffer that the next frame should be rendered into. The size + /// Return a [`Buffer`] that the next frame should be rendered into. The size must /// be set with [`Surface::resize`] first. The initial contents of the buffer may be zeroed, or /// may contain a previous frame. - /// - /// The format of the buffer is as follows. There is one u32 in the buffer for each pixel in - /// the area to draw. The first entry is the upper-left most pixel. The second is one to the right - /// etc. (Row-major top to bottom left to right one u32 per pixel). Within each u32 the highest - /// order 8 bits are to be set to 0. The next highest order 8 bits are the red channel, then the - /// green channel, and then the blue channel in the lowest-order 8 bits. See the examples for - /// one way to build this format using bitwise operations. - /// - /// -------- - /// - /// Pixel format (u32): - /// - /// 00000000RRRRRRRRGGGGGGGGBBBBBBBB - /// - /// 0: Bit is 0 - /// R: Red channel - /// G: Green channel - /// B: Blue channel pub fn buffer_mut(&mut self) -> Result { Ok(Buffer { buffer_impl: self.surface_impl.buffer_mut()?, @@ -328,19 +312,42 @@ impl Surface { } } +/// A buffer that can be written to by the CPU and presented to the window. +/// +/// This derefs to a `[u32]`, which depending on the backend may be a mapping into shared memory +/// accessible to the display server, so presentation doesn't require any (client-side) copying. +/// +/// This trust the display server not to mutate the buffer, which could otherwise be unsound. +/// +/// # Data representation +/// +/// The format of the buffer is as follows. There is one `u32` in the buffer for each pixel in +/// the area to draw. The first entry is the upper-left most pixel. The second is one to the right +/// etc. (Row-major top to bottom left to right one `u32` per pixel). Within each `u32` the highest +/// order 8 bits are to be set to 0. The next highest order 8 bits are the red channel, then the +/// green channel, and then the blue channel in the lowest-order 8 bits. See the examples for +/// one way to build this format using bitwise operations. +/// +/// -------- +/// +/// Pixel format (`u32`): +/// +/// 00000000RRRRRRRRGGGGGGGGBBBBBBBB +/// +/// 0: Bit is 0 +/// R: Red channel +/// G: Green channel +/// B: Blue channel pub struct Buffer<'a> { buffer_impl: BufferDispatch<'a>, _marker: PhantomData<*mut ()>, } impl<'a> Buffer<'a> { - /// Presents buffer returned by [`Surface::buffer_mut`] to the window. + /// Presents buffer to the window. /// /// # Platform dependent behavior /// - /// This section of the documentation details how some platforms may behave when [`present`](Surface::present) - /// is called. - /// /// ## Wayland /// /// On Wayland, calling this function may send requests to the underlying `wl_surface`. The From 20ab5fda713a5a35cae9550f5f1786b0e1734e95 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Fri, 31 Mar 2023 20:58:44 -0700 Subject: [PATCH 23/24] Updates to documentation for `Surface` --- src/lib.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index c9621587..fbc379f5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -317,7 +317,7 @@ impl Surface { /// This derefs to a `[u32]`, which depending on the backend may be a mapping into shared memory /// accessible to the display server, so presentation doesn't require any (client-side) copying. /// -/// This trust the display server not to mutate the buffer, which could otherwise be unsound. +/// This trusts the display server not to mutate the buffer, which could otherwise be unsound. /// /// # Data representation /// @@ -338,6 +338,16 @@ impl Surface { /// R: Red channel /// G: Green channel /// B: Blue channel +/// +/// # Platform dependent behavior +/// No-copy presentation is currently supported on: +/// - Wayland +/// - X, when XShm is available +/// - Win32 +/// - Orbital, when buffer size matches window size +/// Currently [`Buffer::present`] must block copying image data on: +/// - Web +/// - macOS pub struct Buffer<'a> { buffer_impl: BufferDispatch<'a>, _marker: PhantomData<*mut ()>, From 58e277d6c96942ab19b5160bdced010247e59530 Mon Sep 17 00:00:00 2001 From: Ian Douglas Scott Date: Mon, 3 Apr 2023 18:57:57 -0700 Subject: [PATCH 24/24] Use `NonZeroU32` for arguments to `Surface::resize` --- README.md | 8 +++++++- examples/animation.rs | 8 +++++++- examples/fruit.rs | 8 +++++++- examples/libxcb.rs | 8 +++++++- examples/rectangle.rs | 8 +++++++- examples/winit.rs | 8 +++++++- examples/winit_wrong_sized_buffer.rs | 6 +++++- src/cg.rs | 7 ++++--- src/error.rs | 6 +++++- src/lib.rs | 5 +++-- src/orbital.rs | 8 ++++---- src/wayland/mod.rs | 24 ++++++++++++++---------- src/web.rs | 10 +++++++++- src/win32.rs | 16 ++++++++-------- src/x11.rs | 20 ++++++++++++++------ 15 files changed, 108 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index e96e4026..135342b9 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,7 @@ For now, the priority for new platforms is: Example == ```rust,no_run +use std::num::NonZeroU32; use winit::event::{Event, WindowEvent}; use winit::event_loop::{ControlFlow, EventLoop}; use winit::window::WindowBuilder; @@ -72,7 +73,12 @@ fn main() { let size = window.inner_size(); (size.width, size.height) }; - surface.resize(width, height).unwrap(); + surface + .resize( + NonZeroU32::new(width).unwrap(), + NonZeroU32::new(height).unwrap(), + ) + .unwrap(); let mut buffer = surface.buffer_mut().unwrap(); for index in 0..(width * height) { diff --git a/examples/animation.rs b/examples/animation.rs index ddb9e1d0..f729a395 100644 --- a/examples/animation.rs +++ b/examples/animation.rs @@ -2,6 +2,7 @@ use instant::Instant; #[cfg(not(target_arch = "wasm32"))] use rayon::prelude::*; use std::f64::consts::PI; +use std::num::NonZeroU32; use winit::event::{Event, WindowEvent}; use winit::event_loop::{ControlFlow, EventLoop}; use winit::window::WindowBuilder; @@ -49,7 +50,12 @@ fn main() { let frame = &frames[((elapsed * 60.0).round() as usize).clamp(0, 59)]; - surface.resize(width, height).unwrap(); + surface + .resize( + NonZeroU32::new(width).unwrap(), + NonZeroU32::new(height).unwrap(), + ) + .unwrap(); let mut buffer = surface.buffer_mut().unwrap(); buffer.copy_from_slice(frame); buffer.present().unwrap(); diff --git a/examples/fruit.rs b/examples/fruit.rs index 7c19928c..7e28e89c 100644 --- a/examples/fruit.rs +++ b/examples/fruit.rs @@ -1,4 +1,5 @@ use image::GenericImageView; +use std::num::NonZeroU32; use winit::event::{Event, WindowEvent}; use winit::event_loop::{ControlFlow, EventLoop}; use winit::window::WindowBuilder; @@ -35,7 +36,12 @@ fn main() { match event { Event::RedrawRequested(window_id) if window_id == window.id() => { - surface.resize(fruit.width(), fruit.height()).unwrap(); + surface + .resize( + NonZeroU32::new(fruit.width()).unwrap(), + NonZeroU32::new(fruit.height()).unwrap(), + ) + .unwrap(); let mut buffer = surface.buffer_mut().unwrap(); let width = fruit.width() as usize; diff --git a/examples/libxcb.rs b/examples/libxcb.rs index 54e74b1e..4ea97b01 100644 --- a/examples/libxcb.rs +++ b/examples/libxcb.rs @@ -3,6 +3,7 @@ #[cfg(all(feature = "x11", any(target_os = "linux", target_os = "freebsd")))] mod example { use raw_window_handle::{RawDisplayHandle, RawWindowHandle, XcbDisplayHandle, XcbWindowHandle}; + use std::num::NonZeroU32; use x11rb::{ connection::Connection, protocol::{ @@ -98,7 +99,12 @@ mod example { match event { Event::Expose(_) => { // Draw a width x height red rectangle. - surface.resize(width.into(), height.into()).unwrap(); + surface + .resize( + NonZeroU32::new(width.into()).unwrap(), + NonZeroU32::new(height.into()).unwrap(), + ) + .unwrap(); let mut buffer = surface.buffer_mut().unwrap(); buffer.fill(RED); buffer.present().unwrap(); diff --git a/examples/rectangle.rs b/examples/rectangle.rs index a8d1d6f5..5443d3b9 100644 --- a/examples/rectangle.rs +++ b/examples/rectangle.rs @@ -1,3 +1,4 @@ +use std::num::NonZeroU32; use winit::event::{ElementState, Event, KeyboardInput, VirtualKeyCode, WindowEvent}; use winit::event_loop::{ControlFlow, EventLoop}; use winit::window::WindowBuilder; @@ -57,7 +58,12 @@ fn main() { }; // Resize surface if needed - surface.resize(width, height).unwrap(); + surface + .resize( + NonZeroU32::new(width).unwrap(), + NonZeroU32::new(height).unwrap(), + ) + .unwrap(); // Draw something in the window let mut buffer = surface.buffer_mut().unwrap(); diff --git a/examples/winit.rs b/examples/winit.rs index 48b9d481..18aa2784 100644 --- a/examples/winit.rs +++ b/examples/winit.rs @@ -1,3 +1,4 @@ +use std::num::NonZeroU32; use winit::event::{Event, WindowEvent}; use winit::event_loop::{ControlFlow, EventLoop}; use winit::window::WindowBuilder; @@ -33,7 +34,12 @@ fn main() { (size.width, size.height) }; - surface.resize(width, height).unwrap(); + surface + .resize( + NonZeroU32::new(width).unwrap(), + NonZeroU32::new(height).unwrap(), + ) + .unwrap(); let mut buffer = surface.buffer_mut().unwrap(); for index in 0..(width * height) { diff --git a/examples/winit_wrong_sized_buffer.rs b/examples/winit_wrong_sized_buffer.rs index 017361bd..09aff57b 100644 --- a/examples/winit_wrong_sized_buffer.rs +++ b/examples/winit_wrong_sized_buffer.rs @@ -1,3 +1,4 @@ +use std::num::NonZeroU32; use winit::event::{Event, WindowEvent}; use winit::event_loop::{ControlFlow, EventLoop}; use winit::window::WindowBuilder; @@ -32,7 +33,10 @@ fn main() { match event { Event::RedrawRequested(window_id) if window_id == window.id() => { surface - .resize(BUFFER_WIDTH as u32, BUFFER_HEIGHT as u32) + .resize( + NonZeroU32::new(BUFFER_WIDTH as u32).unwrap(), + NonZeroU32::new(BUFFER_HEIGHT as u32).unwrap(), + ) .unwrap(); let mut buffer = surface.buffer_mut().unwrap(); diff --git a/src/cg.rs b/src/cg.rs index 110848ba..fce628d6 100644 --- a/src/cg.rs +++ b/src/cg.rs @@ -12,6 +12,7 @@ use cocoa::base::{id, nil}; use cocoa::quartzcore::{transaction, CALayer, ContentsGravity}; use foreign_types::ForeignType; +use std::num::NonZeroU32; use std::sync::Arc; struct Buffer(Vec); @@ -56,9 +57,9 @@ impl CGImpl { }) } - pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { - self.width = width; - self.height = height; + pub fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { + self.width = width.get(); + self.height = height.get(); Ok(()) } diff --git a/src/error.rs b/src/error.rs index 03ec6d3e..f84c649d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,6 @@ use raw_window_handle::{RawDisplayHandle, RawWindowHandle}; use std::error::Error; +use std::num::NonZeroU32; use thiserror::Error; #[derive(Error, Debug)] @@ -29,7 +30,10 @@ pub enum SoftBufferError { IncompleteDisplayHandle, #[error("Surface size {width}x{height} out of range for backend.")] - SizeOutOfRange { width: u32, height: u32 }, + SizeOutOfRange { + width: NonZeroU32, + height: NonZeroU32, + }, #[error("Platform error")] PlatformError(Option, Option>), diff --git a/src/lib.rs b/src/lib.rs index fbc379f5..f7dadf63 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ mod error; mod util; use std::marker::PhantomData; +use std::num::NonZeroU32; use std::ops; #[cfg(any(wayland_platform, x11_platform))] use std::rc::Rc; @@ -76,7 +77,7 @@ macro_rules! make_dispatch { } impl SurfaceDispatch { - pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + pub fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { match self { $( $(#[$attr])* @@ -297,7 +298,7 @@ impl Surface { /// in the upper-left corner of the window. It is recommended in most production use cases /// to have the buffer fill the entire window. Use your windowing library to find the size /// of the window. - pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + pub fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { self.surface_impl.resize(width, height) } diff --git a/src/orbital.rs b/src/orbital.rs index 35b79bd4..f999b6a9 100644 --- a/src/orbital.rs +++ b/src/orbital.rs @@ -1,5 +1,5 @@ use raw_window_handle::OrbitalWindowHandle; -use std::{cmp, slice, str}; +use std::{cmp, num::NonZeroU32, slice, str}; use crate::SoftBufferError; @@ -68,9 +68,9 @@ impl OrbitalImpl { }) } - pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { - self.width = width; - self.height = height; + pub fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { + self.width = width.get(); + self.height = height.get(); Ok(()) } diff --git a/src/wayland/mod.rs b/src/wayland/mod.rs index 9814b750..2b95fd6f 100644 --- a/src/wayland/mod.rs +++ b/src/wayland/mod.rs @@ -1,6 +1,10 @@ use crate::{error::unwrap, util, SoftBufferError}; use raw_window_handle::{WaylandDisplayHandle, WaylandWindowHandle}; -use std::{cell::RefCell, num::NonZeroI32, rc::Rc}; +use std::{ + cell::RefCell, + num::{NonZeroI32, NonZeroU32}, + rc::Rc, +}; use wayland_client::{ backend::{Backend, ObjectId}, globals::{registry_queue_init, GlobalListContents}, @@ -77,11 +81,11 @@ impl WaylandImpl { }) } - pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + pub fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { self.size = Some( (|| { - let width = NonZeroI32::new(i32::try_from(width).ok()?)?; - let height = NonZeroI32::new(i32::try_from(height).ok()?)?; + let width = NonZeroI32::try_from(width).ok()?; + let height = NonZeroI32::try_from(height).ok()?; Some((width, height)) })() .ok_or(SoftBufferError::SizeOutOfRange { width, height })?, @@ -109,20 +113,20 @@ impl WaylandImpl { } // Resize, if buffer isn't large enough - back.resize(width.into(), height.into()); + back.resize(width.get(), height.get()); } else { // Allocate front and back buffer self.buffers = Some(( WaylandBuffer::new( &self.display.shm, - width.into(), - height.into(), + width.get(), + height.get(), &self.display.qh, ), WaylandBuffer::new( &self.display.shm, - width.into(), - height.into(), + width.get(), + height.get(), &self.display.qh, ), )); @@ -177,7 +181,7 @@ impl<'a> BufferImpl<'a> { imp.surface.damage(0, 0, i32::MAX, i32::MAX); } else { // Introduced in version 4, it is an error to use this request in version 3 or lower. - imp.surface.damage_buffer(0, 0, width.into(), height.into()); + imp.surface.damage_buffer(0, 0, width.get(), height.get()); } imp.surface.commit(); diff --git a/src/web.rs b/src/web.rs index a3c9068f..49df5e79 100644 --- a/src/web.rs +++ b/src/web.rs @@ -11,6 +11,7 @@ use web_sys::ImageData; use crate::SoftBufferError; use std::convert::TryInto; +use std::num::NonZeroU32; /// Display implementation for the web platform. /// @@ -96,7 +97,14 @@ impl WebImpl { } /// Resize the canvas to the given dimensions. - pub(crate) fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + pub(crate) fn resize( + &mut self, + width: NonZeroU32, + height: NonZeroU32, + ) -> Result<(), SoftBufferError> { + let width = width.get(); + let height = height.get(); + self.buffer.resize(total_len(width, height), 0); self.canvas.set_width(width); self.canvas.set_height(height); diff --git a/src/win32.rs b/src/win32.rs index f1b6dba6..8d9a3501 100644 --- a/src/win32.rs +++ b/src/win32.rs @@ -7,7 +7,7 @@ use raw_window_handle::Win32WindowHandle; use std::io; use std::mem; -use std::num::NonZeroI32; +use std::num::{NonZeroI32, NonZeroU32}; use std::ptr::{self, NonNull}; use std::slice; @@ -47,8 +47,8 @@ impl Buffer { let bitmap_info = BitmapInfo { bmi_header: Gdi::BITMAPINFOHEADER { biSize: mem::size_of::() as u32, - biWidth: width.into(), - biHeight: -i32::from(height), + biWidth: width.get(), + biHeight: -height.get(), biPlanes: 1, biBitCount: 32, biCompression: Gdi::BI_BITFIELDS, @@ -164,10 +164,10 @@ impl Win32Impl { }) } - pub fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + pub fn resize(&mut self, width: NonZeroU32, height: NonZeroU32) -> Result<(), SoftBufferError> { let (width, height) = (|| { - let width = NonZeroI32::new(i32::try_from(width).ok()?)?; - let height = NonZeroI32::new(i32::try_from(height).ok()?)?; + let width = NonZeroI32::new(i32::try_from(width.get()).ok()?)?; + let height = NonZeroI32::new(i32::try_from(height.get()).ok()?)?; Some((width, height)) })() .ok_or(SoftBufferError::SizeOutOfRange { width, height })?; @@ -215,8 +215,8 @@ impl<'a> BufferImpl<'a> { imp.dc, 0, 0, - buffer.width.into(), - buffer.height.into(), + buffer.width.get(), + buffer.height.get(), buffer.dc, 0, 0, diff --git a/src/x11.rs b/src/x11.rs index cf8e2b24..1f5386f7 100644 --- a/src/x11.rs +++ b/src/x11.rs @@ -9,7 +9,7 @@ use crate::{util, 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, rc::Rc}; +use std::{fmt, io, mem, num::NonZeroU32, rc::Rc}; use x11_dl::xlib::Display; use x11_dl::xlib_xcb::Xlib_xcb; @@ -230,7 +230,11 @@ impl X11Impl { } /// Resize the internal buffer to the given width and height. - pub(crate) fn resize(&mut self, width: u32, height: u32) -> Result<(), SoftBufferError> { + pub(crate) fn resize( + &mut self, + width: NonZeroU32, + height: NonZeroU32, + ) -> Result<(), SoftBufferError> { log::trace!( "resize: window={:X}, size={}x{}", self.window, @@ -240,12 +244,16 @@ impl X11Impl { // Width and height should fit in u16. let width: u16 = width + .get() .try_into() .or(Err(SoftBufferError::SizeOutOfRange { width, height }))?; - let height: u16 = height.try_into().or(Err(SoftBufferError::SizeOutOfRange { - width: width.into(), - height, - }))?; + let height: u16 = height + .get() + .try_into() + .or(Err(SoftBufferError::SizeOutOfRange { + width: NonZeroU32::new(width.into()).unwrap(), + height, + }))?; if width != self.width || height != self.height { self.buffer