From 9e6ae98362c174febada3f8a06d1965524451cf8 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 13 Mar 2023 17:31:12 +1100 Subject: [PATCH 1/4] Dropped support for BGR;32 mode --- Tests/test_image.py | 2 +- docs/handbook/concepts.rst | 1 - src/PIL/ImageMode.py | 1 - src/libImaging/Storage.c | 8 -------- 4 files changed, 1 insertion(+), 11 deletions(-) diff --git a/Tests/test_image.py b/Tests/test_image.py index 85e3ff55b..c22628509 100644 --- a/Tests/test_image.py +++ b/Tests/test_image.py @@ -58,7 +58,7 @@ class TestImage: Image.new(mode, (1, 1)) @pytest.mark.parametrize( - "mode", ("", "bad", "very very long", "BGR;15", "BGR;16", "BGR;24", "BGR;32") + "mode", ("", "bad", "very very long", "BGR;15", "BGR;16", "BGR;24") ) def test_image_modes_fail(self, mode): with pytest.raises(ValueError) as e: diff --git a/docs/handbook/concepts.rst b/docs/handbook/concepts.rst index 0aa2f1119..e40ed4687 100644 --- a/docs/handbook/concepts.rst +++ b/docs/handbook/concepts.rst @@ -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)`` diff --git a/src/PIL/ImageMode.py b/src/PIL/ImageMode.py index 0973536c9..8b1506e9b 100644 --- a/src/PIL/ImageMode.py +++ b/src/PIL/ImageMode.py @@ -61,7 +61,6 @@ def getmode(mode): "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"), "LA": ("L", "L", ("L", "A"), "|u1"), "La": ("L", "L", ("L", "a"), "|u1"), "PA": ("RGB", "L", ("P", "A"), "|u1"), diff --git a/src/libImaging/Storage.c b/src/libImaging/Storage.c index 76750aaf7..7730b4be8 100644 --- a/src/libImaging/Storage.c +++ b/src/libImaging/Storage.c @@ -152,14 +152,6 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size) { 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; From 01cdfb6b27ed857fc63e625514a97352c5dc89cf Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sun, 19 Mar 2023 23:20:54 +1100 Subject: [PATCH 2/4] Removed duplicate calls to PyTuple_GET_SIZE --- src/_imaging.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/_imaging.c b/src/_imaging.c index 1c25ab00c..e7e403c95 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -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; @@ -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) { @@ -531,7 +535,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)) { @@ -541,7 +545,6 @@ 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); @@ -549,7 +552,7 @@ getink(PyObject *color, Imaging im, char *ink) { 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"); From 11d100ce5deb6028ca91633e75a01d648dc936c5 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 20 Mar 2023 00:30:10 +1100 Subject: [PATCH 3/4] Support creating BGR;15, BGR;16 and BGR;24 images --- Tests/test_image.py | 7 +++--- Tests/test_image_access.py | 14 ++++++------ src/PIL/ImageMode.py | 6 ++--- src/_imaging.c | 45 +++++++++++++++++++++++++++++++++----- src/libImaging/Storage.c | 6 ++--- 5 files changed, 56 insertions(+), 22 deletions(-) diff --git a/Tests/test_image.py b/Tests/test_image.py index c22628509..17f1edb00 100644 --- a/Tests/test_image.py +++ b/Tests/test_image.py @@ -48,6 +48,9 @@ class TestImage: "RGBX", "RGBA", "RGBa", + "BGR;15", + "BGR;16", + "BGR;24", "CMYK", "YCbCr", "LAB", @@ -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") - ) + @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)) diff --git a/Tests/test_image_access.py b/Tests/test_image_access.py index 4079d9358..29eed745e 100644 --- a/Tests/test_image_access.py +++ b/Tests/test_image_access.py @@ -363,8 +363,8 @@ class TestCffi(AccessTest): 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"] INVALID_TYPES = ["foo", 1.0, None] @pytest.mark.parametrize("mode", IMAGE_MODES1) @@ -379,6 +379,11 @@ class TestImagePutPixelError(AccessTest): ( ("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), @@ -407,11 +412,6 @@ class TestImagePutPixelError(AccessTest): 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) - class TestEmbeddable: @pytest.mark.xfail(reason="failing test") diff --git a/src/PIL/ImageMode.py b/src/PIL/ImageMode.py index 8b1506e9b..a0b335142 100644 --- a/src/PIL/ImageMode.py +++ b/src/PIL/ImageMode.py @@ -58,9 +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;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"), diff --git a/src/_imaging.c b/src/_imaging.c index e7e403c95..3b17d638e 100644 --- a/src/_imaging.c +++ b/src/_imaging.c @@ -517,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; } } @@ -596,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; + } } } diff --git a/src/libImaging/Storage.c b/src/libImaging/Storage.c index 7730b4be8..7cf00ef35 100644 --- a/src/libImaging/Storage.c +++ b/src/libImaging/Storage.c @@ -131,7 +131,7 @@ 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; @@ -139,7 +139,7 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size) { } 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; @@ -147,7 +147,7 @@ ImagingNewPrologueSubtype(const char *mode, int xsize, int ysize, int size) { } 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; From a236c272a801b7d7feab591412745656b1b88fc4 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 25 Mar 2023 23:41:27 +1100 Subject: [PATCH 4/4] Added release notes --- docs/releasenotes/9.5.0.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/releasenotes/9.5.0.rst b/docs/releasenotes/9.5.0.rst index bd6e58693..849e87594 100644 --- a/docs/releasenotes/9.5.0.rst +++ b/docs/releasenotes/9.5.0.rst @@ -63,3 +63,13 @@ Added support for saving PDFs in RGBA mode ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Using the JPXDecode filter, PDFs can now be saved in RGBA mode. + +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.