Make more use of helper functions

src/colors.rs

@@ -55,6 +55,16 @@ impl ColorType {
             ColorType::RGBA => 4,
         }
     }
+
+    #[inline]
+    pub fn is_rgb(&self) -> bool {
+        matches!(self, ColorType::RGB { .. } | ColorType::RGBA)
+    }
+
+    #[inline]
+    pub fn has_alpha(&self) -> bool {
+        matches!(self, ColorType::GrayscaleAlpha | ColorType::RGBA)
+    }
 }
 
 #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]

src/headers.rs

@@ -31,8 +31,8 @@ impl IhdrData {
     /// Bits per pixel
     #[must_use]
     #[inline]
-    pub fn bpp(&self) -> u8 {
+    pub fn bpp(&self) -> usize {
-        self.bit_depth.as_u8() * self.color_type.channels_per_pixel()
+        (self.bit_depth.as_u8() * self.color_type.channels_per_pixel()) as usize
     }
 
     /// Byte length of IDAT that is correct for this IHDR
@@ -42,8 +42,8 @@ impl IhdrData {
         let h = self.height as usize;
         let bpp = self.bpp();
 
-        fn bitmap_size(bpp: u8, w: usize, h: usize) -> usize {
+        fn bitmap_size(bpp: usize, w: usize, h: usize) -> usize {
-            (((w / 8) * bpp as usize) + ((w & 7) * bpp as usize + 7) / 8) * h
+            ((w * bpp + 7) / 8) * h
         }
 
         if self.interlaced == Interlacing::None {

src/interlace.rs

@@ -45,11 +45,11 @@ pub fn interlace_image(png: &PngImage) -> PngImage {
         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
-            if i >= (png.ihdr.width * u32::from(bits_per_pixel)) as usize {
+            if i >= (png.ihdr.width as usize * bits_per_pixel) {
                 break;
             }
             // Copy pixels into interlaced passes
-            let pix_modulo = (i / bits_per_pixel as usize) % 8;
+            let pix_modulo = (i / bits_per_pixel) % 8;
             match index % 8 {
                 0 => match pix_modulo {
                     0 => passes[0].push(bit),
@@ -113,7 +113,7 @@ 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 = bits_per_pixel as usize * png.ihdr.width as usize;
+    let bits_per_line = bits_per_pixel * 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];
@@ -126,16 +126,16 @@ fn deinterlace_bits(png: &PngImage) -> Vec<u8> {
             + u32::from(pass_constants.x_step)
             - 1)
             / u32::from(pass_constants.x_step)) as usize
-            * bits_per_pixel as usize;
+            * bits_per_pixel;
         for (i, bit) in bit_vec.iter().by_vals().enumerate() {
             // Avoid moving padded 0's into new image
             if i >= bits_in_line {
                 break;
             }
             let current_x: usize = pass_constants.x_shift as usize
-                + (i / bits_per_pixel as usize) * pass_constants.x_step as usize;
+                + (i / bits_per_pixel) * pass_constants.x_step as usize;
             // Copy this bit into the output line
-            let index = (i % bits_per_pixel as usize) + current_x * bits_per_pixel as usize;
+            let index = (i % bits_per_pixel) + current_x * bits_per_pixel;
             lines[current_y].set(index, bit);
         }
         // Calculate the next line and move to next pass if necessary
@@ -161,7 +161,7 @@ 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 = bytes_per_pixel as usize * png.ihdr.width as usize;
+    let bytes_per_line = bytes_per_pixel * 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;
@@ -170,9 +170,9 @@ fn deinterlace_bytes(png: &PngImage) -> Vec<u8> {
     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;
+                + (i / bytes_per_pixel) * pass_constants.x_step as usize;
             // Copy this byte into the output line
-            let index = (i % bytes_per_pixel as usize) + current_x * bytes_per_pixel as usize;
+            let index = (i % bytes_per_pixel) + current_x * bytes_per_pixel;
             lines[current_y][index] = *byte;
         }
         // Calculate the next line and move to next pass if necessary

src/png/mod.rs

@@ -1,4 +1,4 @@
-use crate::colors::ColorType;
+use crate::colors::{BitDepth, ColorType};
 use crate::deflate;
 use crate::error::PngError;
 use crate::filters::*;
@@ -246,8 +246,18 @@ impl PngImage {
 
     /// Return the number of channels in the image, based on color type
     #[inline]
-    pub fn channels_per_pixel(&self) -> u8 {
+    pub fn channels_per_pixel(&self) -> usize {
-        self.ihdr.color_type.channels_per_pixel()
+        self.ihdr.color_type.channels_per_pixel() as usize
+    }
+
+    /// Return the number of bytes per channel in the image
+    #[inline]
+    pub fn bytes_per_channel(&self) -> usize {
+        match self.ihdr.bit_depth {
+            BitDepth::Sixteen => 2,
+            // Depths lower than 8 will round up to 1 byte
+            _ => 1,
+        }
     }
 
     /// Return an iterator over the scanlines of the image
@@ -259,7 +269,7 @@ impl PngImage {
     /// Reverse all filters applied on the image, returning an unfiltered IDAT bytestream
     fn unfilter_image(&self) -> Result<Vec<u8>, PngError> {
         let mut unfiltered = Vec::with_capacity(self.data.len());
-        let bpp = ((self.ihdr.bit_depth.as_u8() * self.channels_per_pixel() + 7) / 8) as usize;
+        let bpp = self.bytes_per_channel() * self.channels_per_pixel();
         let mut last_line: Vec<u8> = Vec::new();
         let mut last_pass = None;
         let mut unfiltered_buf = Vec::new();
@@ -281,13 +291,12 @@ impl PngImage {
     /// Apply the specified filter type to all rows in the image
     pub fn filter_image(&self, filter: RowFilter, optimize_alpha: bool) -> Vec<u8> {
         let mut filtered = Vec::with_capacity(self.data.len());
-        let bpp = ((self.ihdr.bit_depth.as_u8() * self.channels_per_pixel() + 7) / 8) as usize;
+        let bpp = self.bytes_per_channel() * self.channels_per_pixel();
         // If alpha optimization is enabled, determine how many bytes of alpha there are per pixel
-        let alpha_bytes = match self.ihdr.color_type {
+        let alpha_bytes = if optimize_alpha && self.ihdr.color_type.has_alpha() {
-            ColorType::RGBA | ColorType::GrayscaleAlpha if optimize_alpha => {
+            self.bytes_per_channel()
-                (self.ihdr.bit_depth.as_u8() / 8) as usize
+        } else {
-            }
+            0
-            _ => 0,
         };
 
         let mut prev_line = Vec::new();

src/png/scan_lines.rs

@@ -43,7 +43,7 @@ impl<'a> Iterator for ScanLines<'a> {
 struct ScanLineRanges {
     /// Current pass number, and 0-indexed row within the pass
     pass: Option<(u8, u32)>,
-    bits_per_pixel: u8,
+    bits_per_pixel: usize,
     width: u32,
     height: u32,
     left: usize,
@@ -53,7 +53,7 @@ struct ScanLineRanges {
 impl ScanLineRanges {
     pub fn new(png: &PngImage, has_filter: bool) -> Self {
         Self {
-            bits_per_pixel: png.ihdr.bit_depth.as_u8() * png.channels_per_pixel(),
+            bits_per_pixel: png.ihdr.bpp(),
             width: png.ihdr.width,
             height: png.ihdr.height,
             left: png.data.len(),
@@ -143,8 +143,8 @@ impl Iterator for ScanLineRanges {
             // Standard, non-interlaced PNG scanlines
             (self.width, None)
         };
-        let bits_per_line = pixels_per_line * u32::from(self.bits_per_pixel);
+        let bits_per_line = pixels_per_line as usize * self.bits_per_pixel;
-        let mut len = ((bits_per_line + 7) / 8) as usize;
+        let mut len = (bits_per_line + 7) / 8;
         if self.has_filter {
             len += 1;
         }

src/reduction/alpha.rs

@@ -6,20 +6,16 @@ use crate::png::PngImage;
 
 /// Clean the alpha channel by setting the color of all fully transparent pixels to black
 pub fn cleaned_alpha_channel(png: &PngImage) -> Option<PngImage> {
-    let (bpc, bpp) = match png.ihdr.color_type {
+    if !png.ihdr.color_type.has_alpha() {
-        ColorType::RGBA | ColorType::GrayscaleAlpha => {
-            let cpp = png.channels_per_pixel();
-            let bpc = png.ihdr.bit_depth.as_u8() / 8;
-            (bpc as usize, (bpc * cpp) as usize)
-        }
-        _ => {
         return None;
     }
-    };
+    let byte_depth = png.bytes_per_channel();
+    let bpp = png.channels_per_pixel() * byte_depth;
+    let colored_bytes = bpp - byte_depth;
 
     let mut reduced = Vec::with_capacity(png.data.len());
     for pixel in png.data.chunks(bpp) {
-        if pixel.iter().skip(bpp - bpc).all(|b| *b == 0) {
+        if pixel.iter().skip(colored_bytes).all(|b| *b == 0) {
             reduced.resize(reduced.len() + bpp, 0);
         } else {
             reduced.extend_from_slice(pixel);
@@ -35,15 +31,11 @@ pub fn cleaned_alpha_channel(png: &PngImage) -> Option<PngImage> {
 
 #[must_use]
 pub fn reduced_alpha_channel(png: &PngImage, optimize_alpha: bool) -> Option<PngImage> {
-    if !matches!(
+    if !png.ihdr.color_type.has_alpha() {
-        png.ihdr.color_type,
-        ColorType::GrayscaleAlpha | ColorType::RGBA
-    ) {
         return None;
     }
-    let byte_depth = (png.ihdr.bit_depth.as_u8() >> 3) as usize;
+    let byte_depth = png.bytes_per_channel();
-    let channels = png.channels_per_pixel() as usize;
+    let bpp = png.channels_per_pixel() * byte_depth;
-    let bpp = channels * byte_depth;
     let colored_bytes = bpp - byte_depth;
 
     // If alpha optimisation is enabled, see if the image contains only fully opaque and fully transparent pixels.

src/reduction/bit_depth.rs

@@ -6,12 +6,10 @@ use crate::png::PngImage;
 #[must_use]
 pub fn reduce_bit_depth(png: &PngImage, minimum_bits: usize) -> Option<PngImage> {
     if png.ihdr.bit_depth != BitDepth::Sixteen {
-        return match png.ihdr.color_type {
+        if png.channels_per_pixel() == 1 {
-            ColorType::Indexed { .. } | ColorType::Grayscale { .. } => {
+            return reduce_bit_depth_8_or_less(png, minimum_bits);
-                reduce_bit_depth_8_or_less(png, minimum_bits)
         }
-            _ => None,
+        return None;
-        };
     }
 
     // Reduce from 16 to 8 bits per channel per pixel

src/reduction/color.rs

@@ -141,8 +141,8 @@ 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 = png.ihdr.bit_depth.as_u8() as usize >> 3;
+    let byte_depth = png.bytes_per_channel();
-    let bpp = png.channels_per_pixel() as usize * byte_depth;
+    let bpp = png.channels_per_pixel() * byte_depth;
     let last_color = 2 * byte_depth;
     for pixel in png.data.chunks(bpp) {
         if byte_depth == 1 {

src/reduction/mod.rs

@@ -189,27 +189,20 @@ fn reordered_palette(palette: &[RGBA8], palette_map: &[Option<u8>; 256]) -> Vec<
     new_palette
 }
 
-/// Attempt to reduce the color type of the image
+/// Attempt to reduce the color type of the image, returning the reduced image if successful
-/// Returns true if the color type was reduced, false otherwise
 pub fn reduce_color_type(
     png: &PngImage,
     grayscale_reduction: bool,
     optimize_alpha: bool,
 ) -> Option<PngImage> {
-    let mut should_reduce_bit_depth = false;
+    let was_single_channel = png.channels_per_pixel() == 1;
     let mut reduced = Cow::Borrowed(png);
 
-    // Go down one step at a time
+    // Go down one step at a time - maybe not the most efficient, but it's safe
-    // Maybe not the most efficient, but it's safe
+    // Attempt to reduce RGB to grayscale
-    if grayscale_reduction
+    if grayscale_reduction && reduced.ihdr.color_type.is_rgb() {
-        && matches!(
-            reduced.ihdr.color_type,
-            ColorType::RGBA | ColorType::RGB { .. }
-        )
-    {
         if let Some(r) = reduce_rgb_to_grayscale(&reduced) {
             reduced = Cow::Owned(r);
-            should_reduce_bit_depth = reduced.ihdr.color_type != ColorType::GrayscaleAlpha;
         }
     }
 
@@ -217,17 +210,13 @@ pub fn reduce_color_type(
     if reduced.ihdr.color_type == ColorType::GrayscaleAlpha {
         if let Some(r) = reduced_alpha_channel(&reduced, optimize_alpha) {
             reduced = Cow::Owned(r);
-            should_reduce_bit_depth = true;
         }
     }
 
-    if matches!(
+    // Attempt to reduce to palette, if not already a single channel
-        reduced.ihdr.color_type,
+    if reduced.channels_per_pixel() != 1 {
-        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) {
@@ -243,9 +232,8 @@ pub fn reduce_color_type(
         }
     }
 
-    if should_reduce_bit_depth {
+    // Some conversions will allow us to perform bit depth reduction that wasn't possible before
-        // Some conversions will allow us to perform bit depth reduction that
+    if !was_single_channel && reduced.channels_per_pixel() == 1 {
-        // wasn't possible before
         if let Some(r) = reduce_bit_depth_8_or_less(&reduced, 1) {
             reduced = Cow::Owned(r);
         }