diff --git a/CHANGELOG.md b/CHANGELOG.md index dfac8ec8..31ae6ae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ **Version 0.4.0 (unreleased)** - Performance optimizations - [SEMVER_MAJOR] `-s` automatically infers `--strip safe` + - Update byteorder and clap crates + - Fix issue where interlaced images incorrectly applied filters on the first line of a pass **Version 0.3.0** - Properly decode interlaced images diff --git a/Cargo.toml b/Cargo.toml index c70a3089..50e1baf4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,14 +22,14 @@ doc = false [dependencies] bit-vec = "^0.4.2" -byteorder = "^0.4.0" -clap = "^1.5.4" -crc = "^1.0.0" +byteorder = "^0.5.0" +clap = "^2.2.5" +crc = "^1.2.0" libc = "^0.2.4" libz-sys = "^1.0.0" num_cpus = "^0.2.11" -regex = "^0.1.8" +regex = "^0.1.63" scoped-pool = "^0.1.8" [dev-dependencies] -image = "^0.6.1" +image = "^0.8.0" diff --git a/src/main.rs b/src/main.rs index 90e241bb..02cab19c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -239,7 +239,6 @@ fn main() { handle_optimization(matches.values_of("files") .unwrap() - .iter() .map(PathBuf::from) .collect(), opts); diff --git a/src/png.rs b/src/png.rs index 3c7b3b1a..770a0bbb 100644 --- a/src/png.rs +++ b/src/png.rs @@ -197,6 +197,11 @@ impl<'a> Iterator for ScanLines<'a> { bits_per_line -= gap; } } + let current_pass = if let Some(pass) = self.pass { + Some(pass.0) + } else { + None + }; let bytes_per_line = (bits_per_line as f32 / 8f32).ceil() as usize; self.start = self.end; self.end = self.start + bytes_per_line + 1; @@ -216,6 +221,7 @@ impl<'a> Iterator for ScanLines<'a> { Some(ScanLine { filter: self.png.raw_data[self.start], data: self.png.raw_data[(self.start + 1)..self.end].to_owned(), + pass: current_pass, }) } else { // Standard, non-interlaced PNG scanlines @@ -228,6 +234,7 @@ impl<'a> Iterator for ScanLines<'a> { Some(ScanLine { filter: self.png.raw_data[self.start], data: self.png.raw_data[(self.start + 1)..self.end].to_owned(), + pass: None, }) } } @@ -240,6 +247,8 @@ pub struct ScanLine { pub filter: u8, /// The byte data for the current scan line, encoded with the filter specified in the `filter` field pub data: Vec, + /// The current pass if the image is interlaced + pub pass: Option, } #[derive(Debug,Clone)] @@ -456,34 +465,42 @@ impl PngData { 8f32) .ceil() as usize; let mut last_line: Vec = Vec::new(); + let mut last_pass: Option = None; for line in self.scan_lines() { - match filter { - 0 | 1 | 2 | 3 | 4 => { - filtered.push(filter); - filtered.extend(filter_line(filter, bpp, &line.data, &last_line)); - } - 5 => { - // Heuristically guess best filter per line - // Uses MSAD algorithm mentioned in libpng reference docs - // http://www.libpng.org/pub/png/book/chapter09.html - let mut trials: HashMap> = HashMap::with_capacity(5); - for filter in 0..5 { - trials.insert(filter, filter_line(filter, bpp, &line.data, &last_line)); + if last_pass == line.pass { + match filter { + 0 | 1 | 2 | 3 | 4 => { + filtered.push(filter); + filtered.extend(filter_line(filter, bpp, &line.data, &last_line)); } - let (best_filter, best_line) = trials.iter() - .min_by_key(|x| { - x.1.iter().fold(0u64, |acc, &x| { - let signed = x as i8; - acc + (signed as i16).abs() as u64 + 5 => { + // Heuristically guess best filter per line + // Uses MSAD algorithm mentioned in libpng reference docs + // http://www.libpng.org/pub/png/book/chapter09.html + let mut trials: HashMap> = HashMap::with_capacity(5); + for filter in 0..5 { + trials.insert(filter, filter_line(filter, bpp, &line.data, &last_line)); + } + let (best_filter, best_line) = trials.iter() + .min_by_key(|x| { + x.1.iter().fold(0u64, |acc, &x| { + let signed = x as i8; + acc + + (signed as i16).abs() as u64 + }) }) - }) - .unwrap(); - filtered.push(*best_filter); - filtered.extend_from_slice(best_line); + .unwrap(); + filtered.push(*best_filter); + filtered.extend_from_slice(best_line); + } + _ => unreachable!(), } - _ => unreachable!(), + } else { + filtered.push(0); + filtered.extend(filter_line(0, bpp, &line.data, &last_line)); } last_line = line.data.clone(); + last_pass = line.pass; } filtered } diff --git a/tests/files/interlaced_0_to_1_other_filter_mode.png b/tests/files/interlaced_0_to_1_other_filter_mode.png new file mode 100644 index 00000000..b79a99ee Binary files /dev/null and b/tests/files/interlaced_0_to_1_other_filter_mode.png differ diff --git a/tests/flags.rs b/tests/flags.rs index 3666809d..9468e80c 100644 --- a/tests/flags.rs +++ b/tests/flags.rs @@ -423,3 +423,43 @@ fn interlacing_1_to_0_small_files() { remove_file(output).ok(); } + +#[test] +fn interlaced_0_to_1_other_filter_mode() { + let input = PathBuf::from("tests/files/interlaced_0_to_1_other_filter_mode.png"); + let mut opts = get_opts(&input); + opts.interlace = Some(1); + let mut filter = HashSet::new(); + filter.insert(4); + opts.filter = filter; + let output = opts.out_file.clone(); + + let png = png::PngData::new(&input).unwrap(); + + assert!(png.ihdr_data.interlaced == 0); + + match oxipng::optimize(&input, &opts) { + Ok(_) => (), + Err(x) => panic!(x.to_owned()), + }; + assert!(output.exists()); + + let png = match png::PngData::new(&output) { + Ok(x) => x, + Err(x) => { + remove_file(output).ok(); + panic!(x.to_owned()) + } + }; + + assert!(png.ihdr_data.interlaced == 1); + + let old_png = image::open(&input).unwrap(); + let new_png = image::open(&output).unwrap(); + + // 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(); +}