Skip to content

fixed the bug concerning png-images with transparency palette#149

Closed
d-schmidt wants to merge 1 commit intopython-pillow:masterfrom
d-schmidt:master
Closed

fixed the bug concerning png-images with transparency palette#149
d-schmidt wants to merge 1 commit intopython-pillow:masterfrom
d-schmidt:master

Conversation

@d-schmidt
Copy link
Copy Markdown
Contributor

Comment thread PIL/PngImagePlugin.py
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a + instead of *? This also removes the ability to store with limited palette size.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean something like im.encoderinfo["transparency"][:2**bits]?
How would that work with a real image?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the bytes/string palette? Yes, but I would keep the alpha_bytes variable to make the code more readable.
For example, you need only 16 colors if you store a paletted image with 4 bits.
I would also swap the if statements and check for int instead of bytes (because transparency might be a string on Python 2).

PS: b'\xff' * 4 is b'\xff\xff\xff\xff'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @olt. Looks like encoderinfo['transparency'] can be an int representing # of bytes, the output type of ImageFile._safe_read (bytes or string), or an int16, or a 3-tuple of int16. Line 557 should be a * instead of +.

If we're round tripping the transparency with no change, then it looks like line 554 should be:

   chunk(fp, b"tRNS", im.encoderinfo["transparency"])

Since we get a bytes/string when we don't find a \0 in the tRNS chunk. To roundtrip, we'd have to emit it as we got it.

(note that I have not checked the spec to see how well this is reflected in the proper format of a png image)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, wait, now I see it. It doesn't work when multiple colors at the end are fully transparent.
^\xff+\x00+$ should be ^\xff+\x00\xff*$. Good catch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we discard it all together and always use all given bytes?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On decoding? Yes, probably. But I would keep support for encoding for backwards compatibility (if a user sets transparency).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was probably originally added to make PNG transparency look like GIF transparency, which has just the one fully transparent color. I'd expect that it's expected behaviour from the users. I'd say keep it, but fix the regex as suggested by @olt.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the fixed pattern to the current open pull.

@d-schmidt
Copy link
Copy Markdown
Contributor Author

contains new error, will send another pull

@d-schmidt d-schmidt closed this Mar 21, 2013
radarhere pushed a commit to radarhere/Pillow that referenced this pull request Sep 24, 2023
radarhere pushed a commit that referenced this pull request Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants