Fix many issues

This commit is contained in:
Joshua Holmer 2016-02-18 16:23:16 -05:00
parent 60cf7c96e7
commit 5c0bbbe2bb
3 changed files with 60 additions and 46 deletions

View file

@ -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<u8>)> = 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(())

View file

@ -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<Self::Item> {
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<u8> {
@ -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<Vec<u8>> {
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<Vec<u8>> {
} 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<Vec<u8>> {
}
reduced.push(low_bytes[0]);
low_bytes.clear();
reduced.extend_from_slice(&trans_bytes);
trans_bytes.clear();
}
}
}

View file

@ -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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
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::<Vec<Vec<u8>>>() == new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
remove_file(output).ok();
}
#[test]