From 2978612c10bba1338f12fd5cf59ab0bdce7bb70c Mon Sep 17 00:00:00 2001
From: Andreas Molzer <andreas.molzer@gmx.de>
Date: Thu, 10 Oct 2024 17:31:20 +0200
Subject: [PATCH] Optimize sampling for empty images

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.
---
 src/imageops/sample.rs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/imageops/sample.rs b/src/imageops/sample.rs
index d0f18999e8..9dfd7549d8 100644
--- a/src/imageops/sample.rs
+++ b/src/imageops/sample.rs
@@ -463,6 +463,8 @@ 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>,
@@ -470,6 +472,14 @@ where
     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();
 
@@ -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());
@@ -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
@@ -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));
+    }
 }