diff --git a/CHANGELOG.md b/CHANGELOG.md index a0c9b006..0e230dbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - **Breaking:** Removed `DamageOutOfRange` error case. If the damage value is greater than the backend supports, it is instead clamped to an appropriate value. - **Breaking:** Removed `SurfaceExtWeb` and the associated `NoDisplayHandle` and `NoWindowHandle` helpers. Use `RawWindowHandle::WebCanvas` or `RawWindowHandle::WebOffscreenCanvas` instead. - Fixed `present_with_damage` with bounds out of range on Windows, Web and X11. +- **Breaking:** Changed `Rect.width` and `Rect.height` from `NonZeroU32` to `u32`. # 0.4.8 diff --git a/src/backends/kms.rs b/src/backends/kms.rs index e82770c2..c62be6bf 100644 --- a/src/backends/kms.rs +++ b/src/backends/kms.rs @@ -362,8 +362,8 @@ impl BufferInterface for BufferImpl<'_> { ClipRect::new( util::to_u16_saturating(rect.x), util::to_u16_saturating(rect.y), - util::to_u16_saturating(rect.x.saturating_add(rect.width.get())), - util::to_u16_saturating(rect.y.saturating_add(rect.height.get())), + util::to_u16_saturating(rect.x.saturating_add(rect.width)), + util::to_u16_saturating(rect.y.saturating_add(rect.height)), ) }) .collect(); diff --git a/src/backends/wayland/mod.rs b/src/backends/wayland/mod.rs index 4e1df783..4e024549 100644 --- a/src/backends/wayland/mod.rs +++ b/src/backends/wayland/mod.rs @@ -306,8 +306,8 @@ impl BufferInterface for BufferImpl<'_> { // https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_surface let x = util::to_i32_saturating(rect.x); let y = util::to_i32_saturating(rect.y); - let width = util::to_i32_saturating(rect.width.get()); - let height = util::to_i32_saturating(rect.height.get()); + let width = util::to_i32_saturating(rect.width); + let height = util::to_i32_saturating(rect.height); // Introduced in version 4, it is an error to use this request in version 3 or lower. self.surface.damage_buffer(x, y, width, height); diff --git a/src/backends/web.rs b/src/backends/web.rs index f747a557..c4e653b4 100644 --- a/src/backends/web.rs +++ b/src/backends/web.rs @@ -368,8 +368,8 @@ impl BufferInterface for BufferImpl<'_> { 0.0, rect.x.into(), rect.y.into(), - rect.width.get().into(), - rect.height.get().into(), + rect.width.into(), + rect.height.into(), ) .unwrap(); } diff --git a/src/backends/win32.rs b/src/backends/win32.rs index 566fbff9..8b732e56 100644 --- a/src/backends/win32.rs +++ b/src/backends/win32.rs @@ -329,8 +329,8 @@ impl BufferInterface for BufferImpl<'_> { ); let x = util::to_i32_saturating(rect.x); let y = util::to_i32_saturating(rect.y); - let width = util::to_i32_saturating(rect.width.get()); - let height = util::to_i32_saturating(rect.height.get()); + let width = util::to_i32_saturating(rect.width); + let height = util::to_i32_saturating(rect.height); // TODO: Draw with something else to make transparency work. Gdi::BitBlt( diff --git a/src/backends/x11.rs b/src/backends/x11.rs index c784128a..6c610be7 100644 --- a/src/backends/x11.rs +++ b/src/backends/x11.rs @@ -511,8 +511,8 @@ impl BufferInterface for BufferImpl<'_> { let src_y = util::to_u16_saturating(rect.y); let dst_x = util::to_i16_saturating(rect.x); let dst_y = util::to_i16_saturating(rect.y); - let width = util::to_u16_saturating(rect.width.get()); - let height = util::to_u16_saturating(rect.height.get()); + let width = util::to_u16_saturating(rect.width); + let height = util::to_u16_saturating(rect.height); self.connection .shm_put_image( diff --git a/src/lib.rs b/src/lib.rs index 5e24ede2..69c0c214 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,14 +62,14 @@ impl Context { /// A rectangular region of the buffer coordinate space. #[derive(Clone, Copy, Debug)] pub struct Rect { - /// x coordinate of top left corner + /// X coordinate of top left corner. pub x: u32, - /// y coordinate of top left corner + /// Y coordinate of top left corner. pub y: u32, - /// width - pub width: NonZeroU32, - /// height - pub height: NonZeroU32, + /// Width of the rectangle. + pub width: u32, + /// Height of the rectangle. + pub height: u32, } /// A surface for drawing to a window with software buffers. @@ -382,8 +382,8 @@ impl Buffer<'_> { self.present_with_damage(&[Rect { x: 0, y: 0, - width: NonZeroU32::MAX, - height: NonZeroU32::MAX, + width: u32::MAX, + height: u32::MAX, }]) } @@ -391,6 +391,10 @@ impl Buffer<'_> { /// /// Damage regions that fall outside the surface are ignored. /// + /// Zero-sized damage regions (rectangles with `width` or `height` equal to `0`) may be + /// completely ignored, or may end up increasing the "union" damage region to contain the + /// zero-sized rectangle too. This is platform-specific, and should not be relied upon. + /// /// # Platform dependent behavior /// /// Supported on: diff --git a/src/util.rs b/src/util.rs index 169f3b6b..23f4687a 100644 --- a/src/util.rs +++ b/src/util.rs @@ -8,55 +8,63 @@ use std::ops; use crate::{Pixel, Rect}; -/// Calculates the smallest `Rect` necessary to represent all damaged `Rect`s. -pub(crate) fn union_damage(damage: &[Rect]) -> Option { - struct Region { - left: u32, - top: u32, - bottom: u32, - right: u32, - } +/// The positions at the edge of a rectangle. +#[derive(Default)] +struct Region { + left: u32, + top: u32, + // Invariant: left <= right + right: u32, + // Invariant: top <= bottom + bottom: u32, +} - let region = damage - .iter() - .map(|rect| Region { +impl Region { + fn from_rect(rect: Rect) -> Self { + Self { left: rect.x, top: rect.y, - right: rect.x + rect.width.get(), - bottom: rect.y + rect.height.get(), - }) + right: rect.x.saturating_add(rect.width), + bottom: rect.y.saturating_add(rect.height), + } + } + + fn into_rect(self) -> Rect { + Rect { + x: self.left, + y: self.top, + width: self.right - self.left, + height: self.bottom - self.top, + } + } +} + +/// Calculates the smallest `Rect` necessary to represent all damaged `Rect`s. +pub(crate) fn union_damage(damage: &[Rect]) -> Rect { + damage + .iter() + .map(|rect| Region::from_rect(*rect)) .reduce(|mut prev, next| { prev.left = cmp::min(prev.left, next.left); prev.top = cmp::min(prev.top, next.top); prev.right = cmp::max(prev.right, next.right); prev.bottom = cmp::max(prev.bottom, next.bottom); prev - })?; - - Some(Rect { - x: region.left, - y: region.top, - width: NonZeroU32::new(region.right - region.left) - .expect("`right` must always be bigger then `left`"), - height: NonZeroU32::new(region.bottom - region.top) - .expect("`bottom` must always be bigger then `top`"), - }) + }) + .unwrap_or_default() + .into_rect() } /// 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"), - } + let mut region = Region::from_rect(rect); + + region.left = region.left.min(width.get()); + region.top = region.top.min(height.get()); + region.right = region.right.min(width.get()); + region.bottom = region.bottom.min(height.get()); + + region.into_rect() } /// A wrapper around a `Vec` of pixels that doesn't print the whole buffer on `Debug`. @@ -112,3 +120,103 @@ pub(crate) fn byte_stride(width: u32) -> u32 { let mask = row_alignment * 4 - 1; ((width * 32 + mask) & !mask) >> 3 } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn union() { + // Zero-sized. + let res = union_damage(&[]); + assert_eq!(res.x, 0); + assert_eq!(res.y, 0); + assert_eq!(res.width, 0); + assert_eq!(res.height, 0); + + let res = union_damage(&[ + Rect { + x: 100, + y: 20, + width: 30, + height: 40, + }, + Rect { + x: 10, + y: 200, + width: 30, + height: 40, + }, + ]); + assert_eq!(res.x, 10); + assert_eq!(res.y, 20); + assert_eq!(res.width, 120); + assert_eq!(res.height, 220); + } + + // This is not a guarantee either way, just a test of the current implementation. + #[test] + fn union_considers_zero_sized() { + let res = union_damage(&[ + Rect { + x: 100, + y: 20, + width: 0, + height: 40, + }, + Rect { + x: 10, + y: 200, + width: 30, + height: 0, + }, + ]); + assert_eq!(res.x, 10); + assert_eq!(res.y, 20); + assert_eq!(res.width, 90); + assert_eq!(res.height, 180); + } + + #[test] + fn clamp() { + let rect = Rect { + x: 10, + y: 20, + width: 30, + height: 40, + }; + + // Inside bounds + let res = clamp_rect( + rect, + NonZeroU32::new(50).unwrap(), + NonZeroU32::new(60).unwrap(), + ); + assert_eq!(res.x, 10); + assert_eq!(res.y, 20); + assert_eq!(res.width, 30); + assert_eq!(res.height, 40); + + // Size out of bounds + let res = clamp_rect( + rect, + NonZeroU32::new(33).unwrap(), + NonZeroU32::new(44).unwrap(), + ); + assert_eq!(res.x, 10); + assert_eq!(res.y, 20); + assert_eq!(res.width, 23); + assert_eq!(res.height, 24); + + // Fully beyond bounds + let res = clamp_rect( + rect, + NonZeroU32::new(1).unwrap(), + NonZeroU32::new(2).unwrap(), + ); + assert_eq!(res.x, 1); + assert_eq!(res.y, 2); + assert_eq!(res.width, 0); + assert_eq!(res.height, 0); + } +}