From 1cbe8d6535e6b104a3363a8d699f0efa896a0e00 Mon Sep 17 00:00:00 2001 From: cospectrum Date: Mon, 27 May 2024 18:16:54 +0300 Subject: [PATCH] Revert use of `filter_pixel` in `filter` functions (#654) --- src/filter/mod.rs | 176 +++++++++++++++++++++++----------------------- src/gradients.rs | 19 +++-- src/kernel.rs | 34 +++++++-- 3 files changed, 128 insertions(+), 101 deletions(-) diff --git a/src/filter/mod.rs b/src/filter/mod.rs index 066b5d44..12ecf465 100644 --- a/src/filter/mod.rs +++ b/src/filter/mod.rs @@ -9,7 +9,6 @@ mod sharpen; pub use self::sharpen::*; use image::{GenericImage, GenericImageView, GrayImage, Luma, Pixel, Primitive}; -use itertools::Itertools; use crate::definitions::{Clamp, Image}; use crate::integral_image::{column_running_sum, row_running_sum}; @@ -84,109 +83,107 @@ pub fn box_filter(image: &GrayImage, x_radius: u32, y_radius: u32) -> Image(x: u32, y: u32, kernel: Kernel, f: F, image: &Image

) -> Q -where - P: Pixel, - Q: Pixel, - F: Fn(K) -> Q::Subpixel, - K: num::Num + Copy + From, -{ - let (width, height) = (image.width() as i64, image.height() as i64); - let (k_width, k_height) = (kernel.width as i64, kernel.height as i64); - let (x, y) = (i64::from(x), i64::from(y)); - - let weighted_pixels = (0..kernel.height as i64) - .cartesian_product(0..kernel.width as i64) - .map(|(k_y, k_x)| { - let kernel_weight = *kernel.at(k_x as u32, k_y as u32); - - let window_y = (y + k_y - k_height / 2).clamp(0, height - 1); - let window_x = (x + k_x - k_width / 2).clamp(0, width - 1); - - debug_assert!(image.in_bounds(window_x as u32, window_y as u32)); - - // Safety: we clamped `window_x` and `window_y` to be in bounds. - let window_pixel = unsafe { image.unsafe_get_pixel(window_x as u32, window_y as u32) }; - - //optimisation: remove allocation when `Pixel::map` allows mapping to a different - //type - #[allow(clippy::unnecessary_to_owned)] - let weighted_pixel = window_pixel - .channels() - .to_vec() - .into_iter() - .map(move |c| kernel_weight * K::from(c)); - - weighted_pixel - }); - - let final_channel_sum = weighted_pixels.fold( - //optimisation: do this without allocation when `Pixel` gains a method of constant initialization - vec![K::zero(); Q::CHANNEL_COUNT as usize], - |mut accumulator, weighted_pixel| { - for (i, weighted_subpixel) in weighted_pixel.enumerate() { - accumulator[i] = accumulator[i] + weighted_subpixel; - } - - accumulator - }, - ); - - *Q::from_slice(&final_channel_sum.into_iter().map(f).collect_vec()) -} - /// Returns 2d correlation of an image. Intermediate calculations are performed /// at type K, and the results converted to pixel Q via f. Pads by continuity. -pub fn filter(image: &Image

, kernel: Kernel, f: F) -> Image +/// +/// # Panics +/// If `P::CHANNEL_COUNT != Q::CHANNEL_COUNT` +pub fn filter(image: &Image

, kernel: Kernel, mut f: F) -> Image where P: Pixel, +

::Subpixel: Into, Q: Pixel, - F: Fn(K) -> Q::Subpixel, - K: num::Num + Copy + From, + F: FnMut(K) -> Q::Subpixel, + K: Copy + Num, { - //TODO refactor this to use the `crate::maps` functions once that lands + assert_eq!(P::CHANNEL_COUNT, Q::CHANNEL_COUNT); + let num_channels = P::CHANNEL_COUNT as usize; + let zero = K::zero(); let (width, height) = image.dimensions(); - let mut out = Image::::new(width, height); + let mut acc = vec![zero; num_channels]; + let (k_width, k_height) = (kernel.width as i64, kernel.height as i64); + let (width, height) = (width as i64, height as i64); for y in 0..height { for x in 0..width { - out.put_pixel(x, y, filter_pixel(x, y, kernel, &f, image)); + for k_y in 0..k_height { + let y_p = min(height - 1, max(0, y + k_y - k_height / 2)) as u32; + for k_x in 0..k_width { + let x_p = min(width - 1, max(0, x + k_x - k_width / 2)) as u32; + + debug_assert!(image.in_bounds(x_p, y_p)); + accumulate( + &mut acc, + unsafe { &image.unsafe_get_pixel(x_p, y_p) }, + unsafe { kernel.get_unchecked(k_x as u32, k_y as u32) }, + ); + } + } + let out_channels = out.get_pixel_mut(x as u32, y as u32).channels_mut(); + for (a, c) in acc.iter_mut().zip(out_channels) { + *c = f(*a); + *a = zero; + } } } - out } + #[cfg(feature = "rayon")] #[doc = generate_parallel_doc_comment!("filter")] pub fn filter_parallel(image: &Image

, kernel: Kernel, f: F) -> Image where P: Pixel + Sync, - Q: Pixel + Send + Sync, - P::Subpixel: Sync, - Q::Subpixel: Send + Sync, - F: Fn(K) -> Q::Subpixel + Send + Sync, - K: Num + Copy + From + Sync, + P::Subpixel: Into + Sync, + Q: Pixel + Send, + F: Sync + Fn(K) -> Q::Subpixel, + K: Copy + Num + Sync, { - //TODO refactor this to use the `crate::maps` functions once that lands + use num::Zero; + use rayon::prelude::*; - use rayon::iter::IndexedParallelIterator; - use rayon::iter::ParallelIterator; + assert_eq!(P::CHANNEL_COUNT, Q::CHANNEL_COUNT); + let num_channels = P::CHANNEL_COUNT as usize; - let (width, height) = image.dimensions(); + let (k_width, k_height) = (kernel.width as i64, kernel.height as i64); + let (width, height) = (image.width() as i64, image.height() as i64); - let mut out: Image = Image::new(width, height); + let out_rows: Vec> = (0..height) + .into_par_iter() + .map(|y| { + let mut out_row = Vec::with_capacity(image.width() as usize); + let mut out_pixel = vec![Q::Subpixel::zero(); num_channels]; + let mut acc = vec![K::zero(); num_channels]; - image - .par_enumerate_pixels() - .zip_eq(out.par_pixels_mut()) - .for_each(move |((x, y, _), output_pixel)| { - *output_pixel = filter_pixel(x, y, kernel, &f, image); - }); + for x in 0..width { + for k_y in 0..k_height { + let y_p = min(height - 1, max(0, y + k_y - k_height / 2)) as u32; + for k_x in 0..k_width { + let x_p = min(width - 1, max(0, x + k_x - k_width / 2)) as u32; + + debug_assert!(image.in_bounds(x_p, y_p)); + accumulate( + &mut acc, + unsafe { &image.unsafe_get_pixel(x_p, y_p) }, + unsafe { kernel.get_unchecked(k_x as u32, k_y as u32) }, + ); + } + } + for (a, c) in acc.iter_mut().zip(out_pixel.iter_mut()) { + *c = f(*a); + *a = K::zero(); + } + out_row.push(*Q::from_slice(&out_pixel)); + } + out_row + }) + .collect(); - out + Image::from_fn(image.width(), image.height(), |x, y| { + out_rows[y as usize][x as usize] + }) } #[inline] @@ -268,13 +265,14 @@ where /// the crate `rayon` feature is enabled. pub fn filter_clamped(image: &Image

, kernel: Kernel) -> Image> where - P::Subpixel: Into, - S: Clamp + Primitive, P: WithChannel, + P::Subpixel: Into, K: Num + Copy + From<

::Subpixel>, + S: Clamp + Primitive, { filter(image, kernel, S::clamp) } + #[cfg(feature = "rayon")] #[doc = generate_parallel_doc_comment!("filter_clamped")] pub fn filter_clamped_parallel( @@ -282,12 +280,11 @@ pub fn filter_clamped_parallel( kernel: Kernel, ) -> Image> where - P: Sync, - P::Subpixel: Send + Sync, -

>::Pixel: Send + Sync, - S: Clamp + Primitive + Send + Sync, - P: WithChannel, - K: Num + Copy + Send + Sync + From, + P: Sync + WithChannel, + P::Subpixel: Into + Sync, +

>::Pixel: Send, + K: Num + Copy + From<

::Subpixel> + Sync, + S: Clamp + Primitive + Send, { filter_parallel(image, kernel, S::clamp) } @@ -474,12 +471,12 @@ where fn accumulate(acc: &mut [K], pixel: &P, weight: K) where P: Pixel, -

::Subpixel: Into, + P::Subpixel: Into, K: Num + Copy, { - for i in 0..(P::CHANNEL_COUNT as usize) { - acc[i] = acc[i] + pixel.channels()[i].into() * weight; - } + acc.iter_mut().zip(pixel.channels()).for_each(|(a, &c)| { + *a = *a + c.into() * weight; + }); } fn clamp_and_reset(acc: &mut [K], out_channels: &mut [P::Subpixel], zero: K) @@ -507,6 +504,7 @@ where pub fn laplacian_filter(image: &GrayImage) -> Image> { filter_clamped(image, kernel::FOUR_LAPLACIAN_3X3) } + #[must_use = "the function does not modify the original image"] #[cfg(feature = "rayon")] #[doc = generate_parallel_doc_comment!("laplacian_filter")] diff --git a/src/gradients.rs b/src/gradients.rs index 766a34a0..ee982b7f 100644 --- a/src/gradients.rs +++ b/src/gradients.rs @@ -1,7 +1,7 @@ //! Functions for computing gradients of image intensities. -use crate::definitions::{Clamp, HasBlack, Image}; -use crate::filter::{filter, filter_clamped}; +use crate::definitions::{HasBlack, Image}; +use crate::filter::filter_clamped; use crate::kernel::{self, Kernel}; use crate::map::{ChannelMap, WithChannel}; use image::{GenericImage, GenericImageView, GrayImage, Luma, Pixel}; @@ -95,8 +95,8 @@ pub fn gradients_greyscale( /// # } pub fn gradients( image: &Image

, - kernel1: Kernel, - kernel2: Kernel, + horizontal_kernel: Kernel, + vertical_kernel: Kernel, f: F, ) -> Image where @@ -105,8 +105,8 @@ where ChannelMap: HasBlack, F: Fn(ChannelMap) -> Q, { - let pass1: Image> = filter(image, kernel1, >::clamp); - let pass2: Image> = filter(image, kernel2, >::clamp); + let horizontal = filter_clamped::<_, _, i16>(image, horizontal_kernel); + let vertical = filter_clamped::<_, _, i16>(image, vertical_kernel); let (width, height) = image.dimensions(); let mut out = Image::::new(width, height); @@ -122,7 +122,12 @@ where // x and y are in bounds for image by construction, // vertical and horizontal are the result of calling filter_clamped on image, // and filter_clamped returns an image of the same size as its input - let (h, v) = unsafe { (pass1.unsafe_get_pixel(x, y), pass2.unsafe_get_pixel(x, y)) }; + let (h, v) = unsafe { + ( + horizontal.unsafe_get_pixel(x, y), + vertical.unsafe_get_pixel(x, y), + ) + }; let mut p = ChannelMap::::black(); for (h, v, p) in multizip((h.channels(), v.channels(), p.channels_mut())) { diff --git a/src/kernel.rs b/src/kernel.rs index aa940054..70a2c389 100644 --- a/src/kernel.rs +++ b/src/kernel.rs @@ -2,13 +2,19 @@ /// An borrowed 2D kernel, used to filter images via convolution. #[derive(Debug, Copy, Clone)] -pub struct Kernel<'a, K> { +pub struct Kernel<'a, K> +where + K: Copy, +{ pub(crate) data: &'a [K], pub(crate) width: u32, pub(crate) height: u32, } -impl<'a, K> Kernel<'a, K> { +impl<'a, K> Kernel<'a, K> +where + K: Copy, +{ /// Construct a kernel from a slice and its dimensions. The input slice is /// in row-major order. /// @@ -29,10 +35,28 @@ impl<'a, K> Kernel<'a, K> { /// /// # Panics /// - /// If the `x` or `y` is outside of the width or height of the kernel. + /// If the `y * kernel.width + x` is outside of the kernel data. #[inline] - pub fn at(&self, x: u32, y: u32) -> &K { - &self.data[(y * self.width + x) as usize] + pub fn get(&self, x: u32, y: u32) -> K { + debug_assert!(x < self.width); + debug_assert!(y < self.height); + let at = usize::try_from(y * self.width + x).unwrap(); + self.data[at] + } + + /// Get the value in the kernel at the given `x` and `y` position, without + /// doing bounds checking. + /// + /// # Safety + /// The caller must ensure that `y * self.width + x` is in bounds of the + /// kernel data. + #[inline] + pub unsafe fn get_unchecked(&self, x: u32, y: u32) -> K { + debug_assert!(x < self.width); + debug_assert!(y < self.height); + let at = usize::try_from(y * self.width + x).unwrap(); + debug_assert!(at < self.data.len()); + *self.data.get_unchecked(at) } }