Reduction performance tweaks (#676)

Two key changes here: - Removed some defunct reduction benches and added some more alpha reduction benches. - Changed use of `chunks()` to `chunks_exact()` where appropriate to improve performance. (As a byproduct, this also fixes a potential crash if a malformed file has a PLTE chunk that isn't a multiple of 3.)
shssoichiro · Feb 8, 2025 · 1188b9a · 1188b9a
1 parent 5a4cfa9
commit 1188b9a
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 97 deletions.
diff --git a/benches/reductions.rs b/benches/reductions.rs
@@ -54,36 +54,6 @@ fn reductions_8_to_1_bits(b: &mut Bencher) {
     b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
 }
 
-#[bench]
-fn reductions_4_to_2_bits(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/palette_4_should_be_palette_2.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
-}
-
-#[bench]
-fn reductions_4_to_1_bits(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/palette_4_should_be_palette_1.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
-}
-
-#[bench]
-fn reductions_2_to_1_bits(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/palette_2_should_be_palette_1.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
-}
-
 #[bench]
 fn reductions_grayscale_8_to_4_bits(b: &mut Bencher) {
     let input = test::black_box(PathBuf::from(
@@ -115,49 +85,27 @@ fn reductions_grayscale_8_to_1_bits(b: &mut Bencher) {
 }
 
 #[bench]
-fn reductions_grayscale_4_to_2_bits(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/grayscale_4_should_be_grayscale_2.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
-}
-
-#[bench]
-fn reductions_grayscale_4_to_1_bits(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/grayscale_4_should_be_grayscale_1.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
-}
-
-#[bench]
-fn reductions_grayscale_2_to_1_bits(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/grayscale_2_should_be_grayscale_1.png",
-    ));
+fn reductions_rgba_to_rgb_16(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from("tests/files/rgba_16_should_be_rgb_16.png"));
     let png = PngData::new(&input, &Options::default()).unwrap();
 
-    b.iter(|| bit_depth::reduced_bit_depth_8_or_less(&png.raw));
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, true));
 }
 
 #[bench]
-fn reductions_rgba_to_rgb_16(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from("tests/files/rgba_16_should_be_rgb_16.png"));
+fn reductions_rgba_to_rgb_8(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from("tests/files/rgba_8_should_be_rgb_8.png"));
     let png = PngData::new(&input, &Options::default()).unwrap();
 
-    b.iter(|| alpha::reduced_alpha_channel(&png.raw, false));
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, true));
 }
 
 #[bench]
-fn reductions_rgba_to_rgb_8(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from("tests/files/rgba_8_should_be_rgb_8.png"));
+fn reductions_rgba_to_rgb_trns_8(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from("tests/files/rgba_8_should_be_rgb_trns_8.png"));
     let png = PngData::new(&input, &Options::default()).unwrap();
 
-    b.iter(|| alpha::reduced_alpha_channel(&png.raw, false));
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, true));
 }
 
 #[bench]
@@ -180,32 +128,6 @@ fn reductions_rgba_to_grayscale_alpha_8(b: &mut Bencher) {
     b.iter(|| color::reduced_rgb_to_grayscale(&png.raw));
 }
 
-#[bench]
-fn reductions_rgba_to_grayscale_16(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/rgba_16_should_be_grayscale_16.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| {
-        color::reduced_rgb_to_grayscale(&png.raw)
-            .and_then(|r| alpha::reduced_alpha_channel(&r, false))
-    });
-}
-
-#[bench]
-fn reductions_rgba_to_grayscale_8(b: &mut Bencher) {
-    let input = test::black_box(PathBuf::from(
-        "tests/files/rgba_8_should_be_grayscale_8.png",
-    ));
-    let png = PngData::new(&input, &Options::default()).unwrap();
-
-    b.iter(|| {
-        color::reduced_rgb_to_grayscale(&png.raw)
-            .and_then(|r| alpha::reduced_alpha_channel(&r, false))
-    });
-}
-
 #[bench]
 fn reductions_rgb_to_grayscale_16(b: &mut Bencher) {
     let input = test::black_box(PathBuf::from(
@@ -240,6 +162,36 @@ fn reductions_rgb_to_palette_8(b: &mut Bencher) {
     b.iter(|| color::reduced_to_indexed(&png.raw, true));
 }
 
+#[bench]
+fn reductions_grayscale_alpha_to_grayscale_16(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from(
+        "tests/files/grayscale_alpha_16_should_be_grayscale_16.png",
+    ));
+    let png = PngData::new(&input, &Options::default()).unwrap();
+
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, true));
+}
+
+#[bench]
+fn reductions_grayscale_alpha_to_grayscale_8(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from(
+        "tests/files/grayscale_alpha_8_should_be_grayscale_8.png",
+    ));
+    let png = PngData::new(&input, &Options::default()).unwrap();
+
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, true));
+}
+
+#[bench]
+fn reductions_grayscale_alpha_to_grayscale_trns_8(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from(
+        "tests/files/grayscale_alpha_8_should_be_grayscale_trns_8.png",
+    ));
+    let png = PngData::new(&input, &Options::default()).unwrap();
+
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, true));
+}
+
 #[bench]
 fn reductions_grayscale_8_to_palette_8(b: &mut Bencher) {
     let input = test::black_box(PathBuf::from(

diff --git a/src/filters.rs b/src/filters.rs
@@ -126,8 +126,8 @@ impl RowFilter {
             return;
         }
 
-        let mut pixels: Vec<_> = data.chunks_mut(bpp).collect();
-        let prev_pixels: Vec<_> = prev_line.chunks(bpp).collect();
+        let mut pixels: Vec<_> = data.chunks_exact_mut(bpp).collect();
+        let prev_pixels: Vec<_> = prev_line.chunks_exact(bpp).collect();
         for i in 0..pixels.len() {
             if pixels[i].iter().skip(color_bytes).all(|b| *b == 0) {
                 // If the first pixel in the row is transparent, find the next non-transparent pixel and pretend

diff --git a/src/headers.rs b/src/headers.rs
@@ -225,7 +225,7 @@ fn palette_to_rgba(
 ) -> Result<Vec<RGBA8>, PngError> {
     let palette_data = palette_data.ok_or_else(|| PngError::new("no palette in indexed image"))?;
     let mut palette: Vec<_> = palette_data
-        .chunks(3)
+        .chunks_exact(3)
         .map(|color| RGBA8::new(color[0], color[1], color[2], 255))
         .collect();
 

diff --git a/src/reduction/alpha.rs b/src/reduction/alpha.rs
@@ -17,7 +17,7 @@ pub fn cleaned_alpha_channel(png: &PngImage) -> Option<PngImage> {
     let colored_bytes = bpp - byte_depth;
 
     let mut reduced = Vec::with_capacity(png.data.len());
-    for pixel in png.data.chunks(bpp) {
+    for pixel in png.data.chunks_exact(bpp) {
         if pixel.iter().skip(colored_bytes).all(|b| *b == 0) {
             reduced.resize(reduced.len() + bpp, 0);
         } else {
@@ -46,7 +46,7 @@ pub fn reduced_alpha_channel(png: &PngImage, optimize_alpha: bool) -> Option<Png
     let mut has_transparency = false;
     let mut used_colors = vec![false; 256];
 
-    for pixel in png.data.chunks(bpp) {
+    for pixel in png.data.chunks_exact(bpp) {
         if optimize_alpha && pixel.iter().skip(colored_bytes).all(|b| *b == 0) {
             // Fully transparent, we may be able to reduce with tRNS
             has_transparency = true;
@@ -75,7 +75,7 @@ pub fn reduced_alpha_channel(png: &PngImage, optimize_alpha: bool) -> Option<Png
     };
 
     let mut raw_data = Vec::with_capacity(png.data.len());
-    for pixel in png.data.chunks(bpp) {
+    for pixel in png.data.chunks_exact(bpp) {
         match transparency_pixel {
             Some(trns) if pixel.iter().skip(colored_bytes).all(|b| *b == 0) => {
                 raw_data.resize(raw_data.len() + colored_bytes, trns);

diff --git a/src/reduction/bit_depth.rs b/src/reduction/bit_depth.rs
@@ -16,13 +16,13 @@ pub fn reduced_bit_depth_16_to_8(png: &PngImage, force_scale: bool) -> Option<Pn
     }
 
     // Reduce from 16 to 8 bits per channel per pixel
-    if png.data.chunks(2).any(|pair| pair[0] != pair[1]) {
+    if png.data.chunks_exact(2).any(|pair| pair[0] != pair[1]) {
         // Can't reduce
         return None;
     }
 
     Some(PngImage {
-        data: png.data.iter().step_by(2).copied().collect(),
+        data: png.data.chunks_exact(2).map(|pair| pair[0]).collect(),
         ihdr: IhdrData {
             color_type: png.ihdr.color_type.clone(),
             bit_depth: BitDepth::Eight,
@@ -41,7 +41,7 @@ pub fn scaled_bit_depth_16_to_8(png: &PngImage) -> Option<PngImage> {
     // Reduce from 16 to 8 bits per channel per pixel by scaling when necessary
     let data = png
         .data
-        .chunks(2)
+        .chunks_exact(2)
         .map(|pair| {
             if pair[0] == pair[1] {
                 return pair[0];

diff --git a/src/reduction/color.rs b/src/reduction/color.rs
@@ -106,7 +106,7 @@ pub fn reduced_rgb_to_grayscale(png: &PngImage) -> Option<PngImage> {
     let byte_depth = png.bytes_per_channel();
     let bpp = png.channels_per_pixel() * byte_depth;
     let last_color = 2 * byte_depth;
-    for pixel in png.data.chunks(bpp) {
+    for pixel in png.data.chunks_exact(bpp) {
         if byte_depth == 1 {
             if pixel[0] != pixel[1] || pixel[1] != pixel[2] {
                 return None;