diff --git a/CHANGELOG.md b/CHANGELOG.md index 59a21cf..88d8400 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - Added `Buffer::pixels_iter()` for iterating over each pixel with its associated `x`/`y` coordinate. - **Breaking:** Removed generic type parameters `D` and `W` from `Buffer<'_>` struct. - **Breaking:** Removed `Deref[Mut]` implementation on `Buffer<'_>`. Use `Buffer::pixels()` instead. +- **Breaking:** Removed `DamageOutOfRange` error case. If the damage value is greater than the backend supports, it is instead clamped to an appropriate value. +- Fixed `present_with_damage` with bounds out of range on Windows, Web and X11. # 0.4.7 diff --git a/src/backend_dispatch.rs b/src/backend_dispatch.rs index f2a9454..32d61a1 100644 --- a/src/backend_dispatch.rs +++ b/src/backend_dispatch.rs @@ -185,15 +185,6 @@ macro_rules! make_dispatch { } } - fn present(self) -> Result<(), SoftBufferError> { - match self { - $( - $(#[$attr])* - Self::$name(inner) => inner.present(), - )* - } - } - fn present_with_damage(self, damage: &[Rect]) -> Result<(), SoftBufferError> { match self { $( diff --git a/src/backend_interface.rs b/src/backend_interface.rs index 1eafbf8..d6e1468 100644 --- a/src/backend_interface.rs +++ b/src/backend_interface.rs @@ -40,5 +40,4 @@ pub(crate) trait BufferInterface { fn pixels_mut(&mut self) -> &mut [u32]; fn age(&self) -> u8; fn present_with_damage(self, damage: &[Rect]) -> Result<(), SoftBufferError>; - fn present(self) -> Result<(), SoftBufferError>; } diff --git a/src/backends/android.rs b/src/backends/android.rs index cc23b9c..c58fa89 100644 --- a/src/backends/android.rs +++ b/src/backends/android.rs @@ -142,7 +142,17 @@ impl BufferInterface for BufferImpl<'_> { } // TODO: This function is pretty slow this way - fn present(mut self) -> Result<(), SoftBufferError> { + fn present_with_damage(mut self, damage: &[Rect]) -> Result<(), SoftBufferError> { + // TODO: Android requires the damage rect _at lock time_ + // Since we're faking the backing buffer _anyway_, we could even fake the surface lock + // and lock it here (if it doesn't influence timings). + // + // Android seems to do this because the region can be expanded by the + // system, requesting the user to actually redraw a larger region. + // It's unclear if/when this is used, or if corruption/artifacts occur + // when the enlarged damage region is not re-rendered? + let _ = damage; + let input_lines = self.buffer.chunks(self.native_window_buffer.width()); for (output, input) in self .native_window_buffer @@ -165,11 +175,4 @@ impl BufferInterface for BufferImpl<'_> { } Ok(()) } - - fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> { - // TODO: Android requires the damage rect _at lock time_ - // Since we're faking the backing buffer _anyway_, we could even fake the surface lock - // and lock it here (if it doesn't influence timings). - self.present() - } } diff --git a/src/backends/cg.rs b/src/backends/cg.rs index f45779f..145cb17 100644 --- a/src/backends/cg.rs +++ b/src/backends/cg.rs @@ -296,7 +296,7 @@ impl BufferInterface for BufferImpl<'_> { 0 } - fn present(self) -> Result<(), SoftBufferError> { + fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> { unsafe extern "C-unwind" fn release( _info: *mut c_void, data: NonNull, @@ -361,10 +361,6 @@ impl BufferInterface for BufferImpl<'_> { CATransaction::commit(); Ok(()) } - - fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> { - self.present() - } } #[derive(Debug)] diff --git a/src/backends/kms.rs b/src/backends/kms.rs index a3a982d..ff6024b 100644 --- a/src/backends/kms.rs +++ b/src/backends/kms.rs @@ -325,24 +325,17 @@ impl BufferInterface for BufferImpl<'_> { #[inline] fn present_with_damage(self, damage: &[crate::Rect]) -> Result<(), SoftBufferError> { - let rectangles = damage + let rectangles: Vec<_> = damage .iter() - .map(|&rect| { - let err = || SoftBufferError::DamageOutOfRange { rect }; - Ok::<_, SoftBufferError>(ClipRect::new( - rect.x.try_into().map_err(|_| err())?, - rect.y.try_into().map_err(|_| err())?, - rect.x - .checked_add(rect.width.get()) - .and_then(|x| x.try_into().ok()) - .ok_or_else(err)?, - rect.y - .checked_add(rect.height.get()) - .and_then(|y| y.try_into().ok()) - .ok_or_else(err)?, - )) + .map(|rect| { + ClipRect::new( + to_u16_saturating(rect.x), + to_u16_saturating(rect.y), + to_u16_saturating(rect.x.checked_add(rect.width.get()).unwrap_or(u32::MAX)), + to_u16_saturating(rect.y.checked_add(rect.height.get()).unwrap_or(u32::MAX)), + ) }) - .collect::, _>>()?; + .collect(); // Dirty the framebuffer with out damage rectangles. // @@ -378,17 +371,6 @@ impl BufferInterface for BufferImpl<'_> { Ok(()) } - - #[inline] - fn present(self) -> Result<(), SoftBufferError> { - let (width, height) = self.size; - self.present_with_damage(&[crate::Rect { - x: 0, - y: 0, - width, - height, - }]) - } } impl SharedBuffer { @@ -426,3 +408,8 @@ impl Buffers { self.buffers[0].size() } } + +/// Convert a `u32` to `u16`, and saturate if it overflows. +fn to_u16_saturating(val: u32) -> u16 { + val.try_into().unwrap_or(u16::MAX) +} diff --git a/src/backends/orbital.rs b/src/backends/orbital.rs index 89a718d..3de1f3c 100644 --- a/src/backends/orbital.rs +++ b/src/backends/orbital.rs @@ -176,7 +176,7 @@ impl BufferInterface for BufferImpl<'_> { } } - fn present(self) -> Result<(), SoftBufferError> { + fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> { match self.pixels { Pixels::Mapping(mapping) => { drop(mapping); @@ -190,10 +190,6 @@ impl BufferInterface for BufferImpl<'_> { Ok(()) } - - fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> { - self.present() - } } // Read the current width and size diff --git a/src/backends/wayland/mod.rs b/src/backends/wayland/mod.rs index 77b7b87..36fd67f 100644 --- a/src/backends/wayland/mod.rs +++ b/src/backends/wayland/mod.rs @@ -260,16 +260,15 @@ impl BufferInterface for BufferImpl<'_> { self.surface.damage(0, 0, i32::MAX, i32::MAX); } else { for rect in damage { + // Damage that falls outside the surface is ignored, so we don't need to clamp the + // rect manually. + // https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_surface + let x = to_i32_saturating(rect.x); + let y = to_i32_saturating(rect.y); + let width = to_i32_saturating(rect.width.get()); + let height = to_i32_saturating(rect.height.get()); + // Introduced in version 4, it is an error to use this request in version 3 or lower. - let (x, y, width, height) = (|| { - Some(( - i32::try_from(rect.x).ok()?, - i32::try_from(rect.y).ok()?, - i32::try_from(rect.width.get()).ok()?, - i32::try_from(rect.height.get()).ok()?, - )) - })() - .ok_or(SoftBufferError::DamageOutOfRange { rect: *rect })?; self.surface.damage_buffer(x, y, width, height); } } @@ -284,17 +283,6 @@ impl BufferInterface for BufferImpl<'_> { Ok(()) } - - fn present(self) -> Result<(), SoftBufferError> { - let (width, height) = (self.width, self.height); - self.present_with_damage(&[Rect { - x: 0, - y: 0, - // We know width/height will be non-negative and non-zero. - width: (width as u32).try_into().unwrap(), - height: (height as u32).try_into().unwrap(), - }]) - } } impl Dispatch for State { @@ -321,3 +309,8 @@ impl Dispatch for State { ) { } } + +/// Convert a `u32` to `i32`, and saturate if it overflows. +fn to_i32_saturating(val: u32) -> i32 { + val.try_into().unwrap_or(i32::MAX) +} diff --git a/src/backends/web.rs b/src/backends/web.rs index a829e5b..e9296b5 100644 --- a/src/backends/web.rs +++ b/src/backends/web.rs @@ -331,25 +331,13 @@ impl BufferInterface for BufferImpl<'_> { } /// Push the buffer to the canvas. - fn present(self) -> Result<(), SoftBufferError> { - let (width, height) = self - .size - .expect("Must set size of surface before calling `present()`"); - self.present_with_damage(&[Rect { - x: 0, - y: 0, - width, - height, - }]) - } - fn present_with_damage(self, damage: &[Rect]) -> Result<(), SoftBufferError> { - let (buffer_width, _buffer_height) = self + let (buffer_width, buffer_height) = self .size .expect("Must set size of surface before calling `present_with_damage()`"); let union_damage = if let Some(rect) = util::union_damage(damage) { - rect + util::clamp_rect(rect, buffer_width, buffer_height) } else { return Ok(()); }; @@ -370,8 +358,8 @@ impl BufferInterface for BufferImpl<'_> { .collect(); debug_assert_eq!( - bitmap.len() as u32, - union_damage.width.get() * union_damage.height.get() * 4 + bitmap.len(), + union_damage.width.get() as usize * union_damage.height.get() as usize * 4 ); #[cfg(target_feature = "atomics")] @@ -407,6 +395,8 @@ impl BufferInterface for BufferImpl<'_> { let image_data = result.unwrap(); for rect in damage { + let rect = util::clamp_rect(*rect, buffer_width, buffer_height); + // This can only throw an error if `data` is detached, which is impossible. self.canvas .put_image_data( diff --git a/src/backends/win32.rs b/src/backends/win32.rs index 40bd3bd..4aaba81 100644 --- a/src/backends/win32.rs +++ b/src/backends/win32.rs @@ -3,7 +3,7 @@ //! This module converts the input buffer into a bitmap and then stretches it to the window. use crate::backend_interface::*; -use crate::{Rect, SoftBufferError}; +use crate::{util, Rect, SoftBufferError}; use raw_window_handle::{HasDisplayHandle, HasWindowHandle, RawWindowHandle}; use std::io; @@ -271,29 +271,19 @@ impl BufferInterface for BufferImpl<'_> { } } - fn present(self) -> Result<(), SoftBufferError> { - let (width, height) = (self.buffer.width, self.buffer.height); - self.present_with_damage(&[Rect { - x: 0, - y: 0, - // We know width/height will be non-negative - width: width.try_into().unwrap(), - height: height.try_into().unwrap(), - }]) - } - fn present_with_damage(self, damage: &[Rect]) -> Result<(), SoftBufferError> { unsafe { - for rect in damage.iter().copied() { - let (x, y, width, height) = (|| { - Some(( - i32::try_from(rect.x).ok()?, - i32::try_from(rect.y).ok()?, - i32::try_from(rect.width.get()).ok()?, - i32::try_from(rect.height.get()).ok()?, - )) - })() - .ok_or(SoftBufferError::DamageOutOfRange { rect })?; + for rect in damage { + let rect = util::clamp_rect( + *rect, + self.buffer.width.try_into().unwrap(), + self.buffer.height.try_into().unwrap(), + ); + let x = to_i32_saturating(rect.x); + let y = to_i32_saturating(rect.y); + let width = to_i32_saturating(rect.width.get()); + let height = to_i32_saturating(rect.height.get()); + Gdi::BitBlt( self.dc.0, x, @@ -467,3 +457,8 @@ impl From for OnlyUsedFromOrigin { Self(t) } } + +/// Convert a `u32` to `i32`, and saturate if it overflows. +fn to_i32_saturating(val: u32) -> i32 { + val.try_into().unwrap_or(i32::MAX) +} diff --git a/src/backends/x11.rs b/src/backends/x11.rs index 1ea9cec..4be0399 100644 --- a/src/backends/x11.rs +++ b/src/backends/x11.rs @@ -469,18 +469,18 @@ impl BufferInterface for BufferImpl<'_> { damage .iter() .try_for_each(|rect| { - let (src_x, src_y, dst_x, dst_y, width, height) = (|| { - Some(( - u16::try_from(rect.x).ok()?, - u16::try_from(rect.y).ok()?, - i16::try_from(rect.x).ok()?, - i16::try_from(rect.y).ok()?, - u16::try_from(rect.width.get()).ok()?, - u16::try_from(rect.height.get()).ok()?, - )) - })( - ) - .ok_or(SoftBufferError::DamageOutOfRange { rect: *rect })?; + let rect = util::clamp_rect( + *rect, + surface_width.into(), + surface_height.into(), + ); + let src_x = to_u16_saturating(rect.x); + let src_y = to_u16_saturating(rect.y); + let dst_x = to_i16_saturating(rect.x); + let dst_y = to_i16_saturating(rect.y); + let width = to_u16_saturating(rect.width.get()); + let height = to_u16_saturating(rect.height.get()); + self.connection .shm_put_image( self.window, @@ -516,18 +516,6 @@ impl BufferInterface for BufferImpl<'_> { Ok(()) } - - fn present(self) -> Result<(), SoftBufferError> { - let (width, height) = self - .size - .expect("Must set size of surface before calling `present()`"); - self.present_with_damage(&[Rect { - x: 0, - y: 0, - width: width.into(), - height: height.into(), - }]) - } } impl Buffer { @@ -982,3 +970,13 @@ fn total_len(width: u16, height: u16) -> usize { .and_then(|len| len.checked_mul(4)) .unwrap_or_else(|| panic!("Dimensions are too large: ({} x {})", width, height)) } + +/// Convert a `u32` to `u16`, and saturate if it overflows. +fn to_u16_saturating(val: u32) -> u16 { + val.try_into().unwrap_or(u16::MAX) +} + +/// Convert a `u32` to `i16`, and saturate if it overflows. +fn to_i16_saturating(val: u32) -> i16 { + val.try_into().unwrap_or(i16::MAX) +} diff --git a/src/error.rs b/src/error.rs index b79be6b..9ba6225 100644 --- a/src/error.rs +++ b/src/error.rs @@ -86,12 +86,6 @@ pub enum SoftBufferError { height: NonZeroU32, }, - /// The provided damage rect is outside of the range supported by the backend. - DamageOutOfRange { - /// The damage rect that was out of range. - rect: crate::Rect, - }, - /// A platform-specific backend error occurred. /// /// The first field provides a human-readable description of the error. The second field @@ -133,11 +127,6 @@ impl fmt::Display for SoftBufferError { ), Self::PlatformError(msg, None) => write!(f, "Platform error: {msg:?}"), Self::PlatformError(msg, Some(err)) => write!(f, "Platform error: {msg:?}: {err}"), - Self::DamageOutOfRange { rect } => write!( - f, - "Damage rect {}x{} at ({}, {}) out of range for backend.", - rect.width, rect.height, rect.x, rect.y - ), Self::Unimplemented => write!(f, "This function is unimplemented on this platform."), } } diff --git a/src/lib.rs b/src/lib.rs index d54d2b9..2311248 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -276,12 +276,21 @@ impl Buffer<'_> { /// /// 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 present(self) -> Result<(), SoftBufferError> { - self.buffer_impl.present() + // Damage the entire buffer. + self.present_with_damage(&[Rect { + x: 0, + y: 0, + width: NonZeroU32::MAX, + height: NonZeroU32::MAX, + }]) } /// Presents buffer to the window, with damage regions. /// + /// Damage regions that fall outside the surface are ignored. + /// /// # Platform dependent behavior /// /// Supported on: diff --git a/src/util.rs b/src/util.rs index 5c29071..67a742d 100644 --- a/src/util.rs +++ b/src/util.rs @@ -43,6 +43,22 @@ pub(crate) fn union_damage(damage: &[Rect]) -> Option { }) } +/// Clamp the damage rectangle to be within the given bounds. +pub(crate) fn clamp_rect(rect: Rect, width: NonZeroU32, height: NonZeroU32) -> Rect { + // The positions of the edges of the rectangle. + let left = rect.x.min(width.get()); + let top = rect.y.min(height.get()); + let right = rect.x.saturating_add(rect.width.get()).min(width.get()); + let bottom = rect.y.saturating_add(rect.height.get()).min(height.get()); + + Rect { + x: left, + y: top, + width: NonZeroU32::new(right - left).expect("rect ended up being zero-sized"), + height: NonZeroU32::new(bottom - top).expect("rect ended up being zero-sized"), + } +} + /// A wrapper around a `Vec` of pixels that doesn't print the whole buffer on `Debug`. #[derive(PartialEq, Eq, Hash, Clone)] pub(crate) struct PixelBuffer(pub Vec);