Note the sRGB color space in PNG and APNG output - #2389
Open
robto09 wants to merge 2 commits into
Open
Conversation
Paparazzi writes PNG and APNG snapshots whose pixels are in the sRGB color space, but the files never declared it. As the first step toward wide color screenshots, write the PNG sRGB chunk with the perceptual rendering intent so every decoder knows the image data is sRGB. The sRGB chunk goes after the header chunks and before the image data, which is where the PNG specification requires it. The APNG reader now accepts the chunk and skips its single payload byte, so files written by the updated writer round trip cleanly. Refs: cashapp#40
Assert that a static PNG written by ApngWriter carries an sRGB chunk with the perceptual rendering intent between IHDR and IDAT, and that animated output places the chunk before IDAT as well. Also verify the APNG reader decodes a file written by the updated writer.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Paparazzi writes snapshot images as PNG and APNG files whose pixels are in the sRGB color space, but the files never declared it. Decoders have to guess the color space of every snapshot. I made the color space explicit by writing the PNG
sRGBchunk with the Perceptual rendering intent, as the first step toward wide color screenshots (see #40).What I changed
PngConstants: added theSRGBchunk header and a constant for the Perceptual rendering intent (value 0).ApngWriter: every PNG now carries ansRGBchunk with a one byte payload. I placed the chunk after the header chunks (IHDR, and acTL/fcTL for animated output) and before IDAT, which is where the PNG specification requires it.ApngReader: I taught the reader to accept thesRGBchunk and skip its payload, so files written by the updated writer round trip cleanly. Without this branch the code does not compile, because the chunk dispatcher requires exhaustive handling of every known chunk.CHANGELOG.md: added an entry under Unreleased.Why a real chunk and not a workaround
I used the
sRGBchunk because it is the standard PNG mechanism for declaring the color space of image data, defined in the PNG 1.2 specification. Any conforming decoder, includingImageIOand Android'sBitmapFactory, understands it. This keeps every snapshot self-describing and gives us a solid baseline when we add wide color gamut support later.How I tested it
ApngWriterTest.writesSRGBColorSpaceChunkAfterIHDR, which asserts the chunk id, its position between IHDR and IDAT, the Perceptual intent byte, and a valid CRC.ApngReaderTest.decodesPNGWithSRGBColorSpaceChunk, which writes a PNG with the updated writer and decodes it with the reader.:paparazzi:testsuite: 126 tests, 0 failures.IHDR, sRGB, IDAT, IENDwith intent byte 0 and valid CRCs, and macOSsipsdecodes the file.Follow ups
delta-*.pngdiagnostics written viaImageIO.writeinImageUtilsare a separate output path and I left them unchanged; they can get the same treatment later.ApngReaderbecauseHeader.valueOfthrows for chunk types outside the enum; a graceful skip for unknown chunks would be a good follow up.Fixes #40