Remove DamageOutOfRange error

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
 

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 {
                     $(

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

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

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<c_void>,
@@ -361,10 +361,6 @@ impl BufferInterface for BufferImpl<'_> {
         CATransaction::commit();
         Ok(())
     }
-
-    fn present_with_damage(self, _damage: &[Rect]) -> Result<(), SoftBufferError> {
-        self.present()
-    }
 }
 
 #[derive(Debug)]

src/backends/kms.rs

@@ -325,24 +325,17 @@
 
     #[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::<Result<Vec<_>, _>>()?;
+            .collect();
 
         // Dirty the framebuffer with out damage rectangles.
         //
@@ -378,17 +371,6 @@
 
         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 @@
         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)
+}

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

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<wl_registry::WlRegistry, GlobalListContents> for State {
@@ -321,3 +309,8 @@ impl Dispatch<wl_shm::WlShm, ()> 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)
+}

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(

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<T> From<T> for OnlyUsedFromOrigin<T> {
         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)
+}

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)
+}