Skip to content

Commit

Permalink
Ensure --nx --nz disables all optimisations (#543)
Browse files Browse the repository at this point in the history
This PR makes two changes:
- `--nz` (`idat_recoding`) now disables all zlib recompression,
including iCCP and fDAT chunks. (Perhaps we should rename the option to
`zlib_recompression`?)
- `--nx` now also disables the default deinterlacing, though it can
still be overridden with `-i`.

`--nx --nz` does disable all optimisations in the v8 release and we
should ensure it continues to do so in the next release. (This is
related to discussions around removing the `--check` option.)
  • Loading branch information
andrews05 authored Sep 2, 2023
1 parent 02bd47b commit aa956fb
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 23 deletions.
28 changes: 15 additions & 13 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ pub struct Options {
///
/// Default: `true`
pub grayscale_reduction: bool,
/// Whether to perform IDAT recoding
/// Whether to perform recoding of IDAT and other compressed chunks
///
/// If any type of reduction is performed, IDAT recoding will be performed
/// regardless of this setting
Expand Down Expand Up @@ -904,23 +904,25 @@ fn postprocess_chunks(
None
};
// sRGB-like profile can be replaced with an sRGB chunk with the same rendering intent
// Otherwise try recompressing the profile
if let Some(intent) = intent {
trace!("Replacing iCCP chunk with equivalent sRGB chunk");
png.aux_chunks[iccp_idx] = Chunk {
name: *b"sRGB",
data: vec![intent],
};
} else if let Ok(iccp) = construct_iccp(&icc, opts.deflate) {
let cur_len = png.aux_chunks[iccp_idx].data.len();
let new_len = iccp.data.len();
if new_len < cur_len {
debug!(
"Recompressed iCCP chunk: {} ({} bytes decrease)",
new_len,
cur_len - new_len
);
png.aux_chunks[iccp_idx] = iccp;
} else if opts.idat_recoding {
// Try recompressing the profile
if let Ok(iccp) = construct_iccp(&icc, opts.deflate) {
let cur_len = png.aux_chunks[iccp_idx].data.len();
let new_len = iccp.data.len();
if new_len < cur_len {
debug!(
"Recompressed iCCP chunk: {} ({} bytes decrease)",
new_len,
cur_len - new_len
);
png.aux_chunks[iccp_idx] = iccp;
}
}
}
}
Expand Down Expand Up @@ -950,7 +952,7 @@ fn postprocess_chunks(
.iter_mut()
.filter(|c| &c.name == b"fdAT")
.collect();
if !fdat.is_empty() {
if opts.idat_recoding && !fdat.is_empty() {
let buffer_size = orig_ihdr.raw_data_size();
fdat.par_iter_mut()
.with_max_len(1)
Expand Down
21 changes: 11 additions & 10 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,13 +225,13 @@ fn main() {
)
.arg(
Arg::new("no-reductions")
.help("No reductions")
.help("No reductions or deinterlacing")
.long("nx")
.action(ArgAction::SetTrue),
)
.arg(
Arg::new("no-recoding")
.help("No IDAT recoding unless necessary")
.help("No recoding of IDAT or other compressed chunks unless necessary")
.long("nz")
.action(ArgAction::SetTrue),
)
Expand Down Expand Up @@ -431,14 +431,6 @@ fn parse_opts_into_struct(
Some(level) => Options::from_preset(level.parse::<u8>().unwrap()),
};

if let Some(x) = matches.get_one::<String>("interlace") {
opts.interlace = if x == "keep" {
None
} else {
x.parse::<u8>().unwrap().try_into().ok()
};
}

if let Some(x) = matches.get_one::<IndexSet<u8>>("filters") {
opts.filter.clear();
for &f in x {
Expand Down Expand Up @@ -507,10 +499,19 @@ fn parse_opts_into_struct(
opts.color_type_reduction = false;
opts.palette_reduction = false;
opts.grayscale_reduction = false;
opts.interlace = None;
}

opts.idat_recoding = !matches.get_flag("no-recoding");

if let Some(x) = matches.get_one::<String>("interlace") {
opts.interlace = if x == "keep" {
None
} else {
x.parse::<u8>().unwrap().try_into().ok()
};
}

if let Some(keep) = matches.get_one::<String>("keep") {
let names = keep
.split(',')
Expand Down

0 comments on commit aa956fb

Please sign in to comment.