Remove NonZero from Rect.width and Rect.height

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
 

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();

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);

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();
         }

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(

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(

src/lib.rs

@@ -62,14 +62,14 @@ impl<D: HasDisplayHandle> Context<D> {
 /// 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,15 +382,19 @@ impl Buffer<'_> {
         self.present_with_damage(&[Rect {
             x: 0,
             y: 0,
-            width: NonZeroU32::MAX,
-            height: NonZeroU32::MAX,
+            width: u32::MAX,
+            height: u32::MAX,
         }])
     }
 
     /// Presents buffer to the window, with damage regions.
     ///
     /// 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:

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<Rect> {
-    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);
+    }
+}