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 BGR;15, BGR;16 and BGR;24 access, unpacking and putdata #7303

Merged
merged 4 commits into from
Oct 6, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions Tests/test_image_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,16 @@ def color(mode):
bands = Image.getmodebands(mode)
if bands == 1:
return 1
if mode in ("BGR;15", "BGR;16"):
# These modes have less than 8 bits per band
# So (1, 2, 3) cannot be roundtripped
return (16, 32, 49)
return tuple(range(1, bands + 1))

def check(self, mode, expected_color=None):
if self._need_cffi_access and mode.startswith("BGR;"):
pytest.skip("Support not added to deprecated module for BGR;* modes")

if not expected_color:
expected_color = self.color(mode)

Expand Down Expand Up @@ -203,6 +210,9 @@ def check(self, mode, expected_color=None):
"F",
"P",
"PA",
"BGR;15",
"BGR;16",
"BGR;24",
"RGB",
"RGBA",
"RGBX",
Expand Down
9 changes: 9 additions & 0 deletions Tests/test_image_putdata.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ def test_mode_F():
assert list(im.getdata()) == target


@pytest.mark.parametrize("mode", ("BGR;15", "BGR;16", "BGR;24"))
def test_mode_BGR(mode):
data = [(16, 32, 49), (32, 32, 98)]
im = Image.new(mode, (1, 2))
im.putdata(data)

assert list(im.getdata()) == data


def test_array_B():
# shouldn't segfault
# see https://github.com/python-pillow/Pillow/issues/1008
Expand Down
7 changes: 7 additions & 0 deletions Tests/test_lib_pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,13 @@ def test_RGB(self):
"RGB", "CMYK", 4, (250, 249, 248), (242, 241, 240), (234, 233, 233)
)

def test_BGR(self):
self.assert_unpack("BGR;15", "BGR;15", 3, (8, 131, 0), (24, 0, 8), (41, 131, 8))
self.assert_unpack(
"BGR;16", "BGR;16", 3, (8, 64, 0), (24, 129, 0), (41, 194, 0)
)
self.assert_unpack("BGR;24", "BGR;24", 3, (1, 2, 3), (4, 5, 6), (7, 8, 9))

