diff --git a/Cargo.toml b/Cargo.toml index c5e2764..e7e869b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,14 +1,12 @@ [package] name = "fimg" -version = "0.2.1" +version = "0.2.2" authors = ["bend-n "] 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 } diff --git a/src/affine.rs b/src/affine.rs index b125389..362a746 100644 --- a/src/affine.rs +++ b/src/affine.rs @@ -18,11 +18,14 @@ impl 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) }; - let p = unsafe { self.pixel(x, y) }; - unsafe { self.set_pixel(x, y2, p) }; - unsafe { self.set_pixel(x, y, p2) }; + unsafe { + let p2 = self.pixel(x, y2); + let p = self.pixel(x, y); + self.set_pixel(x, y2, p); + self.set_pixel(x, y, p2); + } } } } @@ -32,10 +35,14 @@ impl 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) }; - let p = unsafe { self.pixel(x, y) }; - unsafe { self.set_pixel(x2, y, p) }; - unsafe { self.set_pixel(x, y, p2) }; + #[allow(clippy::multiple_unsafe_ops_per_block)] + // SAFETY: bounded + unsafe { + 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 Image, 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 Image, 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 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 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) }; - unsafe { self.set_pixel(x, middle, p2) }; - unsafe { self.set_pixel(x2, middle, p) }; + #[allow(clippy::multiple_unsafe_ops_per_block)] + // SAFETY: its just doing the swappy + 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); + } } } } diff --git a/src/lib.rs b/src/lib.rs index c45bde2..a760a88 100644 --- a/src/lib.rs +++ b/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, const CHANNELS: usize> Image 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, const CHANNELS: usize> Image [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, const CHANNELS: usize> Image &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) } } diff --git a/src/overlay.rs b/src/overlay.rs index 1a059e1..9624cb9 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -26,11 +26,17 @@ pub trait Overlay { /// 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 = Simd::from_slice(unsafe { rgb.get_unchecked(dsti..dsti + 16) }); + // SAFETY: definetly ok let new: Simd = Simd::from_slice(unsafe { rgba.get_unchecked(srci..srci + 16) }); let threshold = new.simd_ge(Simd::splat(128)).to_int().cast::(); @@ -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) }; - let end = unsafe { rgb.get_unchecked_mut(dsti..dsti + 3) }; - unsafe { std::ptr::copy_nonoverlapping(src.as_ptr(), end.as_mut_ptr(), 3) }; + // SAFETY: slice is big enough! + let src = unsafe { rgba.get_unchecked(srci..=srci + 2) }; + // 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> 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) }; - let idx_end = unsafe { idx_begin.unchecked_add(4) }; - let own_pixels = unsafe { self.buffer.get_unchecked_mut(idx_begin..idx_end) }; - unsafe { - std::ptr::copy_nonoverlapping( - other_pixels.as_ptr(), - own_pixels.as_mut_ptr(), - 4, - ); - }; + // SAFETY: outside are bounds of index from slice + let own_pixels = unsafe { self.buffer.get_unchecked_mut(i * 4..i * 4 + 4) }; + own_pixels.copy_from_slice(other_pixels); } } self @@ -88,9 +91,9 @@ impl Overlay> for Image<&mut [u8], 4> { impl OverlayAt> 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> 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> 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> 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> for Image<&mut [u8], 3> { impl OverlayAt> 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); }