document unsafe blocks

i never said well

Cargo.toml

@@ -1,14 +1,12 @@
 [package]
 name = "fimg"
-version = "0.2.1"
+version = "0.2.2"
 authors = ["bend-n <bend.n@outlook.com>"]
 license = "MIT"
 edition = "2021"
 description = "fast image operations"
 repository = "https://github.com/bend-n/fimg"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
 [dependencies]
 png = { version = "0.17", features = ["unstable"], optional = true }
 

src/affine.rs

@@ -18,11 +18,14 @@ impl<const CHANNELS: usize> Image<&mut [u8], CHANNELS> {
         for y in 0..self.height() / 2 {
             for x in 0..self.width() {
                 let y2 = self.height() - y - 1;
+                #[allow(clippy::multiple_unsafe_ops_per_block)]
                 // SAFETY: within bounds
-                let p2 = unsafe { self.pixel(x, y2) };
+                unsafe {
-                let p = unsafe { self.pixel(x, y) };
+                    let p2 = self.pixel(x, y2);
-                unsafe { self.set_pixel(x, y2, p) };
+                    let p = self.pixel(x, y);
-                unsafe { self.set_pixel(x, y, p2) };
+                    self.set_pixel(x, y2, p);
+                    self.set_pixel(x, y, p2);
+                }
             }
         }
     }
@@ -32,10 +35,14 @@ impl<const CHANNELS: usize> Image<&mut [u8], CHANNELS> {
         for y in 0..self.height() {
             for x in 0..self.width() / 2 {
                 let x2 = self.width() - x - 1;
-                let p2 = unsafe { self.pixel(x2, y) };
+                #[allow(clippy::multiple_unsafe_ops_per_block)]
-                let p = unsafe { self.pixel(x, y) };
+                // SAFETY: bounded
-                unsafe { self.set_pixel(x2, y, p) };
+                unsafe {
-                unsafe { self.set_pixel(x, y, p2) };
+                    let p2 = self.pixel(x2, y);
+                    let p = self.pixel(x, y);
+                    self.set_pixel(x2, y, p);
+                    self.set_pixel(x, y, p2);
+                }
             }
         }
     }