def test_RGBA(self):
self.assert_unpack("RGBA", "LA", 2, (1, 1, 1, 2), (3, 3, 3, 4), (5, 5, 5, 6))
self.assert_unpack(
Expand Down
55 changes: 41 additions & 14 deletions src/_imaging.c
Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,10 @@
case IMAGING_TYPE_FLOAT32:
return PyFloat_FromDouble(pixel.f);
case IMAGING_TYPE_SPECIAL:
if (strncmp(im->mode, "I;16", 4) == 0) {
if (im->bands == 1) {
return PyLong_FromLong(pixel.h);
} else {
return Py_BuildValue("BBB", pixel.b[0], pixel.b[1], pixel.b[2]);
}
break;
}
Expand Down Expand Up @@ -599,7 +601,7 @@
} 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)) {
} else if (!PyArg_ParseTuple(color, "iiL", &b, &g, &r)) {
return NULL;
}
if (!strcmp(im->mode, "BGR;15")) {
Expand Down Expand Up @@ -1571,21 +1573,46 @@
PyErr_SetString(PyExc_TypeError, must_be_sequence);
return NULL;
}
int endian = strncmp(image->mode, "I;16", 4) == 0 ? (strcmp(image->mode, "I;16B") == 0 ? 2 : 1) : 0;
double value;
for (i = x = y = 0; i < n; i++) {
set_value_to_item(seq, i);
if (scale != 1.0 || offset != 0.0) {
value = value * scale + offset;
if (image->bands == 1) {
int bigendian;
if (image->type == IMAGING_TYPE_SPECIAL) {
// I;16*
bigendian = strcmp(image->mode, "I;16B") == 0;
}
if (endian == 0) {
image->image8[y][x] = (UINT8)CLIP8(value);
} else {
image->image8[y][x * 2 + (endian == 2 ? 1 : 0)] = CLIP8((int)value % 256);
image->image8[y][x * 2 + (endian == 2 ? 0 : 1)] = CLIP8((int)value >> 8);
for (i = x = y = 0; i < n; i++) {
set_value_to_item(seq, i);
if (scale != 1.0 || offset != 0.0) {
value = value * scale + offset;
}
if (image->type == IMAGING_TYPE_SPECIAL) {
image->image8[y][x * 2 + (bigendian ? 1 : 0)] = CLIP8((int)value % 256);
image->image8[y][x * 2 + (bigendian ? 0 : 1)] = CLIP8((int)value >> 8);
} else {
image->image8[y][x] = (UINT8)CLIP8(value);
}
if (++x >= (int)image->xsize) {
x = 0, y++;
}
}
if (++x >= (int)image->xsize) {
x = 0, y++;
} else {
// BGR;*
int b;
for (i = x = y = 0; i < n; i++) {
char ink[4];

op = PySequence_Fast_GET_ITEM(seq, i);
if (!op || !getink(op, image, ink)) {
Py_DECREF(seq);
return NULL;

Check warning on line 1607 in src/_imaging.c

View check run for this annotation

Codecov / codecov/patch

src/_imaging.c#L1606-L1607

Added lines #L1606 - L1607 were not covered by tests
}
/* FIXME: what about scale and offset? */
Copy link
Member Author

Choose a reason for hiding this comment

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

This is copying an existing FIXME

/* FIXME: what about scale and offset? */

for (b = 0; b < image->pixelsize; b++) {
image->image8[y][x * image->pixelsize + b] = ink[b];
}
if (++x >= (int)image->xsize) {
x = 0, y++;
}
}
}
PyErr_Clear(); /* Avoid weird exceptions */
Expand Down
42 changes: 40 additions & 2 deletions src/libImaging/Access.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include "Imaging.h"

/* use make_hash.py from the pillow-scripts repository to calculate these values */
#define ACCESS_TABLE_SIZE 27
#define ACCESS_TABLE_HASH 33051
#define ACCESS_TABLE_SIZE 35
#define ACCESS_TABLE_HASH 8940

static struct ImagingAccessInstance access_table[ACCESS_TABLE_SIZE];

Expand Down Expand Up @@ -87,6 +87,31 @@ get_pixel_16(Imaging im, int x, int y, void *color) {
memcpy(color, in, sizeof(UINT16));
}

static void
get_pixel_BGR15(Imaging im, int x, int y, void *color) {
UINT8 *in = (UINT8 *)&im->image8[y][x * 2];
UINT16 pixel = in[0] + (in[1] << 8);
char *out = color;
out[0] = (pixel & 31) * 255 / 31;
out[1] = ((pixel >> 5) & 31) * 255 / 31;
out[2] = ((pixel >> 10) & 31) * 255 / 31;
}

static void
get_pixel_BGR16(Imaging im, int x, int y, void *color) {
UINT8 *in = (UINT8 *)&im->image8[y][x * 2];
UINT16 pixel = in[0] + (in[1] << 8);
char *out = color;
out[0] = (pixel & 31) * 255 / 31;
out[1] = ((pixel >> 5) & 63) * 255 / 63;
out[2] = ((pixel >> 11) & 31) * 255 / 31;
}

static void
get_pixel_BGR24(Imaging im, int x, int y, void *color) {
memcpy(color, &im->image8[y][x * 3], sizeof(UINT8) * 3);
}

static void
get_pixel_32(Imaging im, int x, int y, void *color) {
memcpy(color, &im->image32[y][x], sizeof(INT32));
Expand Down Expand Up @@ -134,6 +159,16 @@ put_pixel_16B(Imaging im, int x, int y, const void *color) {
out[1] = in[0];
}

static void
put_pixel_BGR1516(Imaging im, int x, int y, const void *color) {
memcpy(&im->image8[y][x * 2], color, 2);
}

static void
put_pixel_BGR24(Imaging im, int x, int y, const void *color) {
memcpy(&im->image8[y][x * 3], color, 3);
}

static void
put_pixel_32L(Imaging im, int x, int y, const void *color) {
memcpy(&im->image8[y][x * 4], color, 4);
Expand Down Expand Up @@ -178,6 +213,9 @@ ImagingAccessInit() {
ADD("F", get_pixel_32, put_pixel_32);
ADD("P", get_pixel_8, put_pixel_8);
ADD("PA", get_pixel_32_2bands, put_pixel_32);
ADD("BGR;15", get_pixel_BGR15, put_pixel_BGR1516);
ADD("BGR;16", get_pixel_BGR16, put_pixel_BGR1516);
ADD("BGR;24", get_pixel_BGR24, put_pixel_BGR24);
ADD("RGB", get_pixel_32, put_pixel_32);
ADD("RGBA", get_pixel_32, put_pixel_32);
ADD("RGBa", get_pixel_32, put_pixel_32);
Expand Down
10 changes: 10 additions & 0 deletions src/libImaging/Unpack.c
Original file line number Diff line number Diff line change
Expand Up @@ -1239,6 +1239,12 @@ copy2(UINT8 *out, const UINT8 *in, int pixels) {
memcpy(out, in, pixels * 2);
}

static void
copy3(UINT8 *out, const UINT8 *in, int pixels) {
/* BGR;24 */
memcpy(out, in, pixels * 3);
}

static void
copy4(UINT8 *out, const UINT8 *in, int pixels) {
/* RGBA, CMYK quadruples */
Expand Down Expand Up @@ -1592,6 +1598,10 @@ static struct {
{"RGB", "B;16B", 16, band216B},
{"RGB", "CMYK", 32, cmyk2rgb},

{"BGR;15", "BGR;15", 16, copy2},
{"BGR;16", "BGR;16", 16, copy2},
{"BGR;24", "BGR;24", 24, copy3},

/* true colour w. alpha */
{"RGBA", "LA", 16, unpackRGBALA},
{"RGBA", "LA;16B", 32, unpackRGBALA16B},
Expand Down
Loading