Refactor reduction evaluation sequence

benches/reductions.rs

@@ -140,7 +140,7 @@ 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, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, false));
 }
 
 #[bench]
@@ -148,7 +148,7 @@ 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, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| alpha::reduced_alpha_channel(&png.raw, false));
 }
 
 #[bench]
@@ -158,7 +158,7 @@ fn reductions_rgba_to_grayscale_alpha_16(b: &mut Bencher) {
     ));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| color::reduce_rgb_to_grayscale(&png.raw));
 }
 
 #[bench]
@@ -168,7 +168,7 @@ fn reductions_rgba_to_grayscale_alpha_8(b: &mut Bencher) {
     ));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| color::reduce_rgb_to_grayscale(&png.raw));
 }
 
 #[bench]
@@ -178,7 +178,10 @@ fn reductions_rgba_to_grayscale_16(b: &mut Bencher) {
     ));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| {
+        color::reduce_rgb_to_grayscale(&png.raw)
+            .and_then(|r| alpha::reduced_alpha_channel(&r, false))
+    });
 }
 
 #[bench]
@@ -188,7 +191,10 @@ fn reductions_rgba_to_grayscale_8(b: &mut Bencher) {
     ));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| {
+        color::reduce_rgb_to_grayscale(&png.raw)
+            .and_then(|r| alpha::reduced_alpha_channel(&r, false))
+    });
 }
 
 #[bench]
@@ -198,7 +204,7 @@ fn reductions_rgb_to_grayscale_16(b: &mut Bencher) {
     ));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| color::reduce_rgb_to_grayscale(&png.raw));
 }
 
 #[bench]
@@ -206,7 +212,7 @@ fn reductions_rgb_to_grayscale_8(b: &mut Bencher) {
     let input = test::black_box(PathBuf::from("tests/files/rgb_8_should_be_grayscale_8.png"));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| color::reduce_rgb_to_grayscale(&png.raw));
 }
 
 #[bench]
@@ -214,7 +220,7 @@ fn reductions_rgba_to_palette_8(b: &mut Bencher) {
     let input = test::black_box(PathBuf::from("tests/files/rgba_8_should_be_palette_8.png"));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| color::reduce_to_palette(&png.raw));
 }
 
 #[bench]
@@ -222,7 +228,7 @@ fn reductions_rgb_to_palette_8(b: &mut Bencher) {
     let input = test::black_box(PathBuf::from("tests/files/rgb_8_should_be_palette_8.png"));
     let png = PngData::new(&input, false).unwrap();
 
-    b.iter(|| reduce_color_type(&png.raw, true, false));
+    b.iter(|| color::reduce_to_palette(&png.raw));
 }
 
 #[bench]

src/colors.rs

@@ -74,6 +74,15 @@ impl ColorType {
     pub fn has_alpha(&self) -> bool {
         matches!(self, ColorType::GrayscaleAlpha | ColorType::RGBA)
     }
+
+    #[inline]
+    pub fn has_trns(&self) -> bool {
+        match self {
+            ColorType::Grayscale { transparent_shade } => transparent_shade.is_some(),
+            ColorType::RGB { transparent_color } => transparent_color.is_some(),
+            _ => false,
+        }
+    }
 }
 
 #[repr(u8)]

src/lib.rs

@@ -25,7 +25,6 @@ extern crate rayon;
 mod rayon;
 
 use crate::atomicmin::AtomicMin;
-use crate::colors::BitDepth;
 use crate::deflate::{crc32, inflate};
 use crate::evaluate::Evaluator;
 use crate::png::PngData;
@@ -477,23 +476,6 @@ fn optimize_png(
     perform_strip(png, opts);
     let stripped_png = png.clone();
 
-    // Interlacing is not part of the evaluator trials but must be done first to evaluate the rest correctly
-    let mut reduction_occurred = false;
-    if let Some(interlacing) = opts.interlace {
-        if let Some(reduced) = png.raw.change_interlacing(interlacing) {
-            png.raw = Arc::new(reduced);
-            reduction_occurred = true;
-        }
-    }
-
-    // If alpha optimization is enabled, perform a black alpha reduction before evaluating reductions
-    // This can allow reductions from alpha to indexed which may not have been possible otherwise
-    if opts.optimize_alpha {
-        if let Some(reduced) = cleaned_alpha_channel(&png.raw) {
-            png.raw = Arc::new(reduced);
-        }
-    }
-
     // Must use normal (lazy) compression, as faster ones (greedy) are not representative
     let eval_compression = 5;
     // None and Bigrams work well together, especially for alpha reductions
@@ -505,7 +487,9 @@ fn optimize_png(
         eval_compression,
         false,
     );
-    perform_reductions(png.raw.clone(), opts, &deadline, &eval);
+    let (baseline, mut reduction_occurred) =
+        perform_reductions(png.raw.clone(), opts, &deadline, &eval);
+    png.raw = baseline;
     let mut eval_filter = if let Some(result) = eval.get_best_candidate() {
         *png = result.image;
         if result.is_reduction {
@@ -664,74 +648,6 @@ fn optimize_png(
     Ok(output)
 }
 
-fn perform_reductions(
-    mut png: Arc<PngImage>,
-    opts: &Options,
-    deadline: &Deadline,
-    eval: &Evaluator,
-) {
-    // The eval baseline will be set from the original png only if we attempt any reductions
-    let baseline = png.clone();
-    let mut reduction_occurred = false;
-
-    if opts.palette_reduction {
-        if let Some(reduced) = reduced_palette(&png, opts.optimize_alpha) {
-            png = Arc::new(reduced);
-            eval.try_image(png.clone());
-            reduction_occurred = true;
-        }
-        if deadline.passed() {
-            return;
-        }
-    }
-
-    if opts.bit_depth_reduction {
-        if let Some(reduced) = reduced_bit_depth_16_to_8(&png) {
-            png = Arc::new(reduced);
-            eval.try_image(png.clone());
-            reduction_occurred = true;
-        }
-        if deadline.passed() {
-            return;
-        }
-    }
-
-    if opts.color_type_reduction {
-        if let Some(reduced) =
-            reduce_color_type(&png, opts.grayscale_reduction, opts.optimize_alpha)
-        {
-            png = Arc::new(reduced);
-            eval.try_image(png.clone());
-            reduction_occurred = true;
-        }
-        if deadline.passed() {
-            return;
-        }
-    }
-
-    if opts.bit_depth_reduction {
-        if let Some(reduced) = reduced_bit_depth_8_or_less(&png, 1) {
-            let previous = png;
-            png = Arc::new(reduced);
-            eval.try_image(png.clone());
-            if png.ihdr.bit_depth < BitDepth::Four && previous.ihdr.bit_depth == BitDepth::Eight {
-                // Also try 16-color mode for all lower bits images, since that may compress better
-                if let Some(reduced) = reduced_bit_depth_8_or_less(&previous, 4) {
-                    eval.try_image(Arc::new(reduced));
-                }
-            }
-            reduction_occurred = true;
-        }
-        if deadline.passed() {
-            return;
-        }
-    }
-
-    if reduction_occurred {
-        eval.set_baseline(baseline);
-    }
-}
-
 /// Execute a compression trial
 fn perform_trial(
     filtered: &[u8],

src/reduction/color.rs

@@ -35,7 +35,7 @@ where
 
 #[must_use]
 pub fn reduce_to_palette(png: &PngImage) -> Option<PngImage> {
-    if png.ihdr.bit_depth != BitDepth::Eight {
+    if png.ihdr.bit_depth != BitDepth::Eight || png.channels_per_pixel() == 1 {
         return None;
     }
     let mut raw_data = Vec::with_capacity(png.data.len());
@@ -154,6 +154,10 @@ pub fn reduce_to_palette(png: &PngImage) -> Option<PngImage> {
 
 #[must_use]
 pub fn reduce_rgb_to_grayscale(png: &PngImage) -> Option<PngImage> {
+    if !png.ihdr.color_type.is_rgb() {
+        return None;
+    }
+
     let mut reduced = Vec::with_capacity(png.data.len());
     let byte_depth = png.bytes_per_channel();
     let bpp = png.channels_per_pixel() * byte_depth;

src/reduction/mod.rs

@@ -1,20 +1,20 @@
 use crate::colors::{BitDepth, ColorType};
+use crate::evaluate::Evaluator;
 use crate::headers::IhdrData;
 use crate::png::PngImage;
+use crate::Deadline;
+use crate::Options;
 use indexmap::map::{Entry::*, IndexMap};
 use rgb::RGBA8;
-use std::borrow::Cow;
+use std::sync::Arc;
 
 pub mod alpha;
-use crate::alpha::reduced_alpha_channel;
+use crate::alpha::*;
 pub mod bit_depth;
+use crate::bit_depth::*;
 pub mod color;
 use crate::color::*;
 
-pub(crate) use crate::alpha::cleaned_alpha_channel;
-pub(crate) use crate::bit_depth::reduced_bit_depth_16_to_8;
-pub(crate) use crate::bit_depth::reduced_bit_depth_8_or_less;
-
 /// Attempt to reduce the number of colors in the palette
 /// Returns `None` if palette hasn't changed
 pub fn reduced_palette(png: &PngImage, optimize_alpha: bool) -> Option<PngImage> {
@@ -190,50 +190,103 @@ fn reordered_palette(palette: &[RGBA8], palette_map: &[Option<u8>; 256]) -> Vec<
     new_palette
 }
 
-/// Attempt to reduce the color type of the image, returning the reduced image if successful
+pub(crate) fn perform_reductions(
-pub fn reduce_color_type(
+    mut png: Arc<PngImage>,
-    png: &PngImage,
+    opts: &Options,
-    grayscale_reduction: bool,
+    deadline: &Deadline,
-    optimize_alpha: bool,
+    eval: &Evaluator,
-) -> Option<PngImage> {
+) -> (Arc<PngImage>, bool) {
-    let mut reduced = Cow::Borrowed(png);
+    let mut reduction_occurred = false;
+    let mut evaluation_added = false;
+
+    // Interlacing must be processed first in order to evaluate the rest correctly
+    if let Some(interlacing) = opts.interlace {
+        if let Some(reduced) = png.change_interlacing(interlacing) {
+            png = Arc::new(reduced);
+            reduction_occurred = true;
+        }
+    }
+
+    // If alpha optimization is enabled, clean the alpha channel before continuing
+    // This can allow some color type reductions which may not have been possible otherwise
+    if opts.optimize_alpha && !deadline.passed() {
+        if let Some(reduced) = cleaned_alpha_channel(&png) {
+            png = Arc::new(reduced);
+            // This does not count as a reduction
+        }
+    }
+
+    // Attempt to reduce 16-bit to 8-bit
+    // This is just removal of bytes and does not need to be evaluated
+    if opts.bit_depth_reduction && !deadline.passed() {
+        if let Some(reduced) = reduced_bit_depth_16_to_8(&png) {
+            png = Arc::new(reduced);
+            reduction_occurred = true;
+        }
+    }
 
-    // Go down one step at a time - maybe not the most efficient, but it's safe
     // Attempt to reduce RGB to grayscale
-    if grayscale_reduction && reduced.ihdr.color_type.is_rgb() {
+    // This is just removal of bytes and does not need to be evaluated
-        if let Some(r) = reduce_rgb_to_grayscale(&reduced) {
+    if opts.color_type_reduction && !deadline.passed() {
-            reduced = Cow::Owned(r);
+        if let Some(reduced) = reduce_rgb_to_grayscale(&png) {
+            png = Arc::new(reduced);
+            reduction_occurred = true;
         }
     }
 
-    // Attempt grayscale alpha reduction before palette, as grayscale will typically be smaller than indexed
+    // Now retain the current png for the evaluator baseline
-    if reduced.ihdr.color_type == ColorType::GrayscaleAlpha {
+    // It will only be entered into the evaluator if there are also others to evaluate
-        if let Some(r) = reduced_alpha_channel(&reduced, optimize_alpha) {
+    let mut baseline = png.clone();
-            reduced = Cow::Owned(r);
-        }
-    }
 
-    // Attempt to reduce to palette, if not already a single channel
+    // Attempt alpha removal
-    if reduced.channels_per_pixel() != 1 {
+    if opts.color_type_reduction && !deadline.passed() {
-        if let Some(r) = reduce_to_palette(&reduced) {
+        if let Some(reduced) = reduced_alpha_channel(&png, opts.optimize_alpha) {
-            reduced = Cow::Owned(r);
+            png = Arc::new(reduced);
-
+            // If the reduction requires a tRNS chunk, enter this into the evaluator
-            // Make sure that palette gets sorted. Ideally, this should be done within reduce_to_palette.
+            // Otherwise it is just removal of bytes and should become the baseline
-            if let Some(r) = reduced_palette(&reduced, optimize_alpha) {
+            if png.ihdr.color_type.has_trns() {
-                reduced = Cow::Owned(r);
+                eval.try_image(png.clone());
+                evaluation_added = true;
+            } else {
+                baseline = png.clone();
+                reduction_occurred = true;
             }
         }
     }
 
-    // Attempt RGBA alpha reduction after palette, so it can be skipped if palette was successful
+    // Attempt to reduce the palette size
-    if reduced.ihdr.color_type == ColorType::RGBA {
+    if opts.palette_reduction && !deadline.passed() {
-        if let Some(r) = reduced_alpha_channel(&reduced, optimize_alpha) {
+        if let Some(reduced) = reduced_palette(&png, opts.optimize_alpha) {
-            reduced = Cow::Owned(r);
+            png = Arc::new(reduced);
+            eval.try_image(png.clone());
+            evaluation_added = true;
         }
     }
 
-    match reduced {
+    // Attempt to reduce to palette
-        Cow::Owned(r) => Some(r),
+    if opts.color_type_reduction && !deadline.passed() {
-        _ => None,
+        if let Some(reduced) = reduce_to_palette(&png) {
+            png = Arc::new(reduced);
+            // Make sure the palette gets sorted (ideally, this should be done within reduce_to_palette)
+            if let Some(reduced) = reduced_palette(&png, opts.optimize_alpha) {
+                png = Arc::new(reduced);
+            }
+            eval.try_image(png.clone());
+            evaluation_added = true;
+        }
     }
+
+    // Attempt to reduce to a lower bit depth
+    if opts.bit_depth_reduction && !deadline.passed() {
+        if let Some(reduced) = reduced_bit_depth_8_or_less(&png, 1) {
+            png = Arc::new(reduced);
+            eval.try_image(png.clone());
+            evaluation_added = true;
+        }
+    }
+
+    if evaluation_added {
+        eval.set_baseline(baseline.clone());
+    }
+    (baseline, reduction_occurred)
 }

tests/regression.rs

@@ -171,8 +171,8 @@ fn issue_52_04() {
         None,
         RGBA,
         BitDepth::Eight,
-        INDEXED,
+        RGB,
-        BitDepth::One,
+        BitDepth::Eight,
     );
 }
 
@@ -243,7 +243,7 @@ fn issue_60() {
         None,
         RGBA,
         BitDepth::Eight,
-        RGBA,
+        GRAYSCALE_ALPHA,
         BitDepth::Eight,
     );
 }