From aa956fbc475556d5896bb2b4ce408325e6d56792 Mon Sep 17 00:00:00 2001
From: andrews05 <andrew@digerati.co.nz>
Date: Sun, 3 Sep 2023 01:00:09 +1200
Subject: [PATCH] Ensure `--nx --nz` disables all optimisations (#543)

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.)
---
 src/lib.rs  | 28 +++++++++++++++-------------
 src/main.rs | 21 +++++++++++----------
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index ae3b93ef..bde4deed 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -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
@@ -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;
+                    }
                 }
             }
         }
@@ -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)
diff --git a/src/main.rs b/src/main.rs
index f2166785..12abcbbb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -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),
         )
@@ -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 {
@@ -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(',')