Fix two bugs in reducing palette and release version 0.14.2

CHANGELOG.md

@@ -1,3 +1,7 @@
+**Version 0.14.2**
+ - Fix a bug when reducing palette in images with bit depth less than 8
+ - Fix a bug when reducing palette in images with transparency
+
 **Version 0.14.1**
  - Remove zlib dependency and switch entirely to miniz, since zlib 1.2.11 was not working with oxipng. This costs some performance, but is better than having a broken application.
 

Cargo.lock

@@ -1,6 +1,6 @@
 [root]
 name = "oxipng"
-version = "0.14.1"
+version = "0.14.2"
 dependencies = [
  "bit-vec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",

Cargo.toml

@@ -8,7 +8,7 @@ homepage = "https://github.com/shssoichiro/oxipng"
 license = "MIT"
 name = "oxipng"
 repository = "https://github.com/shssoichiro/oxipng"
-version = "0.14.1"
+version = "0.14.2"
 
 [lib]
 name = "oxipng"
@@ -55,4 +55,4 @@ nightly-binary = ["binary", "nightly", "regex/simd-accel"]
 dev = ["nightly-binary", "clippy"]
 
 [profile.dev]
-opt-level = 2
+opt-level = 2

src/png.rs

@@ -465,9 +465,29 @@ impl PngData {
             return false;
         }
 
-        // A palette with RGB slices
+        // A palette with RGB or RGBA slices
-        let palette = self.palette.clone().unwrap();
+        let palette = if let Some(ref trns) = self.transparency_palette {
-        let mut indexed_palette: Vec<&[u8]> = palette.chunks(3).collect();
+            self.palette
+                .clone()
+                .unwrap()
+                .chunks(3)
+                .zip(trns.iter())
+                .flat_map(|(pixel, trns)| {
+                    let mut pixel = pixel.to_owned();
+                    pixel.push(*trns);
+                    pixel
+                })
+                .collect()
+        } else {
+            self.palette.clone().unwrap()
+        };
+        let mut indexed_palette: Vec<&[u8]> =
+            palette.chunks(if self.transparency_palette.is_some() {
+                    4
+                } else {
+                    3
+                })
+                .collect();
         // A map of old indexes to new ones, for any moved
         let mut index_map: HashMap<u8, u8> = HashMap::new();
 
@@ -589,49 +609,49 @@ impl PngData {
                 }
                 BitDepth::Four => {
                     for byte in &line.data {
-                        let upper = *byte >> 4;
+                        let upper = *byte & 0b11110000;
                         let lower = *byte & 0b00001111;
                         let mut new_byte = 0u8;
-                        if let Some(new_idx) = index_map.get(&upper) {
+                        new_byte |= if let Some(new_idx) = index_map.get(&upper) {
-                            new_byte &= *new_idx << 4;
+                            *new_idx << 4
                         } else {
-                            new_byte &= upper << 4;
+                            upper << 4
-                        }
+                        };
-                        if let Some(new_idx) = index_map.get(&lower) {
+                        new_byte |= if let Some(new_idx) = index_map.get(&lower) {
-                            new_byte &= *new_idx;
+                            *new_idx
                         } else {
-                            new_byte &= lower;
+                            lower
-                        }
+                        };
                         new_data.push(new_byte);
                     }
                 }
                 BitDepth::Two => {
                     for byte in &line.data {
-                        let one = *byte >> 6;
+                        let one = *byte & 0b11000000;
-                        let two = (*byte >> 4) & 0b00000011;
+                        let two = *byte & 0b00110000;
-                        let three = (*byte >> 2) & 0b00000011;
+                        let three = *byte & 0b00001100;
                         let four = *byte & 0b00000011;
                         let mut new_byte = 0u8;
-                        if let Some(new_idx) = index_map.get(&one) {
+                        new_byte |= if let Some(new_idx) = index_map.get(&one) {
-                            new_byte &= *new_idx << 6;
+                            *new_idx << 6
                         } else {
-                            new_byte &= one << 6;
+                            one << 6
-                        }
+                        };
-                        if let Some(new_idx) = index_map.get(&two) {
+                        new_byte |= if let Some(new_idx) = index_map.get(&two) {
-                            new_byte &= *new_idx << 4;
+                            *new_idx << 4
                         } else {
-                            new_byte &= two << 4;
+                            two << 4
-                        }
+                        };
-                        if let Some(new_idx) = index_map.get(&three) {
+                        new_byte |= if let Some(new_idx) = index_map.get(&three) {
-                            new_byte &= *new_idx << 2;
+                            *new_idx << 2
                         } else {
-                            new_byte &= three << 2;
+                            three << 2
-                        }
+                        };
-                        if let Some(new_idx) = index_map.get(&four) {
+                        new_byte |= if let Some(new_idx) = index_map.get(&four) {
-                            new_byte &= *new_idx;
+                            *new_idx
                         } else {
-                            new_byte &= four;
+                            four
-                        }
+                        };
                         new_data.push(new_byte);
                     }
                 }

tests/filters.rs

@@ -56,8 +56,8 @@ fn test_it_converts(input: &Path,
     let new_png = image::open(output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }

tests/flags.rs

@@ -58,8 +58,8 @@ fn test_it_converts(input: &Path,
     let new_png = image::open(output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -115,8 +115,8 @@ fn strip_headers_list() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -156,8 +156,8 @@ fn strip_headers_safe() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -197,8 +197,8 @@ fn strip_headers_all() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -238,8 +238,8 @@ fn strip_headers_none() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -275,8 +275,8 @@ fn interlacing_0_to_1() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -312,8 +312,8 @@ fn interlacing_1_to_0() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -353,8 +353,8 @@ fn interlacing_0_to_1_small_files() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -394,8 +394,8 @@ fn interlacing_1_to_0_small_files() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -434,8 +434,8 @@ fn interlaced_0_to_1_other_filter_mode() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }

tests/interlaced.rs

@@ -57,8 +57,8 @@ fn test_it_converts(input: &Path,
     let new_png = image::open(output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }

tests/reduction.rs

@@ -57,8 +57,8 @@ fn test_it_converts(input: &Path,
     let new_png = image::open(output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -1268,8 +1268,8 @@ fn palette_should_be_reduced_with_dupes() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -1308,8 +1308,8 @@ fn palette_should_be_reduced_with_unused() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -1348,8 +1348,8 @@ fn palette_should_be_reduced_with_both() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }

tests/regression.rs

@@ -56,8 +56,8 @@ fn test_it_converts(input: &Path,
     let new_png = image::open(output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -65,9 +65,7 @@ fn test_it_converts(input: &Path,
 #[test]
 fn issue_29() {
     let input = PathBuf::from("tests/files/issue-29.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -114,8 +112,8 @@ fn issue_42() {
     let new_png = image::open(&output).unwrap();
 
     // Conversion should be lossless
-    assert_eq!(old_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>(),
+    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>>>());
+            new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
 
     remove_file(output).ok();
 }
@@ -123,9 +121,7 @@ fn issue_42() {
 #[test]
 fn issue_52_01() {
     let input = PathBuf::from("tests/files/issue-52-01.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -140,9 +136,7 @@ fn issue_52_01() {
 #[test]
 fn issue_52_02() {
     let input = PathBuf::from("tests/files/issue-52-02.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -157,9 +151,7 @@ fn issue_52_02() {
 #[test]
 fn issue_52_03() {
     let input = PathBuf::from("tests/files/issue-52-03.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -174,9 +166,7 @@ fn issue_52_03() {
 #[test]
 fn issue_52_04() {
     let input = PathBuf::from("tests/files/issue-52-04.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -191,9 +181,7 @@ fn issue_52_04() {
 #[test]
 fn issue_52_05() {
     let input = PathBuf::from("tests/files/issue-52-05.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -208,9 +196,7 @@ fn issue_52_05() {
 #[test]
 fn issue_52_06() {
     let input = PathBuf::from("tests/files/issue-52-06.png");
-    let mut opts = get_opts(&input);
+    let opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
     let output = opts.out_file.clone();
 
     test_it_converts(&input,
@@ -221,3 +207,18 @@ fn issue_52_06() {
                      ColorType::Indexed,
                      BitDepth::Two);
 }
+
+#[test]
+fn issue_56() {
+    let input = PathBuf::from("tests/files/issue-56.png");
+    let opts = get_opts(&input);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::Indexed,
+                     BitDepth::Four,
+                     ColorType::Indexed,
+                     BitDepth::Four);
+}