Implement Box blur fast filter that could approximate gaussian filter #223
Conversation
|
You mean in crates/kornia-imgproc/benches/bench_filters.rs ? Sure ok |
| @@ -61,6 +61,36 @@ pub fn sobel_kernel_1d(kernel_size: usize) -> (Vec<f32>, Vec<f32>) { | |||
| (kernel_x, kernel_y) | |||
| } | |||
|
|
|||
|
|
|||
| /// Create list of optimized box blur kernels based on gaussian sigma | |||
There was a problem hiding this comment.
| /// Create list of optimized box blur kernels based on gaussian sigma | |
| /// Create list of optimized box blur kernels based on gaussian sigma | |
| /// |
| /// # Arguments | ||
| /// | ||
| /// * `sigma` = The sigma of the gaussian kernel | ||
| /// * `kernels` = The number of times the box blur kernels would be applied, ideally from 3-5 |
There was a problem hiding this comment.
| /// * `kernels` = The number of times the box blur kernels would be applied, ideally from 3-5 | |
| /// * `kernels` = The number of times the box blur kernels would be applied, ideally from 3-5 | |
| /// |
| @@ -92,4 +122,11 @@ mod tests { | |||
| assert_eq!(k, expected[i]); | |||
| } | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
Maybe add also some test that it’s not only ones ?
| /// * `src` - The source image with shape (H, W, C). | ||
| /// * `dst` - The destination image with shape (H, W, C). | ||
| /// * `kernel_size` - The size of the kernel (kernel_x, kernel_y). | ||
| /// * `sigma` - The sigma of the gaussian kernel. |
There was a problem hiding this comment.
Specify that it should be x-y ordered
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
I would some simple numbers test too similar to the other functions to verify that’s doing the right thing
| @@ -88,6 +88,72 @@ pub fn separable_filter<const C: usize>( | |||
| Ok(()) | |||
| } | |||
|
|
|||
|
|
|||
| /// Apply a fast filter horizontally, take advantage of property where all | |||
There was a problem hiding this comment.
Split header docs. Usually there’s a single line explaining in short the purpose of the function followed by end of line then you can add some clarification, formulation of anything needed
| /// * `src` - The source image with shape (H, W, C). | ||
| /// * `dst_transposed` - The destination image with shape (W, H, C). | ||
| /// * `half_kernel_x_size` - Half of the kernel at weight 1. The total size would be 2*this+1 | ||
| pub fn fast_horizontal_filter<const C: usize>( |
There was a problem hiding this comment.
| pub fn fast_horizontal_filter<const C: usize>( | |
| fn fast_horizontal_filter<const C: usize>( |
I wouldn’t make public to users, this is more a utility function
| /// * `half_kernel_x_size` - Half of the kernel at weight 1. The total size would be 2*this+1 | ||
| pub fn fast_horizontal_filter<const C: usize>( | ||
| src: &Image<f32, C>, | ||
| dst_transposed: &mut Image<f32, C>, |
| row_acc[ch] += src_data[kernel_pix_offset]; | ||
| } | ||
| leftmost_pixel[ch] = *source_pixel; | ||
| rightmost_pixel[ch] = src_data[pix_offset+((src.cols()-1)*C)]; |
There was a problem hiding this comment.
(src.cols()-1)*C) could be computed outside before the loops
|
|
||
| if c == 0 { | ||
| row_acc[ch] = *source_pixel * (half_kernel_x_size+1) as f32; | ||
| let mut kernel_pix_offset = pix_offset; |
There was a problem hiding this comment.
Wondering wether this offset could be precomputed beforehand as you are computing several times in the top level function
|
@johnnv1 any idea why python tests are failing (I believe it’s unrelated to this PR). Shouldn’t we be using the new just commands in https://github.com/kornia/kornia-rs/blob/main/.github/workflows/python_test.yml#L40 |
yeah, seems unrelated, but should be working |
solve #168. The algorithm was derived from this blog post