Faster reductions (#479)

* Add grayscale depth tests and benches

* Don't include filter byte in PngImage.data

* Simplify reductions by not using scan lines

* Faster grayscale reduction

* Simplify reduce_color_type

* Faster depth reduction

benches/reductions.rs

@@ -75,6 +75,66 @@ fn reductions_2_to_1_bits(b: &mut Bencher) {
     b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
 }
 
+#[bench]
+fn reductions_grayscale_8_to_4_bits(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from(
+        "tests/files/grayscale_8_should_be_grayscale_4.png",
+    ));
+    let png = PngData::new(&input, false).unwrap();
+
+    b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
+}
+
+#[bench]
+fn reductions_grayscale_8_to_2_bits(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from(
+        "tests/files/grayscale_8_should_be_grayscale_2.png",
+    ));
+    let png = PngData::new(&input, false).unwrap();
+
+    b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
+}
+
+#[bench]
+fn reductions_grayscale_8_to_1_bits(b: &mut Bencher) {
+    let input = test::black_box(PathBuf::from(
+        "tests/files/grayscale_8_should_be_grayscale_1.png",
+    ));
+    let png = PngData::new(&input, false).unwrap();
+
+    b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
+}
+
+#[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, false).unwrap();
+
+    b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
+}
+
+#[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, false).unwrap();
+
+    b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
+}
+
+#[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",
+    ));
+    let png = PngData::new(&input, false).unwrap();
+
+    b.iter(|| bit_depth::reduce_bit_depth(&png.raw, 1));
+}
+
 #[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"));

src/interlace.rs

