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

Fix ImagingAccess for I;16N on big-endian #7921

Merged
merged 11 commits into from
Apr 25, 2024
27 changes: 27 additions & 0 deletions Tests/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,33 @@
uploader = "github_actions"


modes = (
"1",
"L",
"LA",
"La",
"P",
"PA",
"F",
"I",
"I;16",
"I;16L",
"I;16B",
"I;16N",
"RGB",
"RGBA",
"RGBa",
"RGBX",
"BGR;15",
"BGR;16",
"BGR;24",
"CMYK",
"YCbCr",
"HSV",
"LAB",
)


def upload(a: Image.Image, b: Image.Image) -> str | None:
if uploader == "show":
# local img.show for errors.
Expand Down
48 changes: 10 additions & 38 deletions Tests/test_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,43 +28,16 @@
assert_image_similar_tofile,
assert_not_all_same,
hopper,
is_big_endian,
is_win32,
mark_if_feature_version,
modes,
skip_unless_feature,
)

# name, pixel size
image_modes = (
("1", 1),
("L", 1),
("LA", 4),
("La", 4),
("P", 1),
("PA", 4),
("F", 4),
("I", 4),
("I;16", 2),
("I;16L", 2),
("I;16B", 2),
("I;16N", 2),
("RGB", 4),
("RGBA", 4),
("RGBa", 4),
("RGBX", 4),
("BGR;15", 2),
("BGR;16", 2),
("BGR;24", 3),
("CMYK", 4),
("YCbCr", 4),
("HSV", 4),
("LAB", 4),
)

image_mode_names = [name for name, _ in image_modes]


class TestImage:
@pytest.mark.parametrize("mode", image_mode_names)
@pytest.mark.parametrize("mode", modes)
def test_image_modes_success(self, mode: str) -> None:
Image.new(mode, (1, 1))

Expand Down Expand Up @@ -1045,15 +1018,15 @@ def test_close_graceful(self, caplog: pytest.LogCaptureFixture) -> None:


class TestImageBytes:
@pytest.mark.parametrize("mode", image_mode_names)
@pytest.mark.parametrize("mode", modes)
def test_roundtrip_bytes_constructor(self, mode: str) -> None:
im = hopper(mode)
source_bytes = im.tobytes()

reloaded = Image.frombytes(mode, im.size, source_bytes)
assert reloaded.tobytes() == source_bytes

@pytest.mark.parametrize("mode", image_mode_names)
@pytest.mark.parametrize("mode", modes)
def test_roundtrip_bytes_method(self, mode: str) -> None:
im = hopper(mode)
source_bytes = im.tobytes()
Expand All @@ -1062,12 +1035,11 @@ def test_roundtrip_bytes_method(self, mode: str) -> None:
reloaded.frombytes(source_bytes)
assert reloaded.tobytes() == source_bytes

@pytest.mark.parametrize(("mode", "pixelsize"), image_modes)
def test_getdata_putdata(self, mode: str, pixelsize: int) -> None:
im = Image.new(mode, (2, 2))
source_bytes = bytes(range(im.width * im.height * pixelsize))
im.frombytes(source_bytes)

@pytest.mark.parametrize("mode", modes)
def test_getdata_putdata(self, mode: str) -> None:
if is_big_endian and mode in ("BGR;15", "BGR;16"):
pytest.xfail(f"Known failure of {mode} on big-endian")
im = hopper(mode)
reloaded = Image.new(mode, im.size)
reloaded.putdata(im.getdata())
assert_image_equal(im, reloaded)
Expand Down
38 changes: 8 additions & 30 deletions Tests/test_image_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from PIL import Image

from .helper import assert_image_equal, hopper, is_win32
from .helper import assert_image_equal, hopper, is_win32, modes

# CFFI imports pycparser which doesn't support PYTHONOPTIMIZE=2
# https://github.com/eliben/pycparser/pull/198#issuecomment-317001670
Expand Down Expand Up @@ -138,8 +138,8 @@ def color(mode: str) -> int | tuple[int, ...]:
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
# 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))

