Skip to content

Commit

Permalink
Optimize sampling for empty images
Browse files Browse the repository at this point in the history
This is a tip of an iceberg of better sampling, but the most critical
case. Up-sampling in general may in the implementation allocate a larger
temporary buffer than its input. Of course this makes little semantic
sense here: after all, the actual information can not increase by this.

If one dimension increases while the other decreases the unfortunate
consequence is that callers may somewhat reasonably expect a small
buffer but internally will get a very large buffer. The approach of
swapping sampling orders accordingly (first down, then up) might address
the memory issue but is lossy.

So instead let's fix the most pressing issue: if no information was
present in the input, nothing can be lost and we can pretend to perform
everything by a very small intermediate image.
  • Loading branch information
HeroicKatora committed Oct 11, 2024
1 parent 9489d71 commit 2978612
Showing 1 changed file with 34 additions and 0 deletions.
34 changes: 34 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"
);

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

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

let (width, height) = image.dimensions();

if width == 0 || height == 0 {
return ImageBuffer::new(0, 0);
}

// Keep width and height the same for horizontal and
// vertical sampling.
// Note: tmp is not necessarily actually Rgba
Expand Down Expand Up @@ -1257,4 +1279,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();
// 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));
}
}

0 comments on commit 2978612

Please sign in to comment.