Fix loading transparent XPM images - #9848
Open
nyxst4ck wants to merge 1 commit into
Open
Conversation
The 11.3.0 palette refactor stopped adding an entry for 'c None' colours, so palette.index(key) raised ValueError and any XPM with a transparent colour failed to load. Give the transparent colour a real palette entry (black, matching pre-11.3.0 rendering) and store its palette index (P mode) or colour tuple (RGB mode) in info['transparency'].
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.
Any XPM that declares a transparent colour via a
c Noneentry — the standard way GIMP and ImageMagick store transparency — fails to load since 11.3.0:Root cause
#8967's palette refactor (7b459a8) replaced the fixed 256-entry black-filled palette list with a dict of only the declared colours, but the
c Nonebranch stores the character key ininfo["transparency"]without adding a palette entry. Decoding then callspalette_keys.index(key)for the transparent character and raises. The>256-colour RGB path (added in 395bd6b) hits the same missing entry.Before 11.3.0 this worked, if partly by accident: the transparent character's ordinal was a valid index into the black-filled palette, so the image loaded and
info["transparency"]was a usable integer index. Verified: 11.2.1 loads (transparency=32), 11.3.0 / 12.2.0 / current main all raise.Fix
Give the transparent colour a real palette entry of black — matching pre-11.3.0 rendering — and store a usable value in
info["transparency"]: the palette index in P mode, the colour tuple in RGB mode.One design point I'd like your view on: for the RGB path,
info["transparency"] = (0, 0, 0)follows the usual colour-key convention but would also key out genuine black pixels in a >256-colour transparent XPM. If you'd rather only fix the crash there and leave transparency unreported in RGB mode, I'm happy to drop that line and the corresponding assertion.Tests
Two tests added to
Tests/test_file_xpm.py, building XPM data in memory withBytesIO(same style astest_truncated_header) — no new binary fixtures:test_transparency: P mode; asserts the palette index ininfo["transparency"], and thatconvert("RGBA")yields(0, 0, 0, 0)for the transparent pixel and the correct opaque colours. Transparency also survives a PNG save/reload round-trip.test_transparency_rgb: 301 colours → RGB mode; asserts mode, transparency tuple and pixel values.Red on main (both fail with the error above), green with the fix (10 passed).
ruffandblackclean on both files;mypy src/PIL/XpmImagePlugin.pyclean.