-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Support creating BGR;15, BGR;16 and BGR;24 images, but drop support for BGR;32 #7010
Conversation
IMAGE_MODES1 = ["L", "LA", "RGB", "RGBA"] | ||
IMAGE_MODES2 = ["I", "I;16", "BGR;15"] | ||
IMAGE_MODES1 = ["LA", "RGB", "RGBA"] | ||
IMAGE_MODES2 = ["L", "I", "I;16"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later in this file, IMAGE_MODES2
are tested to raise "color must be int or single-element tuple", while IMAGE_MODES1
are tested to raise "color must be int or tuple". This makes sense, as all IMAGE_MODES2
are single channel, while IMAGE_MODES1
have multiple channels.
def test_putpixel_unrecognized_mode(self): | ||
im = hopper("BGR;15") | ||
with pytest.raises(ValueError, match="unrecognized image mode"): | ||
im.putpixel((0, 0), 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error came from converting an image to a mode that putpixel
was unable to getink
with. However, all modes that an image can be converted to are now supported by getink
after this PR.
The error should still be in C in case we break something in the future, but I don't think it can be tested.
Let's also include this in release notes. |
#6769 (comment) mentioned that creating BGR images does not work.
Image.new("BGR;15", (1, 1))
does not work, and neither does BGR;16, BGR;24 or BGR;32.So what are these modes?
Pillow/src/libImaging/Unpack.c
Lines 691 to 693 in 3b4b77b
Pillow/src/libImaging/Unpack.c
Lines 736 to 738 in 3b4b77b
So with that pattern, BGR;24 would allow 8 bits per channel, the same size as an RGB channel. And yes, when converting, the values are just reversed.
Pillow/src/libImaging/Convert.c
Lines 285 to 291 in 3b4b77b
So this PR adds support for creating images with those three image modes.
Continuing on, with that pattern, one could imagine that BGR;32 is a mode that supports 10 or 11-bit values for each channel.
However, there isn't any concrete information about that. Pillow doesn't currently support converting images to BGR;32, nor does it support any kind of unpacking with this mode.
In fact, the only code about this mode is
Pillow/src/libImaging/Storage.c
Lines 155 to 161 in 079caf6
which I think can't be accessed through our Python API without other support for the mode, and
Pillow/src/PIL/ImageMode.py
Line 64 in 079caf6
which doesn't offer any value to the user if nothing else can be done with this mode.
So if ImageMode has the only functionality that would change for users (and it has only been in there since #5935), I'm going to suggest just dropping support for BGR;32 without deprecation - there isn't any demonstrated need for BGR;32, and we would just be inventing a mode by trying to specify what it is.