Decode palette PNGs instead of dropping them - #19
Merged
Conversation
decode_image mapped PNG colour types to a channel count and sent anything unrecognised to a catch-all that returned None. Colour type 3, indexed colour, landed there. Every caller reads None as nothing to draw, so an indexed PNG was silently omitted from both PDF and PNG output with no warning anywhere. These are not unusual. Most tools default to a palette when an image has few colours, and Word embeds whatever it is given. The file attached to #3 carries a 640x480 8-bit colormap PNG, which is why that document appeared to have no image even after the anchor position was corrected in #10. Colour type 3 is now handled: PLTE supplies the palette, tRNS supplies per-entry alpha when present, and each index is expanded to RGB or RGBA after unfiltering. An index past the end of the palette is malformed input, so it draws black rather than throwing the whole image away, and a missing PLTE is still rejected. Fully opaque images carry no alpha channel, matching what the RGBA path already does so PDF viewers do not get an unnecessary SMask. Bit depths below 8 are still rejected by the existing bit_depth check, so sub-byte indexed images remain unsupported rather than wrong. Closes #17.
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.
Closes #17.
The bug
decode_imagemapped PNG colour types to a channel count and sent anything unrecognised to a catch-all:Colour type 3, indexed colour, landed there. Every caller treats
Noneas nothing to draw, so an indexed PNG was silently omitted from both PDF and PNG output.These are not unusual. Most tools default to a palette when an image has few colours, and Word embeds whatever it is handed. The file attached to #3 carries a 640x480 8-bit colormap PNG, which is why that document looked like it had no image even after #10 corrected the anchor position.
The fix
PLTEsupplies the palette andtRNSsupplies per-entry alpha when present. Each index is expanded to RGB or RGBA after unfiltering, which is where the existing code already has a clean one-byte-per-pixel scanline to work from.Three decisions worth calling out:
PLTEis still rejected, because there is nothing to expand against.Bit depths below 8 are still rejected by the existing
bit_depth != 8check. Sub-byte indexed images stay unsupported rather than silently wrong, and that is now a deliberate limit rather than an accident of the catch-all.Verified
Four tests build indexed PNGs from scratch and cover palette expansion,
tRNSalpha including entries it does not mention, the missing palette case, and the out-of-range index case.Rendering the file from #3 with this and #18 applied together, the image finally appears, in the right place, with its transparency intact. The page went from 19 KB to 70 KB of PNG output.
Checks
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warningsand the full suite all pass. 347 tests, up from 343, with four added here.python3 scripts/hash_harness.py --checkalso passes, 28 entries matching. Worth stating plainly why: none of the harness fixtures contains an indexed PNG, so the baselines are genuinely unaffected. This change does alter output for any document that does contain one, which is the entire point of it.