Merge pull request #2347 from radarhere/icc_profile

Moved iCCP chunk before PLTE chunk when saving as PNG

PIL/PngImagePlugin.py

@@ -715,6 +715,32 @@ def _save(im, fp, filename, chunk=putchunk, check=0):
           b'\0',                                # 11: filter category
           b'\0')                                # 12: interlace flag
 
+    chunks = [b"cHRM", b"gAMA", b"sBIT", b"sRGB", b"tIME"]
+
+    icc = im.encoderinfo.get("icc_profile", im.info.get("icc_profile"))
+    if icc:
+        # ICC profile
+        # according to PNG spec, the iCCP chunk contains:
+        # Profile name  1-79 bytes (character string)
+        # Null separator        1 byte (null character)
+        # Compression method    1 byte (0)
+        # Compressed profile    n bytes (zlib with deflate compression)
+        name = b"ICC Profile"
+        data = name + b"\0\0" + zlib.compress(icc)
+        chunk(fp, b"iCCP", data)
+    else:
+        chunks.remove(b"sRGB")
+
+    info = im.encoderinfo.get("pnginfo")
+    if info:
+        chunks_multiple_allowed = [b"sPLT", b"iTXt", b"tEXt", b"zTXt"]
+        for cid, data in info.chunks:
+            if cid in chunks:
+                chunks.remove(cid)
+                chunk(fp, cid, data)
+            elif cid in chunks_multiple_allowed:
+                chunk(fp, cid, data)
+
     if im.mode == "P":
         palette_byte_number = (2 ** bits) * 3
         palette_bytes = im.im.getpalette("RGB")[:palette_byte_number]
@@ -761,20 +787,11 @@ def _save(im, fp, filename, chunk=putchunk, check=0):
 
     info = im.encoderinfo.get("pnginfo")
     if info:
+        chunks = [b"bKGD", b"hIST"]
         for cid, data in info.chunks:
-            chunk(fp, cid, data)
+            if cid in chunks:
-
+                chunks.remove(cid)
-    icc = im.encoderinfo.get("icc_profile", im.info.get("icc_profile"))
+                chunk(fp, cid, data)
-    if icc:
-        # ICC profile
-        # according to PNG spec, the iCCP chunk contains:
-        # Profile name  1-79 bytes (character string)
-        # Null separator        1 byte (null character)
-        # Compression method    1 byte (0)
-        # Compressed profile    n bytes (zlib with deflate compression)
-        name = b"ICC Profile"
-        data = name + b"\0\0" + zlib.compress(icc)
-        chunk(fp, b"iCCP", data)
 
     ImageFile._save(im, _idat(fp, chunk),
                     [("zip", (0, 0)+im.size, 0, rawmode)])

Tests/test_file_png.py

@@ -394,8 +394,8 @@ class TestFilePng(PillowTestCase):
         self.assertIsInstance(im.info["Text"], str)
 
     def test_unicode_text(self):
-        # Check preservation of non-ASCII characters on Python3
+        # Check preservation of non-ASCII characters on Python 3
-        # This cannot really be meaningfully tested on Python2,
+        # This cannot really be meaningfully tested on Python 2,
         # since it didn't preserve charsets to begin with.
 
         def rt_text(value):
@@ -497,6 +497,38 @@ class TestFilePng(PillowTestCase):
         self.assertEqual(repr_png.format, 'PNG')
         self.assert_image_equal(im, repr_png)
 
+    def test_chunk_order(self):
+        im = Image.open("Tests/images/icc_profile.png")
+        test_file = self.tempfile("temp.png")
+        im.convert("P").save(test_file, dpi=(100, 100))
+
+        chunks = []
+        fp = open(test_file, "rb")
+        fp.read(8)
+        png = PngImagePlugin.PngStream(fp)
+        while True:
+            cid, pos, length = png.read()
+            chunks.append(cid)
+            try:
+                s = png.call(cid, pos, length)
+            except EOFError:
+                break
+            png.crc(cid, s)
+
+        # https://www.w3.org/TR/PNG/#5ChunkOrdering
+        # IHDR - shall be first
+        self.assertEqual(chunks.index(b"IHDR"), 0)
+        # PLTE - before first IDAT
+        self.assertLess(chunks.index(b"PLTE"), chunks.index(b"IDAT"))
+        # iCCP - before PLTE and IDAT
+        self.assertLess(chunks.index(b"iCCP"), chunks.index(b"PLTE"))
+        self.assertLess(chunks.index(b"iCCP"), chunks.index(b"IDAT"))
+        # tRNS - after PLTE, before IDAT
+        self.assertGreater(chunks.index(b"tRNS"), chunks.index(b"PLTE"))
+        self.assertLess(chunks.index(b"tRNS"), chunks.index(b"IDAT"))
+        # pHYs - before IDAT
+        self.assertLess(chunks.index(b"pHYs"), chunks.index(b"IDAT"))
+
 
 if __name__ == '__main__':
     unittest.main()