Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance consistency #202

Merged
merged 5 commits into from
Apr 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ num_cpus = "^1.0.0"
zopfli = "^0.4.0"
miniz_oxide = "0.3"
rgb = "0.8.11"
indexmap = { version = "1.3.2", features = ["rayon"] }
libdeflater = "0.2.0"

[dependencies.rayon]
Expand Down
4 changes: 2 additions & 2 deletions src/headers.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use indexmap::IndexSet;
use crate::colors::{BitDepth, ColorType};
use crate::error::PngError;
use crate::PngResult;
use byteorder::{BigEndian, ReadBytesExt};
use crc::crc32;
use std::collections::HashSet;
use std::io::Cursor;

#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -35,7 +35,7 @@ pub enum Headers {
/// Headers that won't affect rendering (all but cHRM, gAMA, iCCP, sBIT, sRGB, bKGD, hIST, pHYs, sPLT)
Safe,
/// Remove all non-critical chunks except these
Keep(HashSet<String>),
Keep(IndexSet<String>),
/// All non-critical headers
All,
}
Expand Down
45 changes: 21 additions & 24 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use crate::png::PngImage;
use crate::reduction::*;
use crc::crc32;
use image::{DynamicImage, GenericImageView, ImageFormat, Pixel};
use indexmap::{IndexSet, IndexMap};
use rayon::prelude::*;
use std::collections::{BTreeMap, HashMap, HashSet};
use std::fmt;
use std::fs::{copy, File};
use std::io::{stdin, stdout, BufWriter, Read, Write};
Expand Down Expand Up @@ -154,7 +154,7 @@ pub struct Options {
/// Which filters to try on the file (0-5)
///
/// Default: `0,5`
pub filter: HashSet<u8>,
pub filter: IndexSet<u8>,
/// Whether to change the interlacing type of the file.
///
/// `None` will not change the current interlacing type.
Expand All @@ -166,11 +166,11 @@ pub struct Options {
/// Which zlib compression levels to try on the file (1-9)
///
/// Default: `9`
pub compression: HashSet<u8>,
pub compression: IndexSet<u8>,
/// Which zlib compression strategies to try on the file (0-3)
///
/// Default: `0-3`
pub strategies: HashSet<u8>,
pub strategies: IndexSet<u8>,
/// Window size to use when compressing the file, as `2^window` bytes.
///
/// Doesn't affect compression but may affect speed and memory usage.
Expand All @@ -179,7 +179,7 @@ pub struct Options {
/// Default: `15`
pub window: u8,
/// Alpha filtering strategies to use
pub alphas: HashSet<colors::AlphaOptim>,
pub alphas: IndexSet<colors::AlphaOptim>,
/// Whether to attempt bit depth reduction
///
/// Default: `true`
Expand Down Expand Up @@ -286,17 +286,17 @@ impl Options {
impl Default for Options {
fn default() -> Options {
// Default settings based on -o 2 from the CLI interface
let mut filter = HashSet::new();
let mut filter = IndexSet::new();
filter.insert(0);
filter.insert(5);
let mut compression = HashSet::new();
let mut compression = IndexSet::new();
compression.insert(9);
let mut strategies = HashSet::new();
let mut strategies = IndexSet::new();
for i in 0..4 {
strategies.insert(i);
}
// We always need NoOp to be present
let mut alphas = HashSet::new();
let mut alphas = IndexSet::new();
alphas.insert(AlphaOptim::NoOp);

Options {
Expand Down Expand Up @@ -502,7 +502,7 @@ fn optimize_png(
eprintln!(" File size = {} bytes", file_original_size);
}

let mut filter = opts.filter.iter().cloned().collect::<Vec<u8>>();
let mut filter = opts.filter.clone();
let compression = &opts.compression;
let mut strategies = opts.strategies.clone();

Expand All @@ -512,14 +512,14 @@ fn optimize_png(
&& png.raw.ihdr.color_type != colors::ColorType::Indexed
{
if filter.is_empty() {
filter.push(5);
filter.insert(5);
}
if strategies.is_empty() {
strategies.insert(1);
}
} else {
if filter.is_empty() {
filter.push(0);
filter.insert(0);
}
if strategies.is_empty() {
strategies.insert(0);
Expand Down Expand Up @@ -583,8 +583,10 @@ fn optimize_png(
eprintln!("Trying: {} combinations", results.len());
}

let filter_iter = filter.par_iter().with_max_len(1);
let filters: HashMap<u8, Vec<u8>> = filter_iter
let filters: IndexMap<u8, Vec<u8>> =
filter
.par_iter()
.with_max_len(1)
.map(|f| {
let png = png.clone();
(*f, png.raw.filter_image(*f))
Expand Down Expand Up @@ -865,15 +867,10 @@ fn perform_strip(png: &mut PngData, opts: &Options) {
// Strip headers
Headers::None => (),
Headers::Keep(ref hdrs) => {
let keys: Vec<[u8; 4]> = raw.aux_headers.keys().cloned().collect();
for hdr in &keys {
let preserve = std::str::from_utf8(hdr)
.ok()
.map_or(false, |name| hdrs.contains(name));
if !preserve {
raw.aux_headers.remove(hdr);
}
}
raw.aux_headers.retain(|hdr, _| {
std::str::from_utf8(hdr)
.map_or(false, |name| hdrs.contains(name))
})
}
Headers::Strip(ref hdrs) => {
for hdr in hdrs {
Expand All @@ -893,7 +890,7 @@ fn perform_strip(png: &mut PngData, opts: &Options) {
}
}
Headers::All => {
raw.aux_headers = BTreeMap::new();
raw.aux_headers = IndexMap::new();
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
#![allow(clippy::cognitive_complexity)]

use clap::{App, AppSettings, Arg, ArgMatches};
use indexmap::IndexSet;
use oxipng::AlphaOptim;
use oxipng::Deflaters;
use oxipng::Headers;
use oxipng::Options;
use oxipng::PngResult;
use oxipng::{InFile, OutFile};
use std::collections::HashSet;
use std::fs::DirBuilder;
use std::path::PathBuf;
use std::process::exit;
Expand Down Expand Up @@ -500,9 +500,9 @@ fn parse_numeric_range_opts(
input: &str,
min_value: u8,
max_value: u8,
) -> Result<HashSet<u8>, String> {
) -> Result<IndexSet<u8>, String> {
const ERROR_MESSAGE: &str = "Not a valid input";
let mut items = HashSet::new();
let mut items = IndexSet::new();

// one value
if let Ok(one_value) = input.parse::<u8>() {
Expand Down
6 changes: 3 additions & 3 deletions src/png/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use crate::headers::*;
use crate::interlace::{deinterlace_image, interlace_image};
use byteorder::{BigEndian, WriteBytesExt};
use crc::crc32;
use indexmap::IndexMap;
use rgb::ComponentSlice;
use rgb::RGBA8;
use std::collections::BTreeMap;
use std::fs::File;
use std::io::{Read, Seek, SeekFrom};
use std::iter::Iterator;
Expand Down Expand Up @@ -38,7 +38,7 @@ pub struct PngImage {
/// The pixel value that should be rendered as transparent
pub transparency_pixel: Option<Vec<u8>>,
/// All non-critical headers from the PNG are stored here
pub aux_headers: BTreeMap<[u8; 4], Vec<u8>>,
pub aux_headers: IndexMap<[u8; 4], Vec<u8>>,
}

/// Contains all data relevant to a PNG image
Expand Down Expand Up @@ -97,7 +97,7 @@ impl PngData {
}
byte_offset += 8;
// Read the data headers
let mut aux_headers: BTreeMap<[u8; 4], Vec<u8>> = BTreeMap::new();
let mut aux_headers: IndexMap<[u8; 4], Vec<u8>> = IndexMap::new();
let mut idat_headers: Vec<u8> = Vec::new();
while let Some(header) = parse_next_header(byte_data, &mut byte_offset, fix_errors)? {
match &header.name {
Expand Down
12 changes: 6 additions & 6 deletions src/reduction/alpha.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use indexmap::IndexSet;
use crate::colors::AlphaOptim;
use crate::colors::ColorType;
use crate::evaluate::Evaluator;
Expand All @@ -8,22 +9,21 @@ use crate::png::PngImage;
use crate::rayon::prelude::*;
#[cfg(feature = "parallel")]
use rayon::prelude::*;
use std::collections::HashSet;
use std::sync::Arc;

pub(crate) fn try_alpha_reductions(
png: Arc<PngImage>,
alphas: &HashSet<AlphaOptim>,
alphas: &IndexSet<AlphaOptim>,
eval: &Evaluator,
) {
if alphas.is_empty() {
return;
}

let alphas = alphas.iter().collect::<Vec<_>>();
let alphas_iter = alphas.par_iter().with_max_len(1);
alphas_iter
.filter_map(|&alpha| filtered_alpha_channel(&png, *alpha))
alphas
.par_iter()
.with_max_len(1)
.filter_map(|&alpha| filtered_alpha_channel(&png, alpha))
.for_each(|image| eval.try_image(Arc::new(image)));
}

Expand Down
6 changes: 3 additions & 3 deletions src/reduction/color.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::colors::{BitDepth, ColorType};
use crate::headers::IhdrData;
use crate::png::PngImage;
use indexmap::IndexMap;
use itertools::Itertools;
use rgb::{FromSlice, RGB8, RGBA8};
use std::collections::HashMap;
use std::hash::Hash;

#[must_use]
Expand Down Expand Up @@ -76,7 +76,7 @@ pub fn reduce_rgba_to_grayscale_alpha(png: &PngImage) -> Option<PngImage> {

fn reduce_scanline_to_palette<T>(
iter: impl IntoIterator<Item = T>,
palette: &mut HashMap<T, u8>,
palette: &mut IndexMap<T, u8>,
reduced: &mut Vec<u8>,
) -> bool
where
Expand Down Expand Up @@ -105,7 +105,7 @@ pub fn reduced_color_to_palette(png: &PngImage) -> Option<PngImage> {
return None;
}
let mut raw_data = Vec::with_capacity(png.data.len());
let mut palette = HashMap::with_capacity(257);
let mut palette = IndexMap::with_capacity(257);
let transparency_pixel = png
.transparency_pixel
.as_ref()
Expand Down
5 changes: 2 additions & 3 deletions src/reduction/mod.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use crate::colors::{BitDepth, ColorType};
use crate::headers::IhdrData;
use crate::png::PngImage;
use indexmap::map::{IndexMap, Entry::*};
use rgb::RGBA8;
use std::borrow::Cow;
use std::collections::hash_map::Entry::*;
use std::collections::HashMap;

pub mod alpha;
use crate::alpha::*;
Expand Down Expand Up @@ -79,7 +78,7 @@ pub fn reduced_palette(png: &PngImage) -> Option<PngImage> {
});

let mut next_index = 0u16;
let mut seen = HashMap::with_capacity(palette.len());
let mut seen = IndexMap::with_capacity(palette.len());
for (i, used) in used_enumerated.iter().cloned() {
if !used {
continue;
Expand Down
8 changes: 3 additions & 5 deletions tests/filters.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use oxipng;

use indexmap::IndexSet;
use oxipng::internal_tests::*;
use oxipng::{InFile, OutFile};
use std::collections::HashSet;
use std::fs::remove_file;
use std::path::Path;
use std::path::PathBuf;
Expand All @@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
let mut options = oxipng::Options::default();
options.verbosity = None;
options.force = true;
let mut filter = HashSet::new();
let mut filter = IndexSet::new();
filter.insert(0);
options.filter = filter;

Expand All @@ -33,7 +31,7 @@ fn test_it_converts(

let (output, mut opts) = get_opts(&input);
let png = PngData::new(&input, opts.fix_errors).unwrap();
opts.filter = HashSet::new();
opts.filter = IndexSet::new();
opts.filter.insert(filter);
assert_eq!(png.raw.ihdr.color_type, color_type_in);
assert_eq!(png.raw.ihdr.bit_depth, bit_depth_in);
Expand Down
8 changes: 3 additions & 5 deletions tests/flags.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
extern crate oxipng;

use indexmap::IndexSet;
use oxipng::internal_tests::*;
use oxipng::{InFile, OutFile};
use std::collections::HashSet;
use std::fs::remove_file;
use std::path::Path;
use std::path::PathBuf;
Expand All @@ -11,7 +9,7 @@ fn get_opts(input: &Path) -> (OutFile, oxipng::Options) {
let mut options = oxipng::Options::default();
options.verbosity = None;
options.force = true;
let mut filter = HashSet::new();
let mut filter = IndexSet::new();
filter.insert(0);
options.filter = filter;

Expand Down Expand Up @@ -342,7 +340,7 @@ fn interlaced_0_to_1_other_filter_mode() {
let input = PathBuf::from("tests/files/interlaced_0_to_1_other_filter_mode.png");
let (output, mut opts) = get_opts(&input);
opts.interlace = Some(1);
let mut filter = HashSet::new();
let mut filter = IndexSet::new();
filter.insert(4);
opts.filter = filter;

Expand Down
Loading