Merge pull request #53 from shssoichiro/52-invalid-chunk-warnings

Reduce certain headers when reducing color type

.travis.yml

@@ -11,7 +11,7 @@ matrix:
       env: TARGET=x86_64-apple-darwin
       cache: cargo
     - os: linux
-      rust: 1.9.0
+      rust: 1.11.0
       env: TARGET=x86_64-unknown-linux-gnu
       cache: cargo
     - os: linux

CHANGELOG.md

@@ -1,3 +1,6 @@
+**Version 0.12.1 [unreleased]**
+ - Fix bug in certain PNG headers when reducing color type ([#52](https://github.com/shssoichiro/oxipng/issues/52))
+
 **Version 0.12.0**
  - Performance optimizations
  - Fix processing filenames that contain commas (@aliceatlas [#50](https://github.com/shssoichiro/oxipng/pull/50))

Cargo.lock

@@ -9,7 +9,7 @@ dependencies = [
  "crc 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "image 0.10.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "libz-sys 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "miniz-sys 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -58,7 +58,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "ansi_term 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "strsim 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "term_size 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "unicode-segmentation 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -82,7 +82,7 @@ dependencies = [
  "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "quine-mc_cluskey 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "regex-syntax 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
- "rustc-serialize 0.3.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rustc-serialize 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)",
  "semver 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "toml 0.1.30 (registry+https://github.com/rust-lang/crates.io-index)",
  "unicode-normalization 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -117,13 +117,13 @@ name = "flate2"
 version = "0.2.14"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "miniz-sys 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "gcc"
-version = "0.3.38"
+version = "0.3.39"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -171,7 +171,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "libc"
-version = "0.2.17"
+version = "0.2.18"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -179,8 +179,8 @@ name = "libz-sys"
 version = "1.0.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "gcc 0.3.38 (registry+https://github.com/rust-lang/crates.io-index)",
+ "gcc 0.3.39 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "pkg-config 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -194,7 +194,7 @@ name = "memchr"
 version = "0.1.11"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -202,8 +202,8 @@ name = "miniz-sys"
 version = "0.1.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "gcc 0.3.38 (registry+https://github.com/rust-lang/crates.io-index)",
+ "gcc 0.3.39 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -229,7 +229,7 @@ dependencies = [
  "num-integer 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.1.36 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)",
- "rustc-serialize 0.3.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rustc-serialize 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -257,7 +257,7 @@ dependencies = [
  "num-bigint 0.1.35 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-integer 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.1.36 (registry+https://github.com/rust-lang/crates.io-index)",
- "rustc-serialize 0.3.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rustc-serialize 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -270,7 +270,7 @@ name = "num_cpus"
 version = "1.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -299,7 +299,7 @@ name = "rand"
 version = "0.3.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -308,7 +308,7 @@ version = "0.4.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "deque 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "num_cpus 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -333,7 +333,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "rustc-serialize"
-version = "0.3.21"
+version = "0.3.22"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -359,7 +359,7 @@ name = "term_size"
 version = "0.1.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -368,7 +368,7 @@ version = "2.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -384,7 +384,7 @@ name = "toml"
 version = "0.1.30"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "rustc-serialize 0.3.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rustc-serialize 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -446,14 +446,14 @@ dependencies = [
 "checksum deque 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "1614659040e711785ed8ea24219140654da1729f3ec8a47a9719d041112fe7bf"
 "checksum enum_primitive 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f79eff5be92a4d7d5bddf7daa7d650717ea71628634efe6ca7bcda85b2183c23"
 "checksum flate2 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "3eeb481e957304178d2e782f2da1257f1434dfecbae883bafb61ada2a9fea3bb"
-"checksum gcc 0.3.38 (registry+https://github.com/rust-lang/crates.io-index)" = "553f11439bdefe755bf366b264820f1da70f3aaf3924e594b886beb9c831bcf5"
+"checksum gcc 0.3.39 (registry+https://github.com/rust-lang/crates.io-index)" = "771e4a97ff6f237cf0f7d5f5102f6e28bb9743814b6198d684da5c58b76c11e0"
 "checksum glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "8be18de09a56b60ed0edf84bc9df007e30040691af7acd1c41874faac5895bfb"
 "checksum image 0.10.4 (registry+https://github.com/rust-lang/crates.io-index)" = "76df2dce95fef56fd35dbc41c36e37b19aede703c6be7739e8b65d5788ffc728"
 "checksum inflate 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "e7e0062d2dc2f17d2f13750d95316ae8a2ff909af0fda957084f5defd87c43bb"
 "checksum itertools 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)" = "c4a9b56eb56058f43dc66e58f40a214b2ccbc9f3df51861b63d51dec7b65bc3f"
 "checksum kernel32-sys 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7507624b29483431c0ba2d82aece8ca6cdba9382bff4ddd0f7490560c056098d"
 "checksum lazy_static 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6abe0ee2e758cd6bc8a2cd56726359007748fbf4128da998b65d0b70f881e19b"
-"checksum libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)" = "044d1360593a78f5c8e5e710beccdc24ab71d1f01bc19a29bcacdba22e8475d8"
+"checksum libc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)" = "a51822fc847e7a8101514d1d44e354ba2ffa7d4c194dcab48870740e327cac70"
 "checksum libz-sys 1.0.7 (registry+https://github.com/rust-lang/crates.io-index)" = "283c2d162f78c5090522e13fc809820c33181570ae40de1bea84f3864c8759f9"
 "checksum matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "efd7622e3022e1a6eaa602c4cea8912254e5582c9c692e9167714182244801b1"
 "checksum memchr 0.1.11 (registry+https://github.com/rust-lang/crates.io-index)" = "d8b629fb514376c675b98c1421e80b151d3817ac42d7c667717d282761418d20"
@@ -473,7 +473,7 @@ dependencies = [
 "checksum rayon 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "0f0783f5880c56f5a308e219ac9309dbe781e064741dd5def4c617c440890305"
 "checksum regex 0.1.80 (registry+https://github.com/rust-lang/crates.io-index)" = "4fd4ace6a8cf7860714a2c2280d6c1f7e6a413486c13298bbc86fd3da019402f"
 "checksum regex-syntax 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "f9ec002c35e86791825ed294b50008eea9ddfc8def4420124fbc6b08db834957"
-"checksum rustc-serialize 0.3.21 (registry+https://github.com/rust-lang/crates.io-index)" = "bff9fc1c79f2dec76b253273d07682e94a978bd8f132ded071188122b2af9818"
+"checksum rustc-serialize 0.3.22 (registry+https://github.com/rust-lang/crates.io-index)" = "237546c689f20bb44980270c73c3b9edd0891c1be49cc1274406134a66d3957b"
 "checksum semver 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "2d5b7638a1f03815d94e88cb3b3c08e87f0db4d683ef499d1836aaf70a45623f"
 "checksum simd 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "63b5847c2d766ca7ce7227672850955802fabd779ba616aeabead4c2c3877023"
 "checksum strsim 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "67f84c44fbb2f91db7fef94554e6b2ac05909c9c0b0bc23bb98d3a1aebfe7f7c"

README.md

@@ -30,7 +30,7 @@ cp target/release/oxipng /usr/local/bin
 Please note that zlib is a required build dependency. Oxipng should work with any 1.x version of zlib,
 but you are advised to use the latest version (currently 1.2.8) for security and bug fixes.
 
-The current minimum supported Rust version is **1.9.0**. Oxipng may compile on earlier versions of Rust,
+The current minimum supported Rust version is **1.11.0**. Oxipng may compile on earlier versions of Rust,
 but there is no guarantee.
 
 Oxipng follows Semantic Versioning.

ci/script.sh

@@ -41,7 +41,7 @@ run_test_suite() {
     cargo test --target $TARGET --release
 
     # sanity check the file type
-    file target/$TARGET/debug/oxipng
+    file target/$TARGET/release/oxipng
 }
 
 main() {

src/png.rs

@@ -418,14 +418,7 @@ impl PngData {
         if self.ihdr_data.bit_depth != BitDepth::Sixteen {
             if self.ihdr_data.color_type == ColorType::Indexed ||
                self.ihdr_data.color_type == ColorType::Grayscale {
-                return match reduce_bit_depth_8_or_less(self) {
+                return reduce_bit_depth_8_or_less(self);
-                    Some((data, depth)) => {
-                        self.raw_data = data;
-                        self.ihdr_data.bit_depth = BitDepth::from_u8(depth);
-                        true
-                    }
-                    None => false,
-                };
             }
             return false;
         }
@@ -660,57 +653,35 @@ impl PngData {
         // Go down one step at a time
         // Maybe not the most efficient, but it's safe
         if self.ihdr_data.color_type == ColorType::RGBA {
-            if let Some(data) = reduce_rgba_to_grayscale_alpha(self) {
+            if reduce_rgba_to_grayscale_alpha(self) {
-                self.raw_data = data;
-                self.ihdr_data.color_type = ColorType::GrayscaleAlpha;
                 changed = true;
-            } else if let Some(data) = reduce_rgba_to_rgb(self) {
+            } else if reduce_rgba_to_rgb(self) {
-                self.raw_data = data;
-                self.ihdr_data.color_type = ColorType::RGB;
                 changed = true;
-            } else if let Some((data, palette, trans)) = reduce_rgba_to_palette(self) {
+            } else if reduce_rgba_to_palette(self) {
-                self.raw_data = data;
-                self.palette = Some(palette);
-                if trans.iter().any(|x| *x != 255) {
-                    self.transparency_palette = Some(trans);
-                } else {
-                    self.transparency_palette = None;
-                }
-                self.ihdr_data.color_type = ColorType::Indexed;
                 changed = true;
                 should_reduce_bit_depth = true;
             }
         }
 
         if self.ihdr_data.color_type == ColorType::GrayscaleAlpha {
-            if let Some(data) = reduce_grayscale_alpha_to_grayscale(self) {
+            if reduce_grayscale_alpha_to_grayscale(self) {
-                self.raw_data = data;
-                self.ihdr_data.color_type = ColorType::Grayscale;
                 changed = true;
                 should_reduce_bit_depth = true;
             }
         }
 
         if self.ihdr_data.color_type == ColorType::RGB {
-            if let Some(data) = reduce_rgb_to_grayscale(self) {
+            if reduce_rgb_to_grayscale(self) {
-                self.raw_data = data;
-                self.ihdr_data.color_type = ColorType::Grayscale;
                 changed = true;
                 should_reduce_bit_depth = true;
-            } else if let Some((data, palette)) = reduce_rgb_to_palette(self) {
+            } else if reduce_rgb_to_palette(self) {
-                self.raw_data = data;
-                self.palette = Some(palette);
-                self.ihdr_data.color_type = ColorType::Indexed;
                 changed = true;
                 should_reduce_bit_depth = true;
             }
         }
 
         if self.ihdr_data.color_type == ColorType::Grayscale {
-            if let Some((data, palette)) = reduce_grayscale_to_palette(self) {
+            if reduce_grayscale_to_palette(self) {
-                self.raw_data = data;
-                self.palette = Some(palette);
-                self.ihdr_data.color_type = ColorType::Indexed;
                 changed = true;
                 should_reduce_bit_depth = true;
             }
@@ -719,10 +690,7 @@ impl PngData {
         if should_reduce_bit_depth {
             // Some conversions will allow us to perform bit depth reduction that
             // wasn't possible before
-            if let Some((data, depth)) = reduce_bit_depth_8_or_less(self) {
+            reduce_bit_depth_8_or_less(self);
-                self.raw_data = data;
-                self.ihdr_data.bit_depth = BitDepth::from_u8(depth);
-            }
         }
 
         changed

src/reduction.rs

@@ -1,9 +1,9 @@
 use bit_vec::BitVec;
-use colors::BitDepth;
+use colors::{BitDepth, ColorType};
 use itertools::Itertools;
 use png::PngData;
 
-pub fn reduce_bit_depth_8_or_less(png: &PngData) -> Option<(Vec<u8>, u8)> {
+pub fn reduce_bit_depth_8_or_less(png: &mut PngData) -> bool {
     let mut reduced = BitVec::with_capacity(png.raw_data.len() * 8);
     let bit_depth: usize = png.ihdr_data.bit_depth.as_u8() as usize;
     let mut allowed_bits = 1;
@@ -15,7 +15,7 @@ pub fn reduce_bit_depth_8_or_less(png: &PngData) -> Option<(Vec<u8>, u8)> {
                 allowed_bits = bit_index.next_power_of_two();
                 if allowed_bits == bit_depth {
                     // Not reducable
-                    return None;
+                    return false;
                 }
             }
         }
@@ -36,14 +36,22 @@ pub fn reduce_bit_depth_8_or_less(png: &PngData) -> Option<(Vec<u8>, u8)> {
         }
     }
 
-    Some((reduced.to_bytes(), allowed_bits as u8))
+    png.raw_data = reduced.to_bytes();
+    png.ihdr_data.bit_depth = BitDepth::from_u8(allowed_bits as u8);
+    true
 }
 
-pub fn reduce_rgba_to_rgb(png: &PngData) -> Option<Vec<u8>> {
+pub fn reduce_rgba_to_rgb(png: &mut PngData) -> bool {
-    reduce_alpha_channel(png, 4)
+    if let Some(reduced) = reduce_alpha_channel(png, 4) {
+        png.raw_data = reduced;
+        png.ihdr_data.color_type = ColorType::RGB;
+        true
+    } else {
+        false
+    }
 }
 
-pub fn reduce_rgba_to_grayscale_alpha(png: &PngData) -> Option<Vec<u8>> {
+pub fn reduce_rgba_to_grayscale_alpha(png: &mut PngData) -> bool {
     let mut reduced = Vec::with_capacity(png.raw_data.len());
     let byte_depth: u8 = png.ihdr_data.bit_depth.as_u8() >> 3;
     let bpp: usize = 4 * byte_depth as usize;
@@ -66,11 +74,11 @@ pub fn reduce_rgba_to_grayscale_alpha(png: &PngData) -> Option<Vec<u8>> {
 
             if i % bpp == bpp - 1 {
                 if low_bytes.iter().unique().count() > 1 {
-                    return None;
+                    return false;
                 }
                 if byte_depth == 2 {
                     if high_bytes.iter().unique().count() > 1 {
-                        return None;
+                        return false;
                     }
                     reduced.push(high_bytes[0]);
                     high_bytes.clear();
@@ -83,12 +91,24 @@ pub fn reduce_rgba_to_grayscale_alpha(png: &PngData) -> Option<Vec<u8>> {
         }
     }
 
-    Some(reduced)
+    if let Some(sbit_header) = png.aux_headers.get_mut(&"sBIT".to_string()) {
+        assert_eq!(sbit_header.len(), 4);
+        sbit_header.remove(1);
+        sbit_header.remove(1);
+    }
+    if let Some(bkgd_header) = png.aux_headers.get_mut(&"bKGD".to_string()) {
+        assert_eq!(bkgd_header.len(), 6);
+        bkgd_header.truncate(2);
+    }
+
+    png.raw_data = reduced;
+    png.ihdr_data.color_type = ColorType::GrayscaleAlpha;
+    true
 }
 
-pub fn reduce_rgba_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>, Vec<u8>)> {
+pub fn reduce_rgba_to_palette(png: &mut PngData) -> bool {
     if png.ihdr_data.bit_depth != BitDepth::Eight {
-        return None;
+        return false;
     }
     let mut reduced = Vec::with_capacity(png.raw_data.len());
     let mut palette = Vec::with_capacity(256);
@@ -104,7 +124,7 @@ pub fn reduce_rgba_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>, Vec<u8
                 } else {
                     let len = palette.len();
                     if len == 256 {
-                        return None;
+                        return false;
                     }
                     palette.push(cur_pixel);
                     reduced.push(len as u8);
@@ -126,12 +146,38 @@ pub fn reduce_rgba_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>, Vec<u8
         }
     }
 
-    Some((reduced, color_palette, trans_palette))
+    if let Some(bkgd_header) = png.aux_headers.get_mut(&"bKGD".to_string()) {
+        assert_eq!(bkgd_header.len(), 6);
+        let header_pixels = bkgd_header.iter().skip(1).step(2).cloned().collect::<Vec<u8>>();
+        if let Some(entry) = color_palette.chunks(3).position(|x| x == header_pixels.as_slice()) {
+            *bkgd_header = vec![entry as u8];
+        } else if color_palette.len() == 255 {
+            return false;
+        } else {
+            let entry = color_palette.len() / 3;
+            color_palette.extend_from_slice(&header_pixels);
+            *bkgd_header = vec![entry as u8];
+        }
+    }
+    if let Some(sbit_header) = png.aux_headers.get_mut(&"sBIT".to_string()) {
+        assert_eq!(sbit_header.len(), 4);
+        sbit_header.pop();
+    }
+
+    png.raw_data = reduced;
+    png.palette = Some(color_palette);
+    if trans_palette.iter().any(|x| *x != 255) {
+        png.transparency_palette = Some(trans_palette);
+    } else {
+        png.transparency_palette = None;
+    }
+    png.ihdr_data.color_type = ColorType::Indexed;
+    true
 }
 
-pub fn reduce_rgb_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>)> {
+pub fn reduce_rgb_to_palette(png: &mut PngData) -> bool {
     if png.ihdr_data.bit_depth != BitDepth::Eight {
-        return None;
+        return false;
     }
     let mut reduced = Vec::with_capacity(png.raw_data.len());
     let mut palette = Vec::with_capacity(256);
@@ -147,7 +193,7 @@ pub fn reduce_rgb_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>)> {
                 } else {
                     let len = palette.len();
                     if len == 256 {
-                        return None;
+                        return false;
                     }
                     palette.push(cur_pixel);
                     reduced.push(len as u8);
@@ -162,12 +208,29 @@ pub fn reduce_rgb_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>)> {
         color_palette.extend_from_slice(color);
     }
 
-    Some((reduced, color_palette))
+    if let Some(bkgd_header) = png.aux_headers.get_mut(&"bKGD".to_string()) {
+        assert_eq!(bkgd_header.len(), 6);
+        let header_pixels = bkgd_header.iter().skip(1).step(2).cloned().collect::<Vec<u8>>();
+        if let Some(entry) = color_palette.chunks(3).position(|x| x == header_pixels.as_slice()) {
+            *bkgd_header = vec![entry as u8];
+        } else if color_palette.len() == 255 {
+            return false;
+        } else {
+            let entry = color_palette.len() / 3;
+            color_palette.extend_from_slice(&header_pixels);
+            *bkgd_header = vec![entry as u8];
+        }
+    }
+
+    png.raw_data = reduced;
+    png.palette = Some(color_palette);
+    png.ihdr_data.color_type = ColorType::Indexed;
+    true
 }
 
-pub fn reduce_grayscale_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>)> {
+pub fn reduce_grayscale_to_palette(png: &mut PngData) -> bool {
     if png.ihdr_data.bit_depth == BitDepth::Sixteen {
-        return None;
+        return false;
     }
     let mut reduced = BitVec::with_capacity(png.raw_data.len() * 8);
     // Only perform reduction if we can get to 4-bits or less
@@ -192,7 +255,7 @@ pub fn reduce_grayscale_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>)>
                 } else {
                     let len = palette.len();
                     if len == 16 {
-                        return None;
+                        return false;
                     }
                     palette.push(pix_slice);
                     let idx = BitVec::from_bytes(&[(len as u8) << bpp_inverse]);
@@ -214,10 +277,34 @@ pub fn reduce_grayscale_to_palette(png: &PngData) -> Option<(Vec<u8>, Vec<u8>)>
         color_palette.extend_from_slice(color);
     }
 
-    Some((reduced.to_bytes(), color_palette))
+    if let Some(bkgd_header) = png.aux_headers.get_mut(&"bKGD".to_string()) {
+        assert_eq!(bkgd_header.len(), 2);
+        let header_pixels = [bkgd_header[1], bkgd_header[1], bkgd_header[1]];
+        if let Some(entry) = color_palette.chunks(3).position(|x| *x == header_pixels) {
+            *bkgd_header = vec![entry as u8];
+        } else if color_palette.len() == 255 {
+            return false;
+        } else {
+            let entry = color_palette.len() / 3;
+            color_palette.extend_from_slice(&header_pixels);
+            *bkgd_header = vec![entry as u8];
+        }
+    }
+
+    if let Some(sbit_header) = png.aux_headers.get_mut(&"sBIT".to_string()) {
+        assert_eq!(sbit_header.len(), 1);
+        let byte = sbit_header[0];
+        sbit_header.push(byte);
+        sbit_header.push(byte);
+    }
+
+    png.raw_data = reduced.to_bytes();
+    png.palette = Some(color_palette);
+    png.ihdr_data.color_type = ColorType::Indexed;
+    true
 }
 
-pub fn reduce_rgb_to_grayscale(png: &PngData) -> Option<Vec<u8>> {
+pub fn reduce_rgb_to_grayscale(png: &mut PngData) -> bool {
     let mut reduced = Vec::with_capacity(png.raw_data.len());
     let byte_depth: u8 = png.ihdr_data.bit_depth.as_u8() >> 3;
     let bpp: usize = 3 * byte_depth as usize;
@@ -229,7 +316,7 @@ pub fn reduce_rgb_to_grayscale(png: &PngData) -> Option<Vec<u8>> {
             if i % bpp == bpp - 1 {
                 if bpp == 3 {
                     if cur_pixel.iter().unique().count() > 1 {
-                        return None;
+                        return false;
                     }
                     reduced.push(cur_pixel[0]);
                 } else {
@@ -243,7 +330,7 @@ pub fn reduce_rgb_to_grayscale(png: &PngData) -> Option<Vec<u8>> {
                         .unique()
                         .collect::<Vec<(u8, u8)>>();
                     if pixel_bytes.len() > 1 {
-                        return None;
+                        return false;
                     }
                     reduced.push(pixel_bytes[0].0);
                     reduced.push(pixel_bytes[0].1);
@@ -252,15 +339,31 @@ pub fn reduce_rgb_to_grayscale(png: &PngData) -> Option<Vec<u8>> {
             }
         }
     }
+    if let Some(sbit_header) = png.aux_headers.get_mut(&"sBIT".to_string()) {
+        assert_eq!(sbit_header.len(), 3);
+        sbit_header.truncate(1);
+    }
+    if let Some(bkgd_header) = png.aux_headers.get_mut(&"bKGD".to_string()) {
+        assert_eq!(bkgd_header.len(), 6);
+        bkgd_header.truncate(2);
+    }
 
-    Some(reduced)
+    png.raw_data = reduced;
+    png.ihdr_data.color_type = ColorType::Grayscale;
+    true
 }
 
-pub fn reduce_grayscale_alpha_to_grayscale(png: &PngData) -> Option<Vec<u8>> {
+pub fn reduce_grayscale_alpha_to_grayscale(png: &mut PngData) -> bool {
-    reduce_alpha_channel(png, 2)
+    if let Some(reduced) = reduce_alpha_channel(png, 2) {
+        png.raw_data = reduced;
+        png.ihdr_data.color_type = ColorType::Grayscale;
+        true
+    } else {
+        false
+    }
 }
 
-fn reduce_alpha_channel(png: &PngData, bpp_factor: usize) -> Option<Vec<u8>> {
+fn reduce_alpha_channel(png: &mut PngData, bpp_factor: usize) -> Option<Vec<u8>> {
     let mut reduced = Vec::with_capacity(png.raw_data.len());
     let byte_depth: u8 = png.ihdr_data.bit_depth.as_u8() >> 3;
     let bpp: usize = bpp_factor * byte_depth as usize;
@@ -277,6 +380,10 @@ fn reduce_alpha_channel(png: &PngData, bpp_factor: usize) -> Option<Vec<u8>> {
             }
         }
     }
+    if let Some(sbit_header) = png.aux_headers.get_mut(&"sBIT".to_string()) {
+        assert_eq!(sbit_header.len(), bpp_factor);
+        sbit_header.pop();
+    }
 
     Some(reduced)
 }

tests/filters.rs

@@ -1183,20 +1183,3 @@ fn filter_5_for_palette_1() {
                      ColorType::Indexed,
                      BitDepth::One);
 }
-
-#[test]
-fn issue_29() {
-    let input = PathBuf::from("tests/files/issue-29.png");
-    let mut opts = get_opts(&input);
-    opts.filter = HashSet::new();
-    opts.filter.insert(0);
-    let output = opts.out_file.clone();
-
-    test_it_converts(&input,
-                     &output,
-                     &opts,
-                     ColorType::RGB,
-                     BitDepth::Eight,
-                     ColorType::RGB,
-                     BitDepth::Eight);
-}

tests/flags.rs

@@ -491,47 +491,6 @@ fn fix_errors() {
     remove_file(output).ok();
 }
 
-#[test]
-fn issue_42() {
-    let input = PathBuf::from("tests/files/issue_42.png");
-    let mut opts = get_opts(&input);
-    opts.interlace = Some(1);
-    let output = opts.out_file.clone();
-
-    let png = png::PngData::new(&input, opts.fix_errors).unwrap();
-
-    assert_eq!(png.ihdr_data.interlaced, 0);
-    assert_eq!(png.ihdr_data.color_type, ColorType::GrayscaleAlpha);
-    assert_eq!(png.ihdr_data.bit_depth, BitDepth::Eight);
-
-    match oxipng::optimize(&input, &opts) {
-        Ok(_) => (),
-        Err(x) => panic!(x.description().to_owned()),
-    };
-    assert!(output.exists());
-
-    let png = match png::PngData::new(&output, opts.fix_errors) {
-        Ok(x) => x,
-        Err(x) => {
-            remove_file(output).ok();
-            panic!(x.description().to_owned())
-        }
-    };
-
-    assert_eq!(png.ihdr_data.interlaced, 1);
-    assert_eq!(png.ihdr_data.color_type, ColorType::GrayscaleAlpha);
-    assert_eq!(png.ihdr_data.bit_depth, BitDepth::Eight);
-
-    let old_png = image::open(&input).unwrap();
-    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>>>(),
-               new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
-
-    remove_file(output).ok();
-}
-
 #[test]
 fn zopfli_mode() {
     let input = PathBuf::from("tests/files/zopfli_mode.png");

tests/regression.rs

@@ -0,0 +1,223 @@
+extern crate image;
+extern crate oxipng;
+
+use image::GenericImage;
+use image::Pixel;
+use oxipng::colors::{BitDepth, ColorType};
+use oxipng::png;
+use std::collections::HashSet;
+use std::error::Error;
+use std::fs::remove_file;
+use std::path::Path;
+use std::path::PathBuf;
+
+fn get_opts(input: &Path) -> oxipng::Options {
+    let mut options = oxipng::Options::default();
+    options.out_file = input.with_extension("out.png").to_owned();
+    options.verbosity = None;
+    options.force = true;
+    let mut filter = HashSet::new();
+    filter.insert(0);
+    options.filter = filter;
+
+    options
+}
+
+fn test_it_converts(input: &Path,
+                    output: &Path,
+                    opts: &oxipng::Options,
+                    color_type_in: ColorType,
+                    bit_depth_in: BitDepth,
+                    color_type_out: ColorType,
+                    bit_depth_out: BitDepth) {
+    let png = png::PngData::new(input, opts.fix_errors).unwrap();
+
+    assert_eq!(png.ihdr_data.color_type, color_type_in);
+    assert_eq!(png.ihdr_data.bit_depth, bit_depth_in);
+
+    match oxipng::optimize(input, opts) {
+        Ok(_) => (),
+        Err(x) => panic!(x.description().to_owned()),
+    };
+    assert!(output.exists());
+
+    let png = match png::PngData::new(output, opts.fix_errors) {
+        Ok(x) => x,
+        Err(x) => {
+            remove_file(output).ok();
+            panic!(x.description().to_owned())
+        }
+    };
+
+    assert_eq!(png.ihdr_data.color_type, color_type_out);
+    assert_eq!(png.ihdr_data.bit_depth, bit_depth_out);
+
+    let old_png = image::open(input).unwrap();
+    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>>>(),
+               new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
+
+    remove_file(output).ok();
+}
+
+#[test]
+fn issue_29() {
+    let input = PathBuf::from("tests/files/issue-29.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGB,
+                     BitDepth::Eight,
+                     ColorType::RGB,
+                     BitDepth::Eight);
+}
+
+#[test]
+fn issue_42() {
+    let input = PathBuf::from("tests/files/issue_42.png");
+    let mut opts = get_opts(&input);
+    opts.interlace = Some(1);
+    let output = opts.out_file.clone();
+
+    let png = png::PngData::new(&input, opts.fix_errors).unwrap();
+
+    assert_eq!(png.ihdr_data.interlaced, 0);
+    assert_eq!(png.ihdr_data.color_type, ColorType::GrayscaleAlpha);
+    assert_eq!(png.ihdr_data.bit_depth, BitDepth::Eight);
+
+    match oxipng::optimize(&input, &opts) {
+        Ok(_) => (),
+        Err(x) => panic!(x.description().to_owned()),
+    };
+    assert!(output.exists());
+
+    let png = match png::PngData::new(&output, opts.fix_errors) {
+        Ok(x) => x,
+        Err(x) => {
+            remove_file(output).ok();
+            panic!(x.description().to_owned())
+        }
+    };
+
+    assert_eq!(png.ihdr_data.interlaced, 1);
+    assert_eq!(png.ihdr_data.color_type, ColorType::GrayscaleAlpha);
+    assert_eq!(png.ihdr_data.bit_depth, BitDepth::Eight);
+
+    let old_png = image::open(&input).unwrap();
+    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>>>(),
+               new_png.pixels().map(|x| x.2.channels().to_owned()).collect::<Vec<Vec<u8>>>());
+
+    remove_file(output).ok();
+}
+
+#[test]
+fn issue_52_01() {
+    let input = PathBuf::from("tests/files/issue-52-01.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGBA,
+                     BitDepth::Eight,
+                     ColorType::Indexed,
+                     BitDepth::Eight);
+}
+
+#[test]
+fn issue_52_02() {
+    let input = PathBuf::from("tests/files/issue-52-02.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGBA,
+                     BitDepth::Eight,
+                     ColorType::Indexed,
+                     BitDepth::Eight);
+}
+
+#[test]
+fn issue_52_03() {
+    let input = PathBuf::from("tests/files/issue-52-03.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGBA,
+                     BitDepth::Eight,
+                     ColorType::Indexed,
+                     BitDepth::Eight);
+}
+
+#[test]
+fn issue_52_04() {
+    let input = PathBuf::from("tests/files/issue-52-04.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGBA,
+                     BitDepth::Eight,
+                     ColorType::Indexed,
+                     BitDepth::One);
+}
+
+#[test]
+fn issue_52_05() {
+    let input = PathBuf::from("tests/files/issue-52-05.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGBA,
+                     BitDepth::Eight,
+                     ColorType::Indexed,
+                     BitDepth::One);
+}
+
+#[test]
+fn issue_52_06() {
+    let input = PathBuf::from("tests/files/issue-52-06.png");
+    let mut opts = get_opts(&input);
+    opts.filter = HashSet::new();
+    opts.filter.insert(0);
+    let output = opts.out_file.clone();
+
+    test_it_converts(&input,
+                     &output,
+                     &opts,
+                     ColorType::RGBA,
+                     BitDepth::Eight,
+                     ColorType::Indexed,
+                     BitDepth::Two);
+}