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
Indexes of colors when reading a GIMP Palette file are wrong #6639
Comments
Since #5552, this is actually no longer the case.
@hugovk you added custom_gimp_palette.gpl in #1326. Any thoughts? |
If I artificially fill the palette up to 256 colors with Python code, custom_gimp_palette.gpl.zip, then the following script generates the results I would expect when reading it. from PIL import _binary
from PIL import GimpPaletteFile
with open("Tests/images/custom_gimp_palette.gpl", "rb") as fp:
palette = GimpPaletteFile.GimpPaletteFile(fp).getpalette()[0]
for i in range(256):
print(tuple(_binary.i8(x) for x in palette[i*3:i*3+3])) So I'm unable to replicate. Could you provide a file to demonstrate this problem? |
Interesting. I could make GimpPaletteFile create an instance of ImagePalette instead of the raw 768 bytes it produces right now. Would that be interesting? |
The file bellow comes with any GIMP 2.x installation (I can attach it later):
So, the last Python expressions prints the last 4 colors in the file, and it can clearly be seen that the This is the output with the code in the PR:
|
GitHub seems to restrict the files that can be attached to more common formats. |
Ok, I see what you're saying about colors being skipped now. Fields and comment lines count towards the 256 total. I guess I was mistaken in my last comment. Thanks for linking to that. |
Anyway, I think the code as it is is fine for fixing this - but I will be fine in upgrading it to return ImagePalette right now (I think long term that is the way to go, anyway). |
It's the same as the one at https://stackoverflow.com/a/815855/724176, I guess from lack of other test files available I grabbed that one as a basic file. We can definitely replace it with a "real" one. |
Oops - I think we just hit a case of https://xkcd.com/978/ . :-) Further palette functionality is something I intend to colaborate on (as on animated image support), and larger palette files could be included later. |
It has been revealed that GIMP palette files can contain more than 256 colors, and requested that we read all of those colors, even though would make for an invalid Pillow palette. I'm reluctant to do so, as I don't want to mislead users into thinking that they have a palette that can be used without a problem elsewhere in Pillow. I thought perhaps instead an argument could be added, |
sounds good. I will merge your modifications and add further changes so we can get both a way to get the full palette and by default limit it at 256 colors. Actually, having the full palette could be postponed, and we could draw a roadmap for a higher level 'ImagePalette' that could hold then. I got an idea: what about instead of adding an arg to . ' getpalette' just set the maximum number of colors in a class attribute? sounds simple, and will at once mitigate unlimited files and allow whoever know what they are doing to read larger palettes if needed. |
Finishing my thought, I've realised that this code comment supports not reading an unlimited amount of data. Pillow/src/PIL/ImtImagePlugin.py Line 66 in 243402e
|
To my mind, adding an argument to a method makes for a simpler API than adding an attribute to control the output of a method. |
I can confirm this bug in my current project. I have an indexed image in GIMP, type palette RGB with 20 colors. I export it as PNG and open it with Pillow 8.4.0. When I view the palette PIL has extracted, it contains 21 colors; the first is a random color, that is not present in the image, and following that are the 20 GIMP colors in the right order. I suspect, that somewhere the pointer pointing at the palette byte-array is wrongly placed, and that the first color is garbage. The result of this is: when you save the image from Pillow to a .png file, and open it in GIMP all colors are shifted one place to the left. The image is unusable. Please fix. |
Re last comment: I inspected the raw PNG image data with a hex editor, and the "garbage" color is actually present in the PLTE chunk in the PNG file, as color[0]. So this seems to be a GIMP problem, rather than a PIL problem. Sorry for blaming PIL. I will try to find the cause with GIMP, as I am running a rather old version. Maybe this can help solve the above problem. |
At PIL root, in an iPython prompt do:
The results are
Inspecting the bytes generated by reading the file, it is possible to see that the 3 first colors in the palette do not correspond to the 3 first colors in the file (the first does, but it is (0, 0, 0) by a coincidence). Rather, the colors specified in the palette file
show up from index 3 on.
Strangely, that is even sorted of "documented" in the color names in the existing sample file, but it is plain wrong, as loading that file in GIMP will assign index 0 to the color currently named "index 3" in custom_gimp_palette.gpl" and so on. Although the behavior is acknowledged, it is straightforward wrong, as one can't replicate the colors from an original GIMP palette file by inspecting the palette created by this loader alone - one has to be aware of this quirk instead.
Moreover, the current implementation will simply ignore the last 3 colors in a 256 GIMP GPL file.
As an additional feature, although the created palette has always 256 colors as needed in other PIL structures using Palettes, one has no way to know which of these colors are from the original file, and which are from the filer gray gradient that is created by default. Annotating the number of colors in the instance of the palette reader is essentially free, so it can be put there should anyone actually want to use this.
The text was updated successfully, but these errors were encountered: