Skip to content

Commit

Permalink
Revert use of filter_pixel in filter functions (#654)
Browse files Browse the repository at this point in the history
  • Loading branch information
cospectrum authored May 27, 2024
1 parent 2c517d4 commit 1cbe8d6
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 101 deletions.
176 changes: 87 additions & 89 deletions src/filter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -84,109 +83,107 @@ pub fn box_filter(image: &GrayImage, x_radius: u32, y_radius: u32) -> Image<Luma
out
}

/// Calculates the new pixel value for a particular pixel and kernel.
fn filter_pixel<P, K, F, Q>(x: u32, y: u32, kernel: Kernel<K>, f: F, image: &Image<P>) -> Q
where
P: Pixel,
Q: Pixel,
F: Fn(K) -> Q::Subpixel,
K: num::Num + Copy + From<P::Subpixel>,
{
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<P, K, F, Q>(image: &Image<P>, kernel: Kernel<K>, f: F) -> Image<Q>
///
/// # Panics
/// If `P::CHANNEL_COUNT != Q::CHANNEL_COUNT`
pub fn filter<P, K, F, Q>(image: &Image<P>, kernel: Kernel<K>, mut f: F) -> Image<Q>
where
P: Pixel,
<P as Pixel>::Subpixel: Into<K>,
Q: Pixel,
F: Fn(K) -> Q::Subpixel,
K: num::Num + Copy + From<P::Subpixel>,
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::<Q>::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<P, K, F, Q>(image: &Image<P>, kernel: Kernel<K>, f: F) -> Image<Q>
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<P::Subpixel> + Sync,
P::Subpixel: Into<K> + 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<Q> = Image::new(width, height);
let out_rows: Vec<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]
Expand Down Expand Up @@ -268,26 +265,26 @@ where
/// the crate `rayon` feature is enabled.
pub fn filter_clamped<P, K, S>(image: &Image<P>, kernel: Kernel<K>) -> Image<ChannelMap<P, S>>
where
P::Subpixel: Into<K>,
S: Clamp<K> + Primitive,
P: WithChannel<S>,
P::Subpixel: Into<K>,
K: Num + Copy + From<<P as image::Pixel>::Subpixel>,
S: Clamp<K> + Primitive,
{
filter(image, kernel, S::clamp)
}

#[cfg(feature = "rayon")]
#[doc = generate_parallel_doc_comment!("filter_clamped")]
pub fn filter_clamped_parallel<P, K, S>(
image: &Image<P>,
kernel: Kernel<K>,
) -> Image<ChannelMap<P, S>>
where
P: Sync,
P::Subpixel: Send + Sync,
<P as WithChannel<S>>::Pixel: Send + Sync,
S: Clamp<K> + Primitive + Send + Sync,
P: WithChannel<S>,
K: Num + Copy + Send + Sync + From<P::Subpixel>,
P: Sync + WithChannel<S>,
P::Subpixel: Into<K> + Sync,
<P as WithChannel<S>>::Pixel: Send,
K: Num + Copy + From<<P as image::Pixel>::Subpixel> + Sync,
S: Clamp<K> + Primitive + Send,
{
filter_parallel(image, kernel, S::clamp)
}
Expand Down Expand Up @@ -474,12 +471,12 @@ where
fn accumulate<P, K>(acc: &mut [K], pixel: &P, weight: K)
where
P: Pixel,
<P as Pixel>::Subpixel: Into<K>,
P::Subpixel: Into<K>,
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<P, K>(acc: &mut [K], out_channels: &mut [P::Subpixel], zero: K)
Expand Down Expand Up @@ -507,6 +504,7 @@ where
pub fn laplacian_filter(image: &GrayImage) -> Image<Luma<i16>> {
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")]
Expand Down
19 changes: 12 additions & 7 deletions src/gradients.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -95,8 +95,8 @@ pub fn gradients_greyscale<P, F, Q>(
/// # }
pub fn gradients<P, F, Q>(
image: &Image<P>,
kernel1: Kernel<i32>,
kernel2: Kernel<i32>,
horizontal_kernel: Kernel<i32>,
vertical_kernel: Kernel<i32>,
f: F,
) -> Image<Q>
where
Expand All @@ -105,8 +105,8 @@ where
ChannelMap<P, u16>: HasBlack,
F: Fn(ChannelMap<P, u16>) -> Q,
{
let pass1: Image<ChannelMap<P, i16>> = filter(image, kernel1, <i16 as Clamp<i32>>::clamp);
let pass2: Image<ChannelMap<P, i16>> = filter(image, kernel2, <i16 as Clamp<i32>>::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::<Q>::new(width, height);
Expand All @@ -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::<P, u16>::black();

for (h, v, p) in multizip((h.channels(), v.channels(), p.channels_mut())) {
Expand Down
34 changes: 29 additions & 5 deletions src/kernel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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)
}
}

Expand Down

0 comments on commit 1cbe8d6

Please sign in to comment.