diff --git a/src/lib.rs b/src/lib.rs index bc47ceb6..d81c1d16 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,7 +14,6 @@ use std::io::BufWriter; use std::io::Write; use std::path::Path; use std::path::PathBuf; -use std::sync::Arc; pub mod deflate { pub mod deflate; @@ -104,7 +103,7 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { png.ihdr_data.bit_depth, palette.len() / 3); } else { - println!("Reducing image to {}x{} bits/pixel, {:?}", + println!("Reducing image to {}x{} bits/pixel, {}", png.channels_per_pixel(), png.ihdr_data.bit_depth, png.ihdr_data.color_type); @@ -119,10 +118,10 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { if opts.idat_recoding || something_changed { // Go through selected permutations and determine the best - let mut best: Option<(u8, u8, u8, u8)> = None; - let scoped_idat = Arc::new(png.idat_data.clone()); + let mut best: Option<(u8, u8, u8, u8, Vec)> = None; let combinations = opts.filter.len() * opts.compression.len() * opts.memory.len() * opts.strategies.len(); + let mut results = Vec::with_capacity(combinations); println!("Trying: {} combinations", combinations); crossbeam::scope(|scope| { for f in &opts.filter { @@ -130,10 +129,8 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { for zc in &opts.compression { for zm in &opts.memory { for zs in &opts.strategies { - let moved_png = png.clone(); - let mut moved_idat = scoped_idat.clone(); let moved_filtered = filtered.clone(); - scope.spawn(move || { + results.push(scope.spawn(move || { let new_idat = match deflate::deflate::deflate(&moved_filtered, *zc, *zm, @@ -142,11 +139,7 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { Ok(x) => x, Err(x) => return Err(x), }; - if new_idat.len() < moved_png.idat_data.len() || - (best.is_none() && something_changed) { - best = Some((*f, *zc, *zm, *zs)); - *Arc::make_mut(&mut moved_idat) = new_idat.clone(); - } + if opts.verbosity == Some(1) { println!(" zc = {} zm = {} zs = {} f = {} {} bytes", *zc, @@ -155,16 +148,26 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { *f, new_idat.len()); } - Ok(()) - }); + + Ok((*f, *zc, *zm, *zs, new_idat.clone())) + })); } } } } }); + for result in results { + if let Ok(ok_result) = result.join() { + if ok_result.4.len() < png.idat_data.len() || + (best.is_none() && something_changed) { + best = Some(ok_result); + } + } + } + if let Some(better) = best { - png.idat_data = Arc::try_unwrap(scoped_idat).unwrap().clone(); + png.idat_data = better.4.clone(); println!("Found better combination:"); println!(" zc = {} zm = {} zs = {} f = {} {} bytes", better.1, @@ -231,7 +234,7 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { png.idat_data.len(), idat_original_size - png.idat_data.len()); } else { - println!(" IDAT size = {} bytes ({} bytes increate)", + println!(" IDAT size = {} bytes ({} bytes increase)", png.idat_data.len(), png.idat_data.len() - idat_original_size); } @@ -239,12 +242,12 @@ pub fn optimize(filepath: &Path, opts: &Options) -> Result<(), String> { println!(" file size = {} bytes ({} bytes = {:.2}% decrease)", output_data.len(), file_original_size - output_data.len(), - (file_original_size - output_data.len()) as f64 / file_original_size as f64); + (file_original_size - output_data.len()) as f64 / file_original_size as f64 * 100f64); } else { println!(" file size = {} bytes ({} bytes = {:.2}% increase)", output_data.len(), output_data.len() - file_original_size, - (output_data.len() - file_original_size) as f64 / file_original_size as f64); + (output_data.len() - file_original_size) as f64 / file_original_size as f64 * 100f64); } Ok(()) diff --git a/src/png.rs b/src/png.rs index 7215fe40..aaccee69 100644 --- a/src/png.rs +++ b/src/png.rs @@ -94,25 +94,30 @@ struct ScanLines<'a> { png: &'a PngData, start: usize, end: usize, - len: usize, +} + +impl<'a> ScanLines<'a> { + fn len(&mut self) -> usize { + self.png.raw_data.len() + } } impl<'a> Iterator for ScanLines<'a> { type Item = ScanLine; fn next(&mut self) -> Option { - if self.end == self.len { + if self.end == self.len() { None } else { let bits_per_line = self.png.ihdr_data.width as usize * self.png.ihdr_data.bit_depth.as_u8() as usize * self.png.channels_per_pixel() as usize; // This avoids casting to and from floats, which is expensive - let bytes_per_line = ((bits_per_line + bits_per_line % 8) >> 3) + 1; + let bytes_per_line = (bits_per_line + bits_per_line % 8) >> 3; self.start = self.end; - self.end = self.start + bytes_per_line; + self.end = self.start + bytes_per_line + 1; Some(ScanLine { filter: self.png.raw_data[self.start], - data: self.png.raw_data[self.start + 1..self.end].to_owned(), + data: self.png.raw_data[(self.start + 1)..self.end].to_owned(), }) } } @@ -324,7 +329,6 @@ impl PngData { png: &self, start: 0, end: 0, - len: self.raw_data.len(), } } pub fn unfilter_image(&self) -> Vec { @@ -658,6 +662,7 @@ impl PngData { if self.ihdr_data.color_type == ColorType::GrayscaleAlpha { if let Some(data) = reduce_grayscale_alpha_to_grayscale(self) { self.raw_data = data; + self.ihdr_data.color_type = ColorType::Grayscale; changed = true; should_reduce_bit_depth = true; } @@ -744,6 +749,7 @@ fn reduce_rgba_to_grayscale_alpha(png: &PngData) -> Option> { 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 % bpp < (bpp - byte_depth as usize) { if byte_depth == 1 || i % 2 == 1 { @@ -751,13 +757,16 @@ fn reduce_rgba_to_grayscale_alpha(png: &PngData) -> Option> { } else { high_bytes.push(*byte); } - } else if i % bpp == bpp - 1 { + } else { + trans_bytes.push(*byte); + } + + if i % bpp == bpp - 1 { low_bytes.sort(); low_bytes.dedup(); if low_bytes.len() > 1 { return None; } - // FIXME: Ugly, is there a better way of making this dynamic for 16-bit content? if byte_depth == 2 { high_bytes.sort(); high_bytes.dedup(); @@ -769,6 +778,8 @@ fn reduce_rgba_to_grayscale_alpha(png: &PngData) -> Option> { } reduced.push(low_bytes[0]); low_bytes.clear(); + reduced.extend_from_slice(&trans_bytes); + trans_bytes.clear(); } } } diff --git a/tests/oxipng.rs b/tests/oxipng.rs index 766aca53..87c1c9d5 100644 --- a/tests/oxipng.rs +++ b/tests/oxipng.rs @@ -75,10 +75,10 @@ fn rgba_should_be_rgba() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -107,10 +107,10 @@ fn rgba_should_be_rgb() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -139,10 +139,10 @@ fn rgba_should_be_palette() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -171,10 +171,10 @@ fn rgba_should_be_grayscale_alpha() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -203,10 +203,10 @@ fn rgba_should_be_grayscale() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -235,10 +235,10 @@ fn rgb_should_be_rgb() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -267,10 +267,10 @@ fn rgb_should_be_palette() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -299,10 +299,10 @@ fn rgb_should_be_grayscale() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -331,10 +331,10 @@ fn palette_should_be_palette() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test] @@ -363,10 +363,10 @@ fn palette_should_be_grayscale() { let old_png = image::open(&input).unwrap(); let new_png = image::open(&output).unwrap(); - remove_file(output).ok(); - // Conversion should be lossless assert!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::>>()); + + remove_file(output).ok(); } #[test]