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

Support creating BGR;15, BGR;16 and BGR;24 images, but drop support for BGR;32 #7010

Merged
merged 5 commits into from
Mar 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions Tests/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class TestImage:
"RGBX",
"RGBA",
"RGBa",
"BGR;15",
"BGR;16",
"BGR;24",
"CMYK",
"YCbCr",
"LAB",
Expand All @@ -57,9 +60,7 @@ class TestImage:
def test_image_modes_success(self, mode):
Image.new(mode, (1, 1))

@pytest.mark.parametrize(
"mode", ("", "bad", "very very long", "BGR;15", "BGR;16", "BGR;24", "BGR;32")
)
@pytest.mark.parametrize("mode", ("", "bad", "very very long"))
def test_image_modes_fail(self, mode):
with pytest.raises(ValueError) as e:
Image.new(mode, (1, 1))
Expand Down
14 changes: 7 additions & 7 deletions Tests/test_image_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,8 @@ def test_p_putpixel_rgb_rgba(self, mode):


class TestImagePutPixelError(AccessTest):
IMAGE_MODES1 = ["L", "LA", "RGB", "RGBA"]
IMAGE_MODES2 = ["I", "I;16", "BGR;15"]
IMAGE_MODES1 = ["LA", "RGB", "RGBA", "BGR;15"]
IMAGE_MODES2 = ["L", "I", "I;16"]
Copy link
Member Author

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.

INVALID_TYPES = ["foo", 1.0, None]