@@ -52,6 +59,7 @@ impl<const CHANNELS: usize> Image<Vec<u8>, CHANNELS> {
     ///
     /// UB if the image is not square
     pub unsafe fn rot_90(&mut self) {
+        // SAFETY: make sure to keep the safety docs linked
         unsafe { self.as_mut().rot_90() }
     }
 
@@ -60,6 +68,7 @@ impl<const CHANNELS: usize> Image<Vec<u8>, CHANNELS> {
     ///
     /// UB if the image is not square
     pub unsafe fn rot_270(&mut self) {
+        // SAFETY: idk this is just a convenience impl
         unsafe { self.as_mut().rot_270() }
     }
 }
@@ -69,11 +78,15 @@ impl<const CHANNELS: usize> Image<&mut [u8], CHANNELS> {
     pub fn rot_180(&mut self) {
         for y in 0..self.height() / 2 {
             for x in 0..self.width() {
+                // SAFETY: x, y come from the loop, must be ok
                 let p = unsafe { self.pixel(x, y) };
                 let x2 = self.width() - x - 1;
                 let y2 = self.height() - y - 1;
+                // SAFETY: values are good
                 let p2 = unsafe { self.pixel(x2, y2) };
+                // SAFETY: swapping would be cool, alas.
                 unsafe { self.set_pixel(x, y, p2) };
+                // SAFETY: although maybe i can cast it to a `[[u8; CHANNELS]]` and swap that 🤔
                 unsafe { self.set_pixel(x2, y2, p) };
             }
         }
@@ -82,11 +95,15 @@ impl<const CHANNELS: usize> Image<&mut [u8], CHANNELS> {
             let middle = self.height() / 2;
 
             for x in 0..self.width() / 2 {
-                let p = unsafe { self.pixel(x, middle) };
                 let x2 = self.width() - x - 1;
-                let p2 = unsafe { self.pixel(x2, middle) };
+                #[allow(clippy::multiple_unsafe_ops_per_block)]
-                unsafe { self.set_pixel(x, middle, p2) };
+                // SAFETY: its just doing the swappy
-                unsafe { self.set_pixel(x2, middle, p) };
+                unsafe {
+                    let p = self.pixel(x, middle);
+                    let p2 = self.pixel(x2, middle);
+                    self.set_pixel(x, middle, p2);
+                    self.set_pixel(x2, middle, p);
+                }
             }
         }
     }

src/lib.rs

@@ -12,6 +12,7 @@
 #![warn(
     clippy::missing_docs_in_private_items,
     clippy::multiple_unsafe_ops_per_block,
+    clippy::undocumented_unsafe_blocks,
     clippy::missing_const_for_fn,
     clippy::missing_safety_doc,
     unsafe_op_in_unsafe_fn,
@@ -66,7 +67,9 @@ impl Image<&[u8], 3> {
 #[inline]
 unsafe fn really_unsafe_index(x: u32, y: u32, w: u32) -> usize {
     // y * w + x
+    // SAFETY: FIXME make safe math
     let tmp = unsafe { (y as usize).unchecked_mul(w as usize) };
+    // SAFETY: FIXME make safe math
     unsafe { tmp.unchecked_add(x as usize) }
 }
 
@@ -137,9 +140,12 @@ impl<T: std::ops::Deref<Target = [u8]>, const CHANNELS: usize> Image<T, CHANNELS
     unsafe fn slice(&self, x: u32, y: u32) -> impl SliceIndex<[u8], Output = [u8]> {
         debug_assert!(x < self.width(), "x out of bounds");
         debug_assert!(y < self.height(), "y out of bounds");
+        // SAFETY: me when uncheck math: 😧
         let index = unsafe { really_unsafe_index(x, y, self.width()) };
+        // SAFETY: 🧐 is unsound? 😖
         let index = unsafe { index.unchecked_mul(CHANNELS) };
         debug_assert!(self.buffer.len() > index);
+        // SAFETY: as long as the buffer isnt wrong, this is 😄
         index..unsafe { index.unchecked_add(CHANNELS) }
     }
 
@@ -160,8 +166,11 @@ impl<T: std::ops::Deref<Target = [u8]>, const CHANNELS: usize> Image<T, CHANNELS
     /// - UB if buffer is too small
     #[inline]
     pub unsafe fn pixel(&self, x: u32, y: u32) -> [u8; CHANNELS] {
+        // SAFETY: we have been told x, y is in bounds
         let idx = unsafe { self.slice(x, y) };
+        // SAFETY: slice always returns a valid index
         let ptr = unsafe { self.buffer.get_unchecked(idx).as_ptr().cast() };
+        // SAFETY: slice always returns a length of `CHANNELS`, so we `cast()` it for convenience.
         unsafe { *ptr }
     }
 }
@@ -174,7 +183,9 @@ impl<T: std::ops::DerefMut<Target = [u8]>, const CHANNELS: usize> Image<T, CHANN
     /// - UB if buffer is too small
     #[inline]
     pub unsafe fn pixel_mut(&mut self, x: u32, y: u32) -> &mut [u8] {
+        // SAFETY: we have been told x, y is in bounds.
         let idx = unsafe { self.slice(x, y) };
+        // SAFETY: slice should always return a valid index
         unsafe { self.buffer.get_unchecked_mut(idx) }
     }
 

src/overlay.rs

@@ -26,11 +26,17 @@ pub trait Overlay<W> {
 /// SIMD accelerated rgba => rgb overlay.
 ///
 /// See [blit](https://en.wikipedia.org/wiki/Bit_blit)
+///
+/// # Safety
+/// UB if rgb.len() % 3 != 0
+/// UB if rgba.len() % 4 != 0
 unsafe fn blit(rgb: &mut [u8], rgba: &[u8]) {
     let mut srci = 0;
     let mut dsti = 0;
     while dsti + 16 <= rgb.len() {
+        // SAFETY: i think it ok
         let old: Simd<u8, 16> = Simd::from_slice(unsafe { rgb.get_unchecked(dsti..dsti + 16) });
+        // SAFETY: definetly ok
         let new: Simd<u8, 16> = Simd::from_slice(unsafe { rgba.get_unchecked(srci..srci + 16) });
 
         let threshold = new.simd_ge(Simd::splat(128)).to_int().cast::<u8>();
@@ -44,6 +50,7 @@ unsafe fn blit(rgb: &mut [u8], rgba: &[u8]) {
 
         let new_rgb = simd_swizzle!(new, [0, 1, 2, 4, 5, 6, 8, 9, 10, 12, 13, 14, 0, 0, 0, 0]);
         let blended = (new_rgb & mask) | (old & !mask);
+        // SAFETY: 4 * 4 == 16, so in bounds
         blended.copy_to_slice(unsafe { rgb.get_unchecked_mut(dsti..dsti + 16) });
 
         srci += 16;
@@ -51,10 +58,13 @@ unsafe fn blit(rgb: &mut [u8], rgba: &[u8]) {
     }
 
     while dsti + 3 <= rgb.len() {
+        // SAFETY: caller gurantees slice is big enough
         if unsafe { *rgba.get_unchecked(srci + 3) } >= 128 {
-            let src = unsafe { rgba.get_unchecked(srci..srci + 3) };
+            // SAFETY: slice is big enough!
-            let end = unsafe { rgb.get_unchecked_mut(dsti..dsti + 3) };
+            let src = unsafe { rgba.get_unchecked(srci..=srci + 2) };
-            unsafe { std::ptr::copy_nonoverlapping(src.as_ptr(), end.as_mut_ptr(), 3) };
+            // SAFETY: i hear it bound
+            let end = unsafe { rgb.get_unchecked_mut(dsti..=dsti + 2) };
+            end.copy_from_slice(src);
         }
 
         srci += 4;
@@ -69,16 +79,9 @@ impl Overlay<Image<&[u8], 4>> for Image<&mut [u8], 4> {
         debug_assert!(self.height() == with.height());
         for (i, other_pixels) in with.chunked().enumerate() {
             if other_pixels[3] >= 128 {
-                let idx_begin = unsafe { i.unchecked_mul(4) };
+                // SAFETY: outside are bounds of index from slice
-                let idx_end = unsafe { idx_begin.unchecked_add(4) };
+                let own_pixels = unsafe { self.buffer.get_unchecked_mut(i * 4..i * 4 + 4) };
-                let own_pixels = unsafe { self.buffer.get_unchecked_mut(idx_begin..idx_end) };
+                own_pixels.copy_from_slice(other_pixels);
-                unsafe {
-                    std::ptr::copy_nonoverlapping(
-                        other_pixels.as_ptr(),
-                        own_pixels.as_mut_ptr(),
-                        4,
-                    );
-                };
             }
         }
         self
@@ -88,9 +91,9 @@ impl Overlay<Image<&[u8], 4>> for Image<&mut [u8], 4> {
 impl OverlayAt<Image<&[u8], 4>> for Image<&mut [u8], 3> {
     #[inline]
     unsafe fn overlay_at(&mut self, with: &Image<&[u8], 4>, x: u32, y: u32) -> &mut Self {
-        // SAFETY: caller upholds these
+        // SAFETY: caller upholds this
         unsafe { assert_unchecked!(x + with.width() <= self.width()) };
-        unsafe { assert_unchecked!(y + with.height() <= self.height()) };
+        debug_assert!(y + with.height() <= self.height());
         for j in 0..with.height() {
             let i_x = j as usize * with.width() as usize * 4
                 ..(j as usize + 1) * with.width() as usize * 4;
@@ -99,8 +102,11 @@ impl OverlayAt<Image<&[u8], 4>> for Image<&mut [u8], 3> {
                     + x as usize
                     + with.width() as usize)
                     * 3;
+            // SAFETY: index is in bounds
             let rgb = unsafe { self.buffer.get_unchecked_mut(o_x) };
+            // SAFETY: bounds are outside index
             let rgba = unsafe { with.buffer.get_unchecked(i_x) };
+            // SAFETY: arguments are 🟢
             unsafe { blit(rgb, rgba) }
         }
         self
@@ -125,7 +131,9 @@ impl OverlayAt<Image<&[u8], 3>> for Image<&mut [u8], 3> {
                     let o_x = ((j + y as usize) * self.width() as usize + x as usize) * 3
                         ..((j + y as usize) * self.width() as usize + x as usize + ($n as usize))
                             * 3;
+                    // SAFETY: bounds are ✅
                     let a = unsafe { self.buffer.get_unchecked_mut(o_x) };
+                    // SAFETY: we are in ⬜!
                     let b = unsafe { with.buffer.get_unchecked(i_x) };
                     a.copy_from_slice(b);
                 }
@@ -151,11 +159,13 @@ impl Overlay<Image<&[u8], 4>> for Image<&mut [u8], 3> {
             .chunks_exact(with.width() as usize * 4)
             .enumerate()
         {
+            // SAFETY: all the bounds are good
             let rgb = unsafe {
                 self.buffer.get_unchecked_mut(
                     i * with.width() as usize * 3..(i + 1) * with.width() as usize * 3,
                 )
             };
+            // SAFETY: we have the rgb and rgba arguments right
             unsafe { blit(rgb, chunk) };
         }
         self
@@ -164,15 +174,30 @@ impl Overlay<Image<&[u8], 4>> for Image<&mut [u8], 3> {
 
 impl OverlayAt<Image<&[u8], 4>> for Image<&mut [u8], 4> {
     #[inline]
+    /// Overlay with => self at coordinates x, y, without blending
+    ///
+    /// # Safety
+    /// UB if x, y is out of bounds
+    /// UB if x + with.width() > u32::MAX
+    /// UB if y + with.height() > u32::MAX
     unsafe fn overlay_at(&mut self, with: &Image<&[u8], 4>, x: u32, y: u32) -> &mut Self {
         for j in 0..with.height() {
             for i in 0..with.width() {
+                // SAFETY: i, j is in bounds.
                 let index = unsafe { really_unsafe_index(i, j, with.width()) };
+                // SAFETY: using .pixel() results in horrible asm (+5k ns/iter)
                 let their_px = unsafe { with.buffer.get_unchecked(index * 4..index * 4 + 4) };
+                // SAFETY: must be sized right
                 if unsafe { *their_px.get_unchecked(3) } >= 128 {
+                    // SAFETY:
+                    // they said it cant go over.
+                    // i dont know why, but this has performance importance™
                     let x = unsafe { i.unchecked_add(x) };
+                    // SAFETY: caller gurantees this cannot overflow.
                     let y = unsafe { j.unchecked_add(y) };
+                    // SAFETY: compute the offset index.
                     let index = unsafe { really_unsafe_index(x, y, self.width()) };
+                    // SAFETY: if everything else goes well, this is fine
                     let our_px = unsafe { self.buffer.get_unchecked_mut(index * 4..index * 4 + 4) };
                     our_px.copy_from_slice(their_px);
                 }