Expand Down Expand Up @@ -168,16 +168,15 @@ def check(self, mode: str, expected_color_int: int | None = None) -> None:
f"expected {expected_color} got {actual_color}"
)

# Check 0
# check 0x0 image with None initial color
Yay295 marked this conversation as resolved.
Show resolved Hide resolved
im = Image.new(mode, (0, 0), None)
assert im.load() is not None

error = ValueError if self._need_cffi_access else IndexError
with pytest.raises(error):
im.putpixel((0, 0), expected_color)
with pytest.raises(error):
im.getpixel((0, 0))
# Check 0 negative index
# check negative index
Yay295 marked this conversation as resolved.
Show resolved Hide resolved
with pytest.raises(error):
im.putpixel((-1, -1), expected_color)
with pytest.raises(error):
Expand All @@ -198,36 +197,15 @@ def check(self, mode: str, expected_color_int: int | None = None) -> None:
f"expected {expected_color} got {actual_color}"
)

# Check 0
# check 0x0 image with initial color
Yay295 marked this conversation as resolved.
Show resolved Hide resolved
im = Image.new(mode, (0, 0), expected_color)
with pytest.raises(error):
im.getpixel((0, 0))
# Check 0 negative index
# check negative index
Yay295 marked this conversation as resolved.
Show resolved Hide resolved
with pytest.raises(error):
im.getpixel((-1, -1))

@pytest.mark.parametrize(
"mode",
(
"1",
"L",
"LA",
"I",
"I;16",
"I;16B",
"F",
"P",
"PA",
"BGR;15",
"BGR;16",
"BGR;24",
"RGB",
"RGBA",
"RGBX",
"CMYK",
"YCbCr",
),
)
@pytest.mark.parametrize("mode", modes)
def test_basic(self, mode: str) -> None:
self.check(mode)

Expand Down
5 changes: 4 additions & 1 deletion Tests/test_lib_pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,10 @@
)

def test_I16(self) -> None:
self.assert_pack("I;16N", "I;16N", 2, 0x0201, 0x0403, 0x0605)
if sys.byteorder == "little":
self.assert_pack("I;16N", "I;16N", 2, 0x0201, 0x0403, 0x0605)
else:
self.assert_pack("I;16N", "I;16N", 2, 0x0102, 0x0304, 0x0506)

Check warning on line 222 in Tests/test_lib_pack.py

View check run for this annotation

Codecov / codecov/patch

Tests/test_lib_pack.py#L222

Added line #L222 was not covered by tests

def test_F_float(self) -> None:
self.assert_pack("F", "F;32F", 4, 1.539989614439558e-36, 4.063216068939723e-34)
Expand Down
12 changes: 5 additions & 7 deletions src/libImaging/Access.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,6 @@ get_pixel_16B(Imaging im, int x, int y, void *color) {
#endif
}

static void
get_pixel_16(Imaging im, int x, int y, void *color) {
UINT8 *in = (UINT8 *)&im->image[y][x + x];
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];
Expand Down Expand Up @@ -207,7 +201,11 @@ ImagingAccessInit() {
ADD("I;16", get_pixel_16L, put_pixel_16L);
ADD("I;16L", get_pixel_16L, put_pixel_16L);
ADD("I;16B", get_pixel_16B, put_pixel_16B);
ADD("I;16N", get_pixel_16, put_pixel_16L);
#ifdef WORDS_BIGENDIAN
ADD("I;16N", get_pixel_16B, put_pixel_16B);
#else
ADD("I;16N", get_pixel_16L, put_pixel_16L);
#endif
ADD("I;32L", get_pixel_32L, put_pixel_32L);
ADD("I;32B", get_pixel_32B, put_pixel_32B);
ADD("F", get_pixel_32, put_pixel_32);
Expand Down
Loading