@pytest.mark.parametrize("mode", IMAGE_MODES1)
Expand All @@ -369,6 +369,11 @@ def test_putpixel_type_error1(self, mode):
(
("L", (0, 2), "color must be int or single-element tuple"),
("LA", (0, 3), "color must be int, or tuple of one or two elements"),
(
"BGR;15",
(0, 2),
"color must be int, or tuple of one or three elements",
),
(
"RGB",
(0, 2, 5),
Expand Down Expand Up @@ -397,11 +402,6 @@ def test_putpixel_overflow_error(self, mode):
with pytest.raises(OverflowError):
im.putpixel((0, 0), 2**80)

def test_putpixel_unrecognized_mode(self):
im = hopper("BGR;15")
with pytest.raises(ValueError, match="unrecognized image mode"):
im.putpixel((0, 0), 0)
Copy link
Member Author

@radarhere radarhere Mar 19, 2023

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.



class TestEmbeddable:
@pytest.mark.xfail(reason="failing test")
Expand Down
1 change: 0 additions & 1 deletion docs/handbook/concepts.rst
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ Pillow also provides limited support for a few additional modes, including:
* ``BGR;15`` (15-bit reversed true colour)
* ``BGR;16`` (16-bit reversed true colour)
* ``BGR;24`` (24-bit reversed true colour)
* ``BGR;32`` (32-bit reversed true colour)

Premultiplied alpha is where the values for each other channel have been
multiplied by the alpha. For example, an RGBA pixel of ``(10, 20, 30, 127)``
Expand Down
11 changes: 10 additions & 1 deletion docs/releasenotes/9.5.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,18 @@ Added support for saving PDFs in RGBA mode

Using the JPXDecode filter, PDFs can now be saved in RGBA mode.


Improved I;16N support
^^^^^^^^^^^^^^^^^^^^^^

Support has been added for I;16N access, packing and unpacking. Conversion to
and from L mode has also been added.

BGR;* modes
^^^^^^^^^^^

It is now possible to create new BGR;15, BGR;16 and BGR;24 images. Conversely, BGR;32
has been removed from ImageMode and its associated methods, dropping the little support
Pillow had for the mode.

With that, all modes listed under :ref:`concept-modes` can now be used to create a new
image.
7 changes: 3 additions & 4 deletions src/PIL/ImageMode.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,9 @@ def getmode(mode):
"HSV": ("RGB", "L", ("H", "S", "V"), "|u1"),
# extra experimental modes
"RGBa": ("RGB", "L", ("R", "G", "B", "a"), "|u1"),
"BGR;15": ("RGB", "L", ("B", "G", "R"), endian + "u2"),
"BGR;16": ("RGB", "L", ("B", "G", "R"), endian + "u2"),
"BGR;24": ("RGB", "L", ("B", "G", "R"), endian + "u3"),
"BGR;32": ("RGB", "L", ("B", "G", "R"), endian + "u4"),
"BGR;15": ("RGB", "L", ("B", "G", "R"), "|u1"),
"BGR;16": ("RGB", "L", ("B", "G", "R"), "|u1"),
"BGR;24": ("RGB", "L", ("B", "G", "R"), "|u1"),
"LA": ("L", "L", ("L", "A"), "|u1"),
"La": ("L", "L", ("L", "a"), "|u1"),
"PA": ("RGB", "L", ("P", "A"), "|u1"),
Expand Down
60 changes: 48 additions & 12 deletions src/_imaging.c
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ getink(PyObject *color, Imaging im, char *ink) {
int g = 0, b = 0, a = 0;
double f = 0;
/* Windows 64 bit longs are 32 bits, and 0xFFFFFFFF (white) is a
python long (not int) that raises an overflow error when trying
Python long (not int) that raises an overflow error when trying
to return it into a 32 bit C long
*/
PY_LONG_LONG r = 0;
Expand All @@ -502,8 +502,12 @@ getink(PyObject *color, Imaging im, char *ink) {
be cast to either UINT8 or INT32 */

int rIsInt = 0;
if (PyTuple_Check(color) && PyTuple_GET_SIZE(color) == 1) {
color = PyTuple_GetItem(color, 0);
int tupleSize;
if (PyTuple_Check(color)) {
tupleSize = PyTuple_GET_SIZE(color);
if (tupleSize == 1) {
color = PyTuple_GetItem(color, 0);
}
}
if (im->type == IMAGING_TYPE_UINT8 || im->type == IMAGING_TYPE_INT32 ||
im->type == IMAGING_TYPE_SPECIAL) {
Expand All @@ -513,15 +517,13 @@ getink(PyObject *color, Imaging im, char *ink) {
return NULL;
}
rIsInt = 1;
} else if (im->type == IMAGING_TYPE_UINT8) {
if (!PyTuple_Check(color)) {
PyErr_SetString(PyExc_TypeError, "color must be int or tuple");
return NULL;
}
} else {
} else if (im->bands == 1) {
PyErr_SetString(
PyExc_TypeError, "color must be int or single-element tuple");
return NULL;
} else if (!PyTuple_Check(color)) {
PyErr_SetString(PyExc_TypeError, "color must be int or tuple");
return NULL;
}
}

Expand All @@ -531,7 +533,7 @@ getink(PyObject *color, Imaging im, char *ink) {
if (im->bands == 1) {
/* unsigned integer, single layer */
if (rIsInt != 1) {
if (PyTuple_GET_SIZE(color) != 1) {
if (tupleSize != 1) {
PyErr_SetString(PyExc_TypeError, "color must be int or single-element tuple");
return NULL;
} else if (!PyArg_ParseTuple(color, "L", &r)) {
Expand All @@ -541,15 +543,14 @@ getink(PyObject *color, Imaging im, char *ink) {
ink[0] = (char)CLIP8(r);
ink[1] = ink[2] = ink[3] = 0;
} else {
a = 255;
if (rIsInt) {
/* compatibility: ABGR */
a = (UINT8)(r >> 24);
b = (UINT8)(r >> 16);
g = (UINT8)(r >> 8);
r = (UINT8)r;
} else {
int tupleSize = PyTuple_GET_SIZE(color);
a = 255;
if (im->bands == 2) {
if (tupleSize != 1 && tupleSize != 2) {
PyErr_SetString(PyExc_TypeError, "color must be int, or tuple of one or two elements");
Expand Down Expand Up @@ -593,6 +594,41 @@ getink(PyObject *color, Imaging im, char *ink) {
ink[1] = (UINT8)(r >> 8);
ink[2] = ink[3] = 0;
return ink;
} else {
if (rIsInt) {
b = (UINT8)(r >> 16);
g = (UINT8)(r >> 8);
r = (UINT8)r;
} else if (tupleSize != 3) {
PyErr_SetString(PyExc_TypeError, "color must be int, or tuple of one or three elements");
return NULL;
} else if (!PyArg_ParseTuple(color, "Lii", &r, &g, &b)) {
return NULL;
}
if (!strcmp(im->mode, "BGR;15")) {
UINT16 v = ((((UINT16)r) << 7) & 0x7c00) +
((((UINT16)g) << 2) & 0x03e0) +
((((UINT16)b) >> 3) & 0x001f);

ink[0] = (UINT8)v;
ink[1] = (UINT8)(v >> 8);
ink[2] = ink[3] = 0;
return ink;
} else if (!strcmp(im->mode, "BGR;16")) {
UINT16 v = ((((UINT16)r) << 8) & 0xf800) +
((((UINT16)g) << 3) & 0x07e0) +
((((UINT16)b) >> 3) & 0x001f);
ink[0] = (UINT8)v;
ink[1] = (UINT8)(v >> 8);
ink[2] = ink[3] = 0;
return ink;
} else if (!strcmp(im->mode, "BGR;24")) {
ink[0] = (UINT8)b;
ink[1] = (UINT8)g;
ink[2] = (UINT8)r;
ink[3] = 0;
return ink;
}
}
}

Expand Down
14 changes: 3 additions & 11 deletions src/libImaging/Storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,35 +131,27 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size) {
} else if (strcmp(mode, "BGR;15") == 0) {
/* EXPERIMENTAL */
/* 15-bit reversed true colour */
im->bands = 1;
im->bands = 3;
im->pixelsize = 2;
im->linesize = (xsize * 2 + 3) & -4;
im->type = IMAGING_TYPE_SPECIAL;

} else if (strcmp(mode, "BGR;16") == 0) {
/* EXPERIMENTAL */
/* 16-bit reversed true colour */
im->bands = 1;
im->bands = 3;
im->pixelsize = 2;
im->linesize = (xsize * 2 + 3) & -4;
im->type = IMAGING_TYPE_SPECIAL;

} else if (strcmp(mode, "BGR;24") == 0) {
/* EXPERIMENTAL */
/* 24-bit reversed true colour */
im->bands = 1;
im->bands = 3;
im->pixelsize = 3;
im->linesize = (xsize * 3 + 3) & -4;
im->type = IMAGING_TYPE_SPECIAL;

} else if (strcmp(mode, "BGR;32") == 0) {
/* EXPERIMENTAL */
/* 32-bit reversed true colour */
im->bands = 1;
im->pixelsize = 4;
im->linesize = (xsize * 4 + 3) & -4;
im->type = IMAGING_TYPE_SPECIAL;

} else if (strcmp(mode, "RGBX") == 0) {
/* 32-bit true colour images with padding */
im->bands = im->pixelsize = 4;
Expand Down