Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only use colors from the palette file #1

Merged
merged 1 commit into from Oct 6, 2022

Conversation

radarhere
Copy link

Suggestion for python-pillow#6640

As mentioned previously, python-pillow#5552 means that palettes don't need to have 256 colors anymore, so this changes GimpPaletteFile to only use colors from the palette file, removing the default palette.

That also means that n_colors is no longer necessary. If the user wants to know how many colors there are, len(palette_file.getpalette()[0]) // 3 can be used.

Copy link
Owner

@jsbueno jsbueno left a comment

Choose a reason for hiding this comment

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

can we just read-in an arbitrary number of colors, as allowed for GPL files used by GIMP?
palette_257
(attached GIMP screenshot showing color with index 260 selected in a custom 512 color palette file)

self.palette = b""
while len(self.palette) < 768:

s = fp.readline()
Copy link
Owner

Choose a reason for hiding this comment

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

Actually, GIMP GPL files are not limited to 256 colors - a file with more than 256 colors is a valid resource: althoug indexed images can only use this many colors, the resources in the app can also function as color source for various painting tools and filters, which does not have this limitation.

Reading to the file end makes more sense - and later on we adjust the higher level ImagePalette to deal (or error) with larger palettes.

Copy link
Owner

@jsbueno jsbueno left a comment

Choose a reason for hiding this comment

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

I will merge this and add a '.max_colors' class attribute. And also, take in acoount that a valid palette file might have comment lines in its body (I will have to revert to reading the file indefinetelly - and add some other internal state to stop a DoS on a malformed file.

@jsbueno jsbueno merged commit 1ed6039 into jsbueno:fix/gimppalette Oct 6, 2022
@radarhere radarhere deleted the gimppalette branch October 6, 2022 21:04
radarhere pushed a commit that referenced this pull request Dec 30, 2022
radarhere pushed a commit that referenced this pull request Dec 30, 2022
radarhere pushed a commit that referenced this pull request Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants