diff --git a/.github/workflows/oxipng.yml b/.github/workflows/oxipng.yml index dfdb48bf..495efd22 100644 --- a/.github/workflows/oxipng.yml +++ b/.github/workflows/oxipng.yml @@ -78,7 +78,7 @@ jobs: token: ${{ secrets.GITHUB_TOKEN }} args: -- -D warnings - name: Run tests - run: cargo test + run: cargo test --features sanity-checks - name: Build benchmarks if: matrix.toolchain == 'nightly' run: cargo bench --no-run diff --git a/Cargo.toml b/Cargo.toml index 8f4a214d..443f81d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,10 @@ name = "oxipng" path = "src/main.rs" required-features = ["binary"] +[[bench]] +name = "zopfli" +required-features = ["zopfli"] + [dependencies] zopfli = { version = "0.7.1", optional = true } rgb = "0.8.33" @@ -53,6 +57,7 @@ optional = true version = "2.1.0" [dependencies.image] +optional = true default-features = false features = ["png"] version = "0.24.3" @@ -65,6 +70,7 @@ binary = ["clap", "wild", "stderrlog"] default = ["binary", "filetime", "parallel", "zopfli"] parallel = ["rayon", "indexmap/rayon", "crossbeam-channel"] freestanding = ["libdeflater/freestanding"] +sanity-checks = ["image"] [lib] name = "oxipng" diff --git a/src/lib.rs b/src/lib.rs index 614246ce..0f5b637f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,12 +31,11 @@ use crate::evaluate::Evaluator; use crate::png::PngData; use crate::png::PngImage; use crate::reduction::*; -use image::{DynamicImage, GenericImageView, ImageFormat, Pixel}; -use log::{debug, error, info, warn}; +use log::{debug, info, warn}; use rayon::prelude::*; use std::fmt; use std::fs::{copy, File, Metadata}; -use std::io::{stdin, stdout, BufWriter, Cursor, Read, Write}; +use std::io::{stdin, stdout, BufWriter, Read, Write}; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; @@ -659,7 +658,8 @@ fn optimize_png( ); } - debug_assert!(validate_output(&output, original_data)); + #[cfg(feature = "sanity-checks")] + debug_assert!(sanity_checks::validate_output(&output, original_data)); Ok(output) } @@ -1044,46 +1044,54 @@ fn copy_times(input_path_meta: &Metadata, out_path: &Path) -> PngResult<()> { }) } -/// Validate that the output png data still matches the original image -fn validate_output(output: &[u8], original_data: &[u8]) -> bool { - let (old_png, new_png) = rayon::join( - || load_png_image_from_memory(original_data), - || load_png_image_from_memory(output), - ); +#[cfg(feature = "sanity-checks")] +mod sanity_checks { + use super::*; + use image::{DynamicImage, GenericImageView, ImageFormat, Pixel}; + use log::error; + use std::io::Cursor; - match (new_png, old_png) { - (Err(new_err), _) => { - error!("Failed to read output image for validation: {}", new_err); - false + /// Validate that the output png data still matches the original image + pub(super) fn validate_output(output: &[u8], original_data: &[u8]) -> bool { + let (old_png, new_png) = rayon::join( + || load_png_image_from_memory(original_data), + || load_png_image_from_memory(output), + ); + + match (new_png, old_png) { + (Err(new_err), _) => { + error!("Failed to read output image for validation: {}", new_err); + false + } + (_, Err(old_err)) => { + // The original image might be invalid if, for example, there is a CRC error, + // and we set fix_errors to true. In that case, all we can do is check that the + // new image is decodable. + warn!("Failed to read input image for validation: {}", old_err); + true + } + (Ok(new_png), Ok(old_png)) => images_equal(&old_png, &new_png), } - (_, Err(old_err)) => { - // The original image might be invalid if, for example, there is a CRC error, - // and we set fix_errors to true. In that case, all we can do is check that the - // new image is decodable. - warn!("Failed to read input image for validation: {}", old_err); - true - } - (Ok(new_png), Ok(old_png)) => images_equal(&old_png, &new_png), + } + + /// Loads a PNG image from memory to a [DynamicImage] + fn load_png_image_from_memory(png_data: &[u8]) -> Result { + let mut reader = image::io::Reader::new(Cursor::new(png_data)); + reader.set_format(ImageFormat::Png); + reader.no_limits(); + reader.decode() + } + + /// Compares images pixel by pixel for equivalent content + fn images_equal(old_png: &DynamicImage, new_png: &DynamicImage) -> bool { + let a = old_png.pixels().filter(|x| { + let p = x.2.channels(); + !(p.len() == 4 && p[3] == 0) + }); + let b = new_png.pixels().filter(|x| { + let p = x.2.channels(); + !(p.len() == 4 && p[3] == 0) + }); + a.eq(b) } } - -/// Loads a PNG image from memory to a [DynamicImage] -fn load_png_image_from_memory(png_data: &[u8]) -> Result { - let mut reader = image::io::Reader::new(Cursor::new(png_data)); - reader.set_format(ImageFormat::Png); - reader.no_limits(); - reader.decode() -} - -/// Compares images pixel by pixel for equivalent content -fn images_equal(old_png: &DynamicImage, new_png: &DynamicImage) -> bool { - let a = old_png.pixels().filter(|x| { - let p = x.2.channels(); - !(p.len() == 4 && p[3] == 0) - }); - let b = new_png.pixels().filter(|x| { - let p = x.2.channels(); - !(p.len() == 4 && p[3] == 0) - }); - a.eq(b) -} diff --git a/src/rayon.rs b/src/rayon.rs index 768db04c..ae1914c2 100644 --- a/src/rayon.rs +++ b/src/rayon.rs @@ -52,6 +52,7 @@ where impl ParallelIterator for I {} +#[allow(dead_code)] pub fn join(a: impl FnOnce() -> A, b: impl FnOnce() -> B) -> (A, B) { (a(), b()) } diff --git a/tests/flags.rs b/tests/flags.rs index 38db85b7..fc7ace9e 100644 --- a/tests/flags.rs +++ b/tests/flags.rs @@ -27,6 +27,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) { } /// Add callback to allow checks before the output file is deleted again +#[allow(clippy::too_many_arguments)] fn test_it_converts_callbacks( input: PathBuf, output: &OutFile, @@ -38,8 +39,8 @@ fn test_it_converts_callbacks( mut callback_pre: CBPRE, mut callback_post: CBPOST, ) where - CBPOST: FnMut(&Path) -> (), - CBPRE: FnMut(&Path) -> (), + CBPOST: FnMut(&Path), + CBPRE: FnMut(&Path), { let png = PngData::new(&input, opts.fix_errors).unwrap(); @@ -48,14 +49,14 @@ fn test_it_converts_callbacks( callback_pre(&input); - match oxipng::optimize(&InFile::Path(input), &output, &opts) { + match oxipng::optimize(&InFile::Path(input), output, opts) { Ok(_) => (), Err(x) => panic!("{}", x), }; let output = output.path().unwrap(); assert!(output.exists()); - callback_post(&output); + callback_post(output); let png = match PngData::new(output, opts.fix_errors) { Ok(x) => x, diff --git a/tests/reduction.rs b/tests/reduction.rs index f3afdb88..d88cc58c 100644 --- a/tests/reduction.rs +++ b/tests/reduction.rs @@ -834,7 +834,7 @@ fn small_files() { let png = match PngData::new(output, opts.fix_errors) { Ok(x) => x, Err(x) => { - remove_file(&output).ok(); + remove_file(output).ok(); panic!("{}", x) } }; @@ -866,7 +866,7 @@ fn palette_should_be_reduced_with_dupes() { let png = match PngData::new(output, opts.fix_errors) { Ok(x) => x, Err(x) => { - remove_file(&output).ok(); + remove_file(output).ok(); panic!("{}", x) } }; @@ -899,7 +899,7 @@ fn palette_should_be_reduced_with_unused() { let png = match PngData::new(output, opts.fix_errors) { Ok(x) => x, Err(x) => { - remove_file(&output).ok(); + remove_file(output).ok(); panic!("{}", x) } }; @@ -932,7 +932,7 @@ fn palette_should_be_reduced_with_both() { let png = match PngData::new(output, opts.fix_errors) { Ok(x) => x, Err(x) => { - remove_file(&output).ok(); + remove_file(output).ok(); panic!("{}", x) } };