Move Zlib-specific options under Zlib compressor (#210)

Make it statically obvious that they're not compatible with other compressors.

src/deflate/mod.rs

@@ -2,6 +2,7 @@ use crate::atomicmin::AtomicMin;
 use crate::error::PngError;
 use crate::Deadline;
 use crate::PngResult;
+use indexmap::IndexSet;
 use miniz_oxide;
 use std::cmp::max;
 use zopfli;
@@ -59,11 +60,27 @@ pub fn zopfli_deflate(data: &[u8]) -> PngResult<Vec<u8>> {
     Ok(output)
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 /// DEFLATE algorithms supported by oxipng
 pub enum Deflaters {
     /// Use the Zlib/Miniz DEFLATE implementation
-    Zlib,
+    Zlib {
+        /// Which zlib compression levels to try on the file (1-9)
+        ///
+        /// Default: `9`
+        compression: IndexSet<u8>,
+        /// Which zlib compression strategies to try on the file (0-3)
+        ///
+        /// Default: `0-3`
+        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.
+        /// 8-15 are valid values.
+        ///
+        /// Default: `15`
+        window: u8,
+    },
     /// Use the better but slower Zopfli implementation
     Zopfli,
     /// Use libdeflater.

src/lib.rs

@@ -155,21 +155,6 @@ pub struct Options {
     ///
     /// Default: `None`
     pub interlace: Option<u8>,
-    /// Which zlib compression levels to try on the file (1-9)
-    ///
-    /// Default: `9`
-    pub compression: IndexSet<u8>,
-    /// Which zlib compression strategies to try on the file (0-3)
-    ///
-    /// Default: `0-3`
-    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.
-    /// 8-15 are valid values.
-    ///
-    /// Default: `15`
-    pub window: u8,
     /// Alpha filtering strategies to use
     pub alphas: IndexSet<colors::AlphaOptim>,
     /// Whether to attempt bit depth reduction
@@ -229,14 +214,18 @@ impl Options {
     // on an `Options` struct generated by the `default` method.
     fn apply_preset_0(mut self) -> Self {
         self.idat_recoding = false;
-        self.compression.clear();
+        if let Deflaters::Zlib { compression, .. } = &mut self.deflate {
-        self.compression.insert(3);
+            compression.clear();
+            compression.insert(3);
+        }
         self
     }
 
     fn apply_preset_1(mut self) -> Self {
         self.filter.clear();
-        self.strategies.clear();
+        if let Deflaters::Zlib { strategies, .. } = &mut self.deflate {
+            strategies.clear();
+        }
         self.use_heuristics = true;
         self
     }
@@ -257,15 +246,21 @@ impl Options {
     }
 
     fn apply_preset_5(mut self) -> Self {
-        for i in 3..9 {
+        if let Deflaters::Zlib { compression, .. } = &mut self.deflate {
-            self.compression.insert(i);
+            compression.clear();
+            for i in 3..9 {
+                compression.insert(i);
+            }
         }
         self.apply_preset_4()
     }
 
     fn apply_preset_6(mut self) -> Self {
-        for i in 1..3 {
+        if let Deflaters::Zlib { compression, .. } = &mut self.deflate {
-            self.compression.insert(i);
+            compression.clear();
+            for i in 1..3 {
+                compression.insert(i);
+            }
         }
         self.apply_preset_5()
     }
@@ -297,16 +292,17 @@ impl Default for Options {
             verbosity: Some(0),
             filter,
             interlace: None,
-            compression,
-            strategies,
-            window: 15,
             alphas,
             bit_depth_reduction: true,
             color_type_reduction: true,
             palette_reduction: true,
             idat_recoding: true,
             strip: Headers::None,
-            deflate: Deflaters::Zlib,
+            deflate: Deflaters::Zlib {
+                compression,
+                strategies,
+                window: 15,
+            },
             use_heuristics: false,
             timeout: None,
         }
@@ -476,26 +472,26 @@ fn optimize_png(
     }
 
     let mut filter = opts.filter.clone();
-    let compression = &opts.compression;
+    let mut strategies = match &opts.deflate {
-    let mut strategies = opts.strategies.clone();
+        Deflaters::Zlib { strategies, .. } => Some(strategies.clone()),
+        _ => None,
+    };
 
     if opts.use_heuristics {
         // Heuristically determine which set of options to use
-        if png.raw.ihdr.bit_depth.as_u8() >= 8
+        let (use_filter, use_strategy) = if png.raw.ihdr.bit_depth.as_u8() >= 8
             && png.raw.ihdr.color_type != colors::ColorType::Indexed
         {
-            if filter.is_empty() {
+            (5, 1)
-                filter.insert(5);
-            }
-            if strategies.is_empty() {
-                strategies.insert(1);
-            }
         } else {
-            if filter.is_empty() {
+            (0, 0)
-                filter.insert(0);
+        };
-            }
+        if filter.is_empty() {
+            filter.insert(use_filter);
+        }
+        if let Some(strategies) = &mut strategies {
             if strategies.is_empty() {
-                strategies.insert(0);
+                strategies.insert(use_strategy);
             }
         }
     }
@@ -517,17 +513,17 @@ fn optimize_png(
 
     if opts.idat_recoding || reduction_occurred {
         // Go through selected permutations and determine the best
-        let combinations = if opts.deflate == Deflaters::Zlib && !deadline.passed() {
+        let combinations = if let Deflaters::Zlib { compression, .. } = &opts.deflate {
-            filter.len() * compression.len() * strategies.len()
+            filter.len() * compression.len() * strategies.as_ref().unwrap().len()
         } else {
             filter.len()
         };
         let mut results: Vec<TrialOptions> = Vec::with_capacity(combinations);
 
         for f in &filter {
-            if opts.deflate == Deflaters::Zlib {
+            if let Deflaters::Zlib { compression, .. } = &opts.deflate {
                 for zc in compression {
-                    for zs in &strategies {
+                    for zs in strategies.as_ref().unwrap() {
                         results.push(TrialOptions {
                             filter: *f,
                             compression: *zc,
@@ -576,11 +572,11 @@ fn optimize_png(
             }
             let filtered = &filters[&trial.filter];
             let new_idat = match opts.deflate {
-                Deflaters::Zlib => deflate::deflate(
+                Deflaters::Zlib { window, .. } => deflate::deflate(
                     filtered,
                     trial.compression,
                     trial.strategy,
-                    opts.window,
+                    window,
                     &best_size,
                     &deadline,
                 ),

src/main.rs

@@ -325,14 +325,6 @@ fn parse_opts_into_struct(
         opts.filter = parse_numeric_range_opts(x, 0, 5).unwrap();
     }
 
-    if let Some(x) = matches.value_of("compression") {
-        opts.compression = parse_numeric_range_opts(x, 1, 9).unwrap();
-    }
-
-    if let Some(x) = matches.value_of("strategies") {
-        opts.strategies = parse_numeric_range_opts(x, 0, 3).unwrap();
-    }
-
     if let Some(x) = matches.value_of("timeout") {
         let num = x
             .parse()
@@ -340,18 +332,6 @@ fn parse_opts_into_struct(
         opts.timeout = Some(Duration::from_secs(num));
     }
 
-    match matches.value_of("window") {
-        Some("256") => opts.window = 8,
-        Some("512") => opts.window = 9,
-        Some("1k") => opts.window = 10,
-        Some("2k") => opts.window = 11,
-        Some("4k") => opts.window = 12,
-        Some("8k") => opts.window = 13,
-        Some("16k") => opts.window = 14,
-        // 32k is default
-        _ => (),
-    }
-
     let out_dir = if let Some(x) = matches.value_of("output_dir") {
         let path = PathBuf::from(x);
         if !path.exists() {
@@ -482,9 +462,46 @@ fn parse_opts_into_struct(
     }
 
     if matches.is_present("libdeflater") {
+        if matches.is_present("zopfli") {
+            return Err("zopfli and libdeflater can't be used simultaneously".to_owned());
+        }
         opts.deflate = Deflaters::Libdeflater;
     }
 
+    if let Deflaters::Zlib {
+        compression,
+        strategies,
+        window,
+    } = &mut opts.deflate
+    {
+        if let Some(x) = matches.value_of("compression") {
+            *compression = parse_numeric_range_opts(x, 1, 9).unwrap();
+        }
+
+        if let Some(x) = matches.value_of("strategies") {
+            *strategies = parse_numeric_range_opts(x, 0, 3).unwrap();
+        }
+
+        match matches.value_of("window") {
+            Some("256") => *window = 8,
+            Some("512") => *window = 9,
+            Some("1k") => *window = 10,
+            Some("2k") => *window = 11,
+            Some("4k") => *window = 12,
+            Some("8k") => *window = 13,
+            Some("16k") => *window = 14,
+            // 32k is default
+            _ => (),
+        }
+    } else if matches.is_present("compression")
+        || matches.is_present("strategies")
+        || matches.is_present("window")
+    {
+        return Err(
+            "compression, strategies and window options are compatible only with zlib".to_owned(),
+        );
+    }
+
     if let Some(x) = matches.value_of("threads") {
         let threads = x.parse::<usize>().unwrap();