Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize sampling for empty images #2342

Merged
merged 2 commits into from
Oct 12, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions src/imageops/sample.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,13 +463,23 @@ pub fn interpolate_bilinear<P: Pixel>(
// ```filter``` is the filter to use for sampling.
// The return value is not necessarily Rgba, the underlying order of channels in ```image``` is
// preserved.
//
// Note: if an empty image is passed in, panics unless the image is truly empty.
fn vertical_sample<I, P, S>(image: &I, new_height: u32, filter: &mut Filter) -> Rgba32FImage
where
I: GenericImageView<Pixel = P>,
P: Pixel<Subpixel = S> + 'static,
S: Primitive + 'static,
{
let (width, height) = image.dimensions();

// This is protection against a regression in memory usage such as #2340. Since the strategy to
// deal with it depends on the caller it is a precondition of this function.
assert!(
height != 0 || width == 0,
"Unexpected prior allocation size. This case should have been handled by the caller"
);
HeroicKatora marked this conversation as resolved.
Show resolved Hide resolved

let mut out = ImageBuffer::new(width, new_height);
let mut ws = Vec::new();

Expand Down Expand Up @@ -916,6 +926,16 @@ where
I::Pixel: 'static,
<I::Pixel as Pixel>::Subpixel: 'static,
{
// Check if there is nothing to sample from.
let is_empty = {
let (width, height) = image.dimensions();
width == 0 || height == 0
};

if is_empty {
return ImageBuffer::new(nwidth, nheight);
}

// check if the new dimensions are the same as the old. if they are, make a copy instead of resampling
if (nwidth, nheight) == image.dimensions() {
let mut tmp = ImageBuffer::new(image.width(), image.height());
Expand Down Expand Up @@ -968,6 +988,11 @@ where
};

let (width, height) = image.dimensions();
let is_empty = width == 0 || height == 0;

if is_empty {
return ImageBuffer::new(width, height);
}

// Keep width and height the same for horizontal and
// vertical sampling.
Expand Down Expand Up @@ -1257,4 +1282,16 @@ mod tests {
let result = resize(&image, 22, 22, FilterType::Lanczos3);
assert!(result.into_raw().into_iter().any(|c| c != 0));
}

#[test]
fn issue_2340() {
let empty = crate::GrayImage::from_raw(1 << 31, 0, vec![]).unwrap();
HeroicKatora marked this conversation as resolved.
Show resolved Hide resolved
// Really we're checking that no overflow / outsized allocation happens here.
let result = resize(&empty, 1, 1, FilterType::Lanczos3);
assert!(result.into_raw().into_iter().all(|c| c == 0));
// With the previous strategy before the regression this would allocate 1TB of memory for a
// temporary during the sampling evaluation.
let result = resize(&empty, 256, 256, FilterType::Lanczos3);
assert!(result.into_raw().into_iter().all(|c| c == 0));
}
}
Loading