@@ -41,40 +41,7 @@ impl Display for Interlacing {
 pub fn interlace_image(png: &PngImage) -> PngImage {
     let mut passes: Vec<BitVec<u8, Msb0>> = vec![BitVec::new(); 7];
     let bits_per_pixel = png.ihdr.bpp();
-    for (index, line) in png.scan_lines().enumerate() {
-        match index % 8 {
-            // Add filter bytes to passes that will be in the output image
-            0 => {
-                passes[0].extend_from_raw_slice(&[0]);
-                if png.ihdr.width >= 5 {
-                    passes[1].extend_from_raw_slice(&[0]);
-                }
-                if png.ihdr.width >= 3 {
-                    passes[3].extend_from_raw_slice(&[0]);
-                }
-                if png.ihdr.width >= 2 {
-                    passes[5].extend_from_raw_slice(&[0]);
-                }
-            }
-            4 => {
-                passes[2].extend_from_raw_slice(&[0]);
-                if png.ihdr.width >= 3 {
-                    passes[3].extend_from_raw_slice(&[0]);
-                }
-                if png.ihdr.width >= 2 {
-                    passes[5].extend_from_raw_slice(&[0]);
-                }
-            }
-            2 | 6 => {
-                passes[4].extend_from_raw_slice(&[0]);
-                if png.ihdr.width >= 2 {
-                    passes[5].extend_from_raw_slice(&[0]);
-                }
-            }
-            _ => {
-                passes[6].extend_from_raw_slice(&[0]);
-            }
-        }
+    for (index, line) in png.scan_lines(false).enumerate() {
         let bit_vec = line.data.view_bits::<Msb0>();
         for (i, bit) in bit_vec.iter().by_vals().enumerate() {
             // Avoid moving padded 0's into new image
@@ -148,15 +115,14 @@ pub fn deinterlace_image(png: &PngImage) -> PngImage {
 /// Deinterlace by bits, for images with less than 8bpp
 fn deinterlace_bits(png: &PngImage) -> Vec<u8> {
     let bits_per_pixel = png.ihdr.bpp();
-    let bits_per_line = 8 + bits_per_pixel as usize * png.ihdr.width as usize;
-    // Initialize each output line with a starting filter byte of 0
-    // as well as some blank data
+    let bits_per_line = bits_per_pixel as usize * png.ihdr.width as usize;
+    // Initialize each output line with blank data
     let mut lines: Vec<BitVec<u8, Msb0>> =
         vec![bitvec![u8, Msb0; 0; bits_per_line]; png.ihdr.height as usize];
     let mut current_pass = 1;
     let mut pass_constants = interlaced_constants(current_pass);
     let mut current_y: usize = pass_constants.y_shift as usize;
-    for line in png.scan_lines() {
+    for line in png.scan_lines(false) {
         let bit_vec = line.data.view_bits::<Msb0>();
         let bits_in_line = ((png.ihdr.width - u32::from(pass_constants.x_shift)
             + u32::from(pass_constants.x_step)
@@ -170,8 +136,8 @@ fn deinterlace_bits(png: &PngImage) -> Vec<u8> {
             }
             let current_x: usize = pass_constants.x_shift as usize
                 + (i / bits_per_pixel as usize) * pass_constants.x_step as usize;
-            // Copy this bit into the output line, offset by 8 because of filter byte
-            let index = 8 + (i % bits_per_pixel as usize) + current_x * bits_per_pixel as usize;
+            // Copy this bit into the output line
+            let index = (i % bits_per_pixel as usize) + current_x * bits_per_pixel as usize;
             lines[current_y].set(index, bit);
         }
         // Calculate the next line and move to next pass if necessary
@@ -197,19 +163,18 @@ fn deinterlace_bits(png: &PngImage) -> Vec<u8> {
 /// Deinterlace by bytes, for images with at least 8bpp
 fn deinterlace_bytes(png: &PngImage) -> Vec<u8> {
     let bytes_per_pixel = png.ihdr.bpp() / 8;
-    let bytes_per_line = 1 + bytes_per_pixel as usize * png.ihdr.width as usize;
-    // Initialize each output line with a starting filter byte of 0
-    // as well as some blank data
+    let bytes_per_line = bytes_per_pixel as usize * png.ihdr.width as usize;
+    // Initialize each output line with some blank data
     let mut lines: Vec<Vec<u8>> = vec![vec![0; bytes_per_line]; png.ihdr.height as usize];
     let mut current_pass = 1;
     let mut pass_constants = interlaced_constants(current_pass);
     let mut current_y: usize = pass_constants.y_shift as usize;
-    for line in png.scan_lines() {
+    for line in png.scan_lines(false) {
         for (i, byte) in line.data.iter().enumerate() {
             let current_x: usize = pass_constants.x_shift as usize
                 + (i / bytes_per_pixel as usize) * pass_constants.x_step as usize;
-            // Copy this byte into the output line, offset by 1 because of filter byte
-            let index = 1 + (i % bytes_per_pixel as usize) + current_x * bytes_per_pixel as usize;
+            // Copy this byte into the output line
+            let index = (i % bytes_per_pixel as usize) + current_x * bytes_per_pixel as usize;
             lines[current_y][index] = *byte;
         }
         // Calculate the next line and move to next pass if necessary

src/png/mod.rs

@@ -18,7 +18,7 @@ use std::sync::Arc;
 
 pub(crate) mod scan_lines;
 
-use self::scan_lines::{ScanLines, ScanLinesMut};
+use self::scan_lines::ScanLines;
 
 /// Compression level to use for the Brute filter strategy
 const BRUTE_LEVEL: i32 = 1; // 1 is fastest, 2-4 are not useful, 5 is slower but more effective
@@ -29,7 +29,7 @@ const BRUTE_LINES: usize = 4; // Values over 8 are generally not useful
 pub struct PngImage {
     /// The headers stored in the IHDR chunk
     pub ihdr: IhdrData,
-    /// The uncompressed, optionally filtered data from the IDAT chunk
+    /// The uncompressed, unfiltered data from the IDAT chunk
     pub data: Vec<u8>,
     /// The palette containing colors used in an Indexed image
     /// Contains 3 bytes per color (R+G+B), up to 768
@@ -280,14 +280,8 @@ impl PngImage {
 
     /// Return an iterator over the scanlines of the image
     #[inline]
-    pub fn scan_lines(&self) -> ScanLines<'_> {
-        ScanLines::new(self)
-    }
-
-    /// Return an iterator over the scanlines of the image
-    #[inline]
-    pub fn scan_lines_mut(&mut self) -> ScanLinesMut<'_> {
-        ScanLinesMut::new(self)
+    pub fn scan_lines(&self, has_filter: bool) -> ScanLines<'_> {
+        ScanLines::new(self, has_filter)
     }
 
     /// Reverse all filters applied on the image, returning an unfiltered IDAT bytestream
@@ -297,7 +291,7 @@ impl PngImage {
         let mut last_line: Vec<u8> = Vec::new();
         let mut last_pass = None;
         let mut unfiltered_buf = Vec::new();
-        for line in self.scan_lines() {
+        for line in self.scan_lines(true) {
             if last_pass != line.pass {
                 last_line.clear();
                 last_pass = line.pass;
@@ -305,7 +299,6 @@ impl PngImage {
             last_line.resize(line.data.len(), 0);
             let filter = RowFilter::try_from(line.filter).map_err(|_| PngError::InvalidData)?;
             filter.unfilter_line(bpp, line.data, &last_line, &mut unfiltered_buf)?;
-            unfiltered.push(0);
             unfiltered.extend_from_slice(&unfiltered_buf);
             std::mem::swap(&mut last_line, &mut unfiltered_buf);
             unfiltered_buf.clear();
@@ -328,7 +321,7 @@ impl PngImage {
         let mut prev_line = Vec::new();
         let mut prev_pass: Option<u8> = None;
         let mut f_buf = Vec::new();
-        for line in self.scan_lines() {
+        for line in self.scan_lines(false) {
             if prev_pass != line.pass || line.data.len() != prev_line.len() {
                 prev_line = vec![0; line.data.len()];
             }

src/png/scan_lines.rs

@@ -7,13 +7,16 @@ pub struct ScanLines<'a> {
     iter: ScanLineRanges,
     /// A reference to the PNG image being iterated upon
     raw_data: &'a [u8],
+    /// Whether the raw data contains filter bytes
+    has_filter: bool,
 }
 
 impl<'a> ScanLines<'a> {
-    pub fn new(png: &'a PngImage) -> Self {
+    pub fn new(png: &'a PngImage, has_filter: bool) -> Self {
         Self {
-            iter: ScanLineRanges::new(png),
+            iter: ScanLineRanges::new(png, has_filter),
             raw_data: &png.data,
+            has_filter,
         }
     }
 }
@@ -25,43 +28,16 @@ impl<'a> Iterator for ScanLines<'a> {
         self.iter.next().map(|(len, pass)| {
             let (data, rest) = self.raw_data.split_at(len);
             self.raw_data = rest;
-            let (&filter, data) = data.split_first().unwrap();
+            let (&filter, data) = if self.has_filter {
+                data.split_first().unwrap()
+            } else {
+                (&0, data)
+            };
             ScanLine { filter, data, pass }
         })
     }
 }
 
-#[derive(Debug)]
-/// An iterator over the scan lines of a PNG image
-pub struct ScanLinesMut<'a> {
-    iter: ScanLineRanges,
-    /// A reference to the PNG image being iterated upon
-    raw_data: Option<&'a mut [u8]>,
-}
-
-impl<'a> ScanLinesMut<'a> {
-    pub fn new(png: &'a mut PngImage) -> Self {
-        Self {
-            iter: ScanLineRanges::new(png),
-            raw_data: Some(&mut png.data),
-        }
-    }
-}
-
-impl<'a> Iterator for ScanLinesMut<'a> {
-    type Item = ScanLineMut<'a>;
-    #[inline]
-    fn next(&mut self) -> Option<Self::Item> {
-        self.iter.next().map(|(len, pass)| {
-            let tmp = self.raw_data.take().unwrap();
-            let (data, rest) = tmp.split_at_mut(len);
-            self.raw_data = Some(rest);
-            let (&mut filter, data) = data.split_first_mut().unwrap();
-            ScanLineMut { filter, data, pass }
-        })
-    }
-}
-
 #[derive(Debug, Clone)]
 /// An iterator over the scan line locations of a PNG image
 struct ScanLineRanges {
@@ -71,10 +47,11 @@ struct ScanLineRanges {
     width: u32,
     height: u32,
     left: usize,
+    has_filter: bool,
 }
 
 impl ScanLineRanges {
-    pub fn new(png: &PngImage) -> Self {
+    pub fn new(png: &PngImage, has_filter: bool) -> Self {
         Self {
             bits_per_pixel: png.ihdr.bit_depth.as_u8() * png.channels_per_pixel(),
             width: png.ihdr.width,
@@ -85,6 +62,7 @@ impl ScanLineRanges {
             } else {
                 None
             },
+            has_filter,
         }
     }
 }
@@ -166,8 +144,10 @@ impl Iterator for ScanLineRanges {
             (self.width, None)
         };
         let bits_per_line = pixels_per_line * u32::from(self.bits_per_pixel);
-        let bytes_per_line = ((bits_per_line + 7) / 8) as usize;
-        let len = bytes_per_line + 1;
+        let mut len = ((bits_per_line + 7) / 8) as usize;
+        if self.has_filter {
+            len += 1;
+        }
         self.left = self.left.checked_sub(len)?;
         Some((len, current_pass))
     }
@@ -183,14 +163,3 @@ pub struct ScanLine<'a> {
     /// The current pass if the image is interlaced
     pub pass: Option<u8>,
 }
-
-#[derive(Debug)]
-/// A scan line in a PNG image
-pub struct ScanLineMut<'a> {
-    /// The filter type used to encode the current scan line (0-4)
-    pub filter: u8,
-    /// The byte data for the current scan line, encoded with the filter specified in the `filter` field
-    pub data: &'a mut [u8],
-    /// The current pass if the image is interlaced
-    pub pass: Option<u8>,
-}

src/reduction/alpha.rs

@@ -16,14 +16,11 @@ pub fn cleaned_alpha_channel(png: &PngImage) -> Option<PngImage> {
     };
 
     let mut reduced = Vec::with_capacity(png.data.len());
-    for line in png.scan_lines() {
-        reduced.push(line.filter);
-        for pixel in line.data.chunks(bpp) {
-            if pixel.iter().skip(bpp - bpc).all(|b| *b == 0) {
-                reduced.resize(reduced.len() + bpp, 0);
-            } else {
-                reduced.extend_from_slice(pixel);
-            }
+    for pixel in png.data.chunks(bpp) {
+        if pixel.iter().skip(bpp - bpc).all(|b| *b == 0) {
+            reduced.resize(reduced.len() + bpp, 0);
+        } else {
+            reduced.extend_from_slice(pixel);
         }
     }
 
@@ -54,18 +51,16 @@ 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 line in png.scan_lines() {
-        for pixel in line.data.chunks(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;
-            } else if pixel.iter().skip(colored_bytes).any(|b| *b != 255) {
-                // Partially transparent, the image is not reducible
-                return None;
-            } else if optimize_alpha && pixel.iter().take(colored_bytes).all(|b| *b == pixel[0]) {
-                // Opaque shade of gray, we can't use this color for tRNS
-                used_colors[pixel[0] as usize] = true;
-            }
+    for pixel in png.data.chunks(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;
+        } else if pixel.iter().skip(colored_bytes).any(|b| *b != 255) {
+            // Partially transparent, the image is not reducible
+            return None;
+        } else if optimize_alpha && pixel.iter().take(colored_bytes).all(|b| *b == pixel[0]) {
+            // Opaque shade of gray, we can't use this color for tRNS
+            used_colors[pixel[0] as usize] = true;
         }
     }
 
@@ -83,16 +78,13 @@ pub fn reduced_alpha_channel(png: &PngImage, optimize_alpha: bool) -> Option<Png
     };
 
     let mut raw_data = Vec::with_capacity(png.data.len());
-    for line in png.scan_lines() {
-        raw_data.push(line.filter);
-        for pixel in line.data.chunks(bpp) {
-            match transparency_pixel {
-                Some(ref trns) if pixel.iter().skip(colored_bytes).all(|b| *b == 0) => {
-                    raw_data.resize(raw_data.len() + colored_bytes, trns[1]);
-                }
-                _ => raw_data.extend_from_slice(&pixel[0..colored_bytes]),
-            };
-        }
+    for pixel in png.data.chunks(bpp) {
+        match transparency_pixel {
+            Some(ref trns) if pixel.iter().skip(colored_bytes).all(|b| *b == 0) => {
+                raw_data.resize(raw_data.len() + colored_bytes, trns[1]);
+            }
+            _ => raw_data.extend_from_slice(&pixel[0..colored_bytes]),
+        };
     }
 
     let mut aux_headers = png.aux_headers.clone();

src/reduction/bit_depth.rs

@@ -1,28 +1,6 @@
 use crate::colors::{BitDepth, ColorType};
 use crate::headers::IhdrData;
 use crate::png::PngImage;
-use bitvec::prelude::*;
-
-const ONE_BIT_PERMUTATIONS: [u8; 2] = [0b0000_0000, 0b1111_1111];
-const TWO_BIT_PERMUTATIONS: [u8; 4] = [0b0000_0000, 0b0101_0101, 0b1010_1010, 0b1111_1111];
-const FOUR_BIT_PERMUTATIONS: [u8; 16] = [
-    0b0000_0000,
-    0b0001_0001,
-    0b0010_0010,
-    0b0011_0011,
-    0b0100_0100,
-    0b0101_0101,
-    0b0110_0110,
-    0b0111_0111,
-    0b1000_1000,
-    0b1001_1001,
-    0b1010_1010,
-    0b1011_1011,
-    0b1100_1100,
-    0b1101_1101,
-    0b1110_1110,
-    0b1111_1111,
-];
 
 /// Attempt to reduce the bit depth of the image
 /// Returns true if the bit depth was reduced, false otherwise
@@ -37,31 +15,13 @@ pub fn reduce_bit_depth(png: &PngImage, minimum_bits: usize) -> Option<PngImage>
     }
 
     // Reduce from 16 to 8 bits per channel per pixel
-    let mut reduced = Vec::with_capacity(
-        (png.ihdr.width * png.ihdr.height * u32::from(png.channels_per_pixel()) + png.ihdr.height)
-            as usize,
-    );
-    let mut high_byte = 0;
-
-    for line in png.scan_lines() {
-        reduced.push(line.filter);
-        for (i, &byte) in line.data.iter().enumerate() {
-            if i % 2 == 0 {
-                // High byte
-                high_byte = byte;
-            } else {
-                // Low byte
-                if high_byte != byte {
-                    // Can't reduce, exit early
-                    return None;
-                }
-                reduced.push(byte);
-            }
-        }
+    if png.data.chunks(2).any(|pair| pair[0] != pair[1]) {
+        // Can't reduce
+        return None;
     }
 
     Some(PngImage {
-        data: reduced,
+        data: png.data.iter().step_by(2).cloned().collect(),
         ihdr: IhdrData {
             bit_depth: BitDepth::Eight,
             ..png.ihdr
@@ -76,11 +36,14 @@ pub fn reduce_bit_depth(png: &PngImage, minimum_bits: usize) -> Option<PngImage>
 pub fn reduce_bit_depth_8_or_less(png: &PngImage, mut minimum_bits: usize) -> Option<PngImage> {
     assert!((1..8).contains(&minimum_bits));
     let bit_depth: usize = png.ihdr.bit_depth.as_u8() as usize;
-    if minimum_bits >= bit_depth {
+    if minimum_bits >= bit_depth || bit_depth > 8 {
         return None;
     }
-    for line in png.scan_lines() {
-        if png.ihdr.color_type == ColorType::Indexed {
+    // Calculate the current number of pixels per byte
+    let ppb = 8 / bit_depth;
+
+    if png.ihdr.color_type == ColorType::Indexed {
+        for line in png.scan_lines(false) {
             let line_max = line
                 .data
                 .iter()
@@ -107,40 +70,62 @@ pub fn reduce_bit_depth_8_or_less(png: &PngImage, mut minimum_bits: usize) -> Op
                     return None;
                 }
             }
-        } else {
-            for &byte in line.data {
-                while minimum_bits < bit_depth {
-                    let permutations: &[u8] = if minimum_bits == 1 {
-                        &ONE_BIT_PERMUTATIONS
-                    } else if minimum_bits == 2 {
-                        &TWO_BIT_PERMUTATIONS
-                    } else if minimum_bits == 4 {
-                        &FOUR_BIT_PERMUTATIONS
-                    } else {
-                        return None;
-                    };
-                    if permutations.iter().any(|perm| *perm == byte) {
-                        break;
+        }
+    } else {
+        // Checking for grayscale depth reduction is quite different than for indexed
+        // Note: In rare cases, padding bits in the data may cause this to incorrectly return None
+        let mut mask = (1 << minimum_bits) - 1;
+        let mut divisions = 1..(bit_depth / minimum_bits);
+        for &b in &png.data {
+            if b == 0 || b == 255 {
+                continue;
+            }
+            'try_depth: loop {
+                let mut byte = b;
+                // Loop over each pixel in the byte
+                for _ in 0..ppb {
+                    // Align the first pixel division with the mask
+                    byte = byte.rotate_left(minimum_bits as u32);
+                    // Each potential division of this pixel must be identical to successfully reduce
+                    let compare = byte & mask;
+                    for _ in divisions.clone() {
+                        // Align the next division with the mask
+                        byte = byte.rotate_left(minimum_bits as u32);
+                        if byte & mask != compare {
+                            // This depth is not possible, try the next one up
+                            minimum_bits <<= 1;
+                            if minimum_bits == bit_depth {
+                                return None;
+                            }
+                            mask = (1 << minimum_bits) - 1;
+                            divisions = 1..(bit_depth / minimum_bits);
+                            continue 'try_depth;
+                        }
                     }
-                    minimum_bits <<= 1;
                 }
+                break;
             }
         }
     }
 
-    let mut reduced = BitVec::<u8, Msb0>::with_capacity(png.data.len() * 8);
-    for line in png.scan_lines() {
-        reduced.extend_from_raw_slice(&[line.filter]);
-        let bit_vec = line.data.view_bits::<Msb0>();
-        for (i, bit) in bit_vec.iter().by_vals().enumerate() {
-            let bit_index = bit_depth - (i % bit_depth);
-            if bit_index <= minimum_bits {
-                reduced.push(bit);
+    let mut reduced = Vec::with_capacity(png.data.len());
+    let mask = (1 << minimum_bits) - 1;
+    for line in png.scan_lines(false) {
+        // Loop over the data in chunks that will produce 1 byte of output
+        for chunk in line.data.chunks(bit_depth / minimum_bits) {
+            let mut new_byte = 0;
+            let mut shift = 8;
+            for &(mut byte) in chunk {
+                // Loop over each pixel in the byte
+                for _ in 0..ppb {
+                    // Align the current pixel with the mask
+                    byte = byte.rotate_left(bit_depth as u32);
+                    shift -= minimum_bits;
+                    // Take the low bits of the pixel and shift them into the output byte
+                    new_byte |= (byte & mask) << shift;
+                }
             }
-        }
-        // Pad end of line to get 8 bits per byte
-        while reduced.len() % 8 != 0 {
-            reduced.push(false);
+            reduced.push(new_byte);
         }
     }
 
@@ -167,7 +152,7 @@ pub fn reduce_bit_depth_8_or_less(png: &PngImage, mut minimum_bits: usize) -> Op
     }
 
     Some(PngImage {
-        data: reduced.as_raw_slice().to_vec(),
+        data: reduced,
         ihdr: IhdrData {
             bit_depth: BitDepth::from_u8(minimum_bits as u8),
             ..png.ihdr

src/reduction/color.rs

@@ -2,83 +2,12 @@ use crate::colors::{BitDepth, ColorType};
 use crate::headers::IhdrData;
 use crate::png::PngImage;
 use indexmap::IndexMap;
-use itertools::Itertools;
 use rgb::{FromSlice, RGB8, RGBA, RGBA8};
 use rustc_hash::FxHasher;
 use std::hash::{BuildHasherDefault, Hash};
 
 type FxIndexMap<K, V> = IndexMap<K, V, BuildHasherDefault<FxHasher>>;
 
-#[must_use]
-pub fn reduce_rgba_to_grayscale_alpha(png: &PngImage) -> Option<PngImage> {
-    let mut reduced = Vec::with_capacity(png.data.len());
-    let byte_depth = png.ihdr.bit_depth.as_u8() >> 3;
-    let bpp = 4 * byte_depth;
-    let bpp_mask = bpp - 1;
-    if 0 != bpp & bpp_mask {
-        return None;
-    }
-    let colored_bytes = bpp - byte_depth;
-    for line in png.scan_lines() {
-        reduced.push(line.filter);
-        let mut low_bytes = Vec::with_capacity(4);
-        let mut high_bytes = Vec::with_capacity(4);
-        let mut trans_bytes = Vec::with_capacity(byte_depth as usize);
-        for (i, byte) in line.data.iter().enumerate() {
-            if i as u8 & bpp_mask < colored_bytes {
-                if byte_depth == 1 || i % 2 == 1 {
-                    low_bytes.push(*byte);
-                } else {
-                    high_bytes.push(*byte);
-                }
-            } else {
-                trans_bytes.push(*byte);
-            }
-
-            if (i as u8 & bpp_mask) == bpp - 1 {
-                if low_bytes.iter().unique().count() > 1 {
-                    return None;
-                }
-                if byte_depth == 2 {
-                    if high_bytes.iter().unique().count() > 1 {
-                        return None;
-                    }
-                    reduced.push(high_bytes[0]);
-                    high_bytes.clear();
-                }
-                reduced.push(low_bytes[0]);
-                low_bytes.clear();
-                reduced.extend_from_slice(&trans_bytes);
-                trans_bytes.clear();
-            }
-        }
-    }
-
-    let mut aux_headers = png.aux_headers.clone();
-    if let Some(sbit_header) = png.aux_headers.get(b"sBIT") {
-        if let Some(&s) = sbit_header.first() {
-            aux_headers.insert(*b"sBIT", vec![s]);
-        }
-    }
-
-    if let Some(bkgd_header) = png.aux_headers.get(b"bKGD") {
-        if let Some(b) = bkgd_header.get(0..2) {
-            aux_headers.insert(*b"bKGD", b.to_owned());
-        }
-    }
-
-    Some(PngImage {
-        data: reduced,
-        ihdr: IhdrData {
-            color_type: ColorType::GrayscaleAlpha,
-            ..png.ihdr
-        },
-        palette: None,
-        transparency_pixel: None,
-        aux_headers,
-    })
-}
-
 fn reduce_scanline_to_palette<T>(
     iter: impl IntoIterator<Item = T>,
     palette: &mut FxIndexMap<T, u8>,
@@ -117,42 +46,39 @@ pub fn reduce_to_palette(png: &PngImage) -> Option<PngImage> {
         .as_ref()
         .filter(|t| png.ihdr.color_type == ColorType::RGB && t.len() >= 6)
         .map(|t| RGB8::new(t[1], t[3], t[5]));
-    for line in png.scan_lines() {
-        raw_data.push(line.filter);
-        let ok = if png.ihdr.color_type == ColorType::RGB {
-            reduce_scanline_to_palette(
-                line.data.as_rgb().iter().cloned().map(|px| {
-                    px.alpha(if Some(px) != transparency_pixel {
-                        255
-                    } else {
-                        0
-                    })
-                }),
-                &mut palette,
-                &mut raw_data,
-            )
-        } else if png.ihdr.color_type == ColorType::GrayscaleAlpha {
-            reduce_scanline_to_palette(
-                line.data.as_gray_alpha().iter().cloned().map(|px| RGBA {
-                    r: px.0,
-                    g: px.0,
-                    b: px.0,
-                    a: px.1,
-                }),
-                &mut palette,
-                &mut raw_data,
-            )
-        } else {
-            debug_assert_eq!(png.ihdr.color_type, ColorType::RGBA);
-            reduce_scanline_to_palette(
-                line.data.as_rgba().iter().cloned(),
-                &mut palette,
-                &mut raw_data,
-            )
-        };
-        if !ok {
-            return None;
-        }
+    let ok = if png.ihdr.color_type == ColorType::RGB {
+        reduce_scanline_to_palette(
+            png.data.as_rgb().iter().cloned().map(|px| {
+                px.alpha(if Some(px) != transparency_pixel {
+                    255
+                } else {
+                    0
+                })
+            }),
+            &mut palette,
+            &mut raw_data,
+        )
+    } else if png.ihdr.color_type == ColorType::GrayscaleAlpha {
+        reduce_scanline_to_palette(
+            png.data.as_gray_alpha().iter().cloned().map(|px| RGBA {
+                r: px.0,
+                g: px.0,
+                b: px.0,
+                a: px.1,
+            }),
+            &mut palette,
+            &mut raw_data,
+        )
+    } else {
+        debug_assert_eq!(png.ihdr.color_type, ColorType::RGBA);
+        reduce_scanline_to_palette(
+            png.data.as_rgba().iter().cloned(),
+            &mut palette,
+            &mut raw_data,
+        )
+    };
+    if !ok {
+        return None;
     }
 
     let num_transparent = palette
@@ -218,36 +144,18 @@ pub fn reduce_to_palette(png: &PngImage) -> Option<PngImage> {
 #[must_use]
 pub fn reduce_rgb_to_grayscale(png: &PngImage) -> Option<PngImage> {
     let mut reduced = Vec::with_capacity(png.data.len());
-    let byte_depth: u8 = png.ihdr.bit_depth.as_u8() >> 3;
-    let bpp: usize = 3 * byte_depth as usize;
-    let mut cur_pixel = Vec::with_capacity(bpp);
-    for line in png.scan_lines() {
-        reduced.push(line.filter);
-        for (i, byte) in line.data.iter().enumerate() {
-            cur_pixel.push(*byte);
-            if i % bpp == bpp - 1 {
-                if bpp == 3 {
-                    if cur_pixel.iter().unique().count() > 1 {
-                        return None;
-                    }
-                    reduced.push(cur_pixel[0]);
-                } else {
-                    let pixel_bytes = cur_pixel
-                        .iter()
-                        .step_by(2)
-                        .cloned()
-                        .zip(cur_pixel.iter().skip(1).step_by(2).cloned())
-                        .unique()
-                        .collect::<Vec<(u8, u8)>>();
-                    if pixel_bytes.len() > 1 {
-                        return None;
-                    }
-                    reduced.push(pixel_bytes[0].0);
-                    reduced.push(pixel_bytes[0].1);
-                }
-                cur_pixel.clear();
+    let byte_depth = png.ihdr.bit_depth.as_u8() as usize >> 3;
+    let bpp = png.channels_per_pixel() as usize * byte_depth;
+    let last_color = 2 * byte_depth;
+    for pixel in png.data.chunks(bpp) {
+        if byte_depth == 1 {
+            if pixel[0] != pixel[1] || pixel[1] != pixel[2] {
+                return None;
             }
+        } else if pixel[0..2] != pixel[2..4] || pixel[2..4] != pixel[4..6] {
+            return None;
         }
+        reduced.extend_from_slice(&pixel[last_color..]);
     }
 
     let transparency_pixel = if let Some(ref trns) = png.transparency_pixel {
@@ -275,7 +183,10 @@ pub fn reduce_rgb_to_grayscale(png: &PngImage) -> Option<PngImage> {
     Some(PngImage {
         data: reduced,
         ihdr: IhdrData {
-            color_type: ColorType::Grayscale,
+            color_type: match png.ihdr.color_type {
+                ColorType::RGBA => ColorType::GrayscaleAlpha,
+                _ => ColorType::Grayscale,
+            },
             ..png.ihdr
         },
         aux_headers,

src/reduction/mod.rs

@@ -34,29 +34,27 @@ pub fn reduced_palette(png: &PngImage, optimize_alpha: bool) -> Option<PngImage>
         let palette = png.palette.as_ref()?;
 
         // Find palette entries that are never used
-        for line in png.scan_lines() {
-            match png.ihdr.bit_depth {
-                BitDepth::Eight => {
-                    for &byte in line.data {
-                        used[byte as usize] = true;
-                    }
+        match png.ihdr.bit_depth {
+            BitDepth::Eight => {
+                for &byte in &png.data {
+                    used[byte as usize] = true;
                 }
-                BitDepth::Four => {
-                    for &byte in line.data {
-                        used[(byte & 0x0F) as usize] = true;
-                        used[(byte >> 4) as usize] = true;
-                    }
-                }
-                BitDepth::Two => {
-                    for &byte in line.data {
-                        used[(byte & 0x03) as usize] = true;
-                        used[((byte >> 2) & 0x03) as usize] = true;
-                        used[((byte >> 4) & 0x03) as usize] = true;
-                        used[(byte >> 6) as usize] = true;
-                    }
-                }
-                _ => unreachable!(),
             }
+            BitDepth::Four => {
+                for &byte in &png.data {
+                    used[(byte & 0x0F) as usize] = true;
+                    used[(byte >> 4) as usize] = true;
+                }
+            }
+            BitDepth::Two => {
+                for &byte in &png.data {
+                    used[(byte & 0x03) as usize] = true;
+                    used[((byte >> 2) & 0x03) as usize] = true;
+                    used[((byte >> 4) & 0x03) as usize] = true;
+                    used[(byte >> 6) as usize] = true;
+                }
+            }
+            _ => unreachable!(),
         }
 
         let mut used_enumerated: Vec<(usize, &bool)> = used.iter().enumerate().collect();
@@ -117,15 +115,9 @@ pub fn reduced_palette(png: &PngImage, optimize_alpha: bool) -> Option<PngImage>
 #[must_use]
 fn do_palette_reduction(png: &PngImage, palette_map: &[Option<u8>; 256]) -> Option<PngImage> {
     let byte_map = palette_map_to_byte_map(png, palette_map)?;
-    let mut raw_data = Vec::with_capacity(png.data.len());
 
     // Reassign data bytes to new indices
-    for line in png.scan_lines() {
-        raw_data.push(line.filter);
-        for byte in line.data {
-            raw_data.push(byte_map[*byte as usize]);
-        }
-    }
+    let raw_data = png.data.iter().map(|b| byte_map[*b as usize]).collect();
 
     let mut aux_headers = png.aux_headers.clone();
     if let Some(bkgd_header) = png.aux_headers.get(b"bKGD") {
@@ -207,48 +199,40 @@ pub fn reduce_color_type(
 
     // Go down one step at a time
     // Maybe not the most efficient, but it's safe
-    if reduced.ihdr.color_type == ColorType::RGBA {
-        if let Some(r) = if grayscale_reduction {
-            reduce_rgba_to_grayscale_alpha(&reduced)
-        } else {
-            None
-        }
-        .or_else(|| reduced_alpha_channel(&reduced, optimize_alpha))
-        {
+    if grayscale_reduction && matches!(reduced.ihdr.color_type, ColorType::RGBA | ColorType::RGB) {
+        if let Some(r) = reduce_rgb_to_grayscale(&reduced) {
             reduced = Cow::Owned(r);
-        } else if let Some(r) = reduce_to_palette(&reduced) {
-            reduced = Cow::Owned(r);
-            should_reduce_bit_depth = true;
+            should_reduce_bit_depth = reduced.ihdr.color_type == ColorType::Grayscale;
         }
     }
 
+    // Attempt grayscale alpha reduction before palette, as grayscale will typically be smaller than indexed
     if reduced.ihdr.color_type == ColorType::GrayscaleAlpha {
-        if let Some(r) =
-            reduced_alpha_channel(&reduced, optimize_alpha).or_else(|| reduce_to_palette(&reduced))
-        {
+        if let Some(r) = reduced_alpha_channel(&reduced, optimize_alpha) {
             reduced = Cow::Owned(r);
             should_reduce_bit_depth = true;
         }
     }
 
-    if reduced.ihdr.color_type == ColorType::RGB {
-        if let Some(r) = if grayscale_reduction {
-            reduce_rgb_to_grayscale(&reduced)
-        } else {
-            None
-        }
-        .or_else(|| reduce_to_palette(&reduced))
-        {
+    if matches!(
+        reduced.ihdr.color_type,
+        ColorType::RGBA | ColorType::RGB | ColorType::GrayscaleAlpha
+    ) {
+        if let Some(r) = reduce_to_palette(&reduced) {
             reduced = Cow::Owned(r);
             should_reduce_bit_depth = true;
+
+            // Make sure that palette gets sorted. Ideally, this should be done within reduce_to_palette.
+            if let Some(r) = reduced_palette(&reduced, optimize_alpha) {
+                reduced = Cow::Owned(r);
+            }
         }
     }
 
-    //Make sure that palette gets sorted. Ideally, this should be done within reduced_color_to_palette.
-    if should_reduce_bit_depth && reduced.ihdr.color_type == ColorType::Indexed {
-        if let Some(r) = reduced_palette(&reduced, optimize_alpha) {
+    // Attempt RGBA alpha reduction after palette, so it can be skipped if palette was successful
+    if reduced.ihdr.color_type == ColorType::RGBA {
+        if let Some(r) = reduced_alpha_channel(&reduced, optimize_alpha) {
             reduced = Cow::Owned(r);
-            should_reduce_bit_depth = true;
         }
     }
 

tests/reduction.rs

@@ -718,6 +718,78 @@ fn grayscale_8_should_be_grayscale_8() {
     );
 }
 
+#[test]
+fn grayscale_8_should_be_grayscale_4() {
+    test_it_converts(
+        "tests/files/grayscale_8_should_be_grayscale_4.png",
+        false,
+        ColorType::Grayscale,
+        BitDepth::Eight,
+        ColorType::Grayscale,
+        BitDepth::Four,
+    );
+}
+
+#[test]
+fn grayscale_8_should_be_grayscale_2() {
+    test_it_converts(
+        "tests/files/grayscale_8_should_be_grayscale_2.png",
+        false,
+        ColorType::Grayscale,
+        BitDepth::Eight,
+        ColorType::Grayscale,
+        BitDepth::Two,
+    );
+}
+
+#[test]
+fn grayscale_4_should_be_grayscale_2() {
+    test_it_converts(
+        "tests/files/grayscale_4_should_be_grayscale_2.png",
+        false,
+        ColorType::Grayscale,
+        BitDepth::Four,
+        ColorType::Grayscale,
+        BitDepth::Two,
+    );
+}
+
+#[test]
+fn grayscale_8_should_be_grayscale_1() {
+    test_it_converts(
+        "tests/files/grayscale_8_should_be_grayscale_1.png",
+        false,
+        ColorType::Grayscale,
+        BitDepth::Eight,
+        ColorType::Grayscale,
+        BitDepth::One,
+    );
+}
+
+#[test]
+fn grayscale_4_should_be_grayscale_1() {
+    test_it_converts(
+        "tests/files/grayscale_4_should_be_grayscale_1.png",
+        false,
+        ColorType::Grayscale,
+        BitDepth::Four,
+        ColorType::Grayscale,
+        BitDepth::One,
+    );
+}
+
+#[test]
+fn grayscale_2_should_be_grayscale_1() {
+    test_it_converts(
+        "tests/files/grayscale_2_should_be_grayscale_1.png",
+        false,
+        ColorType::Grayscale,
+        BitDepth::Two,
+        ColorType::Grayscale,
+        BitDepth::One,
+    );
+}
+
 #[test]
 fn grayscale_alpha_16_should_be_grayscale_trns_16() {
     test_it_converts(