diff --git a/Tests/test_image_filter.py b/Tests/test_image_filter.py index 6fe6b594f61..15170609d2f 100644 --- a/Tests/test_image_filter.py +++ b/Tests/test_image_filter.py @@ -6,6 +6,8 @@ from .helper import assert_image_equal, hopper +MODES = ("L", "LA", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") + @pytest.mark.parametrize( "filter_to_apply", @@ -35,9 +37,7 @@ ImageFilter.UnsharpMask(10), ), ) -@pytest.mark.parametrize( - "mode", ("L", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") -) +@pytest.mark.parametrize("mode", MODES) def test_sanity( filter_to_apply: ImageFilter.Filter | type[ImageFilter.Filter], mode: str ) -> None: @@ -51,20 +51,18 @@ def test_sanity( assert out.size == im.size -@pytest.mark.parametrize( - "mode", ("L", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") -) +@pytest.mark.parametrize("mode", MODES) def test_sanity_error(mode: str) -> None: im = hopper(mode) with pytest.raises(TypeError): im.filter("hello") # type: ignore[arg-type] -# crashes on small images @pytest.mark.parametrize("size", ((1, 1), (2, 2), (3, 3))) -def test_crash(size: tuple[int, int]) -> None: +def test_noop_on_small_images(size: tuple[int, int]) -> None: + # If image is smaller than kernel size, return it as-is im = Image.new("RGB", size) - im.filter(ImageFilter.SMOOTH) + assert_image_equal(im, im.filter(ImageFilter.SMOOTH)) @pytest.mark.parametrize( @@ -172,42 +170,67 @@ def test_kernel_not_enough_coefficients() -> None: ImageFilter.Kernel((3, 3), (0, 0)) +# fmt: off +EMBOSS_3x3 = ( + -1, -1, 0, + -1, 0, 1, + 0, 1, 1, +) +EMBOSS_5x5 = ( + -1, -1, -1, -1, 0, + -1, -1, -1, 0, 1, + -1, -1, 0, 1, 1, + -1, 0, 1, 1, 1, + 0, 1, 1, 1, 1, +) +# fmt: on + + @pytest.mark.parametrize( - "mode", ("L", "LA", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") + "size, weights, expected", + [ + pytest.param( + (3, 3), + EMBOSS_3x3, + "Tests/images/hopper_emboss.bmp", + id="3x3", + ), + pytest.param( + (5, 5), EMBOSS_5x5, "Tests/images/hopper_emboss_more.bmp", id="5x5" + ), + ], ) -def test_consistency_3x3(mode: str) -> None: +def test_consistency( + size: tuple[int, int], weights: tuple[int, ...], expected: str +) -> None: with Image.open("Tests/images/hopper.bmp") as source: - with Image.open("Tests/images/hopper_emboss.bmp") as reference: - kernel = ImageFilter.Kernel( - (3, 3), - # fmt: off - (-1, -1, 0, - -1, 0, 1, - 0, 1, 1), - # fmt: on - 0.3, - ) - assert_image_equal(source.filter(kernel), reference) + with Image.open(expected) as reference: + kernel = ImageFilter.Kernel(size, weights, 0.3) + result = source.filter(kernel) + assert_image_equal(result, reference) @pytest.mark.parametrize( - "mode", ("L", "LA", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") + "size, weights", + [ + pytest.param((3, 3), EMBOSS_3x3, id="3x3"), + pytest.param((5, 5), EMBOSS_5x5, id="5x5"), + ], ) -def test_consistency_5x5(mode: str) -> None: - with Image.open("Tests/images/hopper.bmp") as source: - with Image.open("Tests/images/hopper_emboss_more.bmp") as reference: - kernel = ImageFilter.Kernel( - (5, 5), - # fmt: off - (-1, -1, -1, -1, 0, - -1, -1, -1, 0, 1, - -1, -1, 0, 1, 1, - -1, 0, 1, 1, 1, - 0, 1, 1, 1, 1), - # fmt: on - 0.3, - ) - assert_image_equal(source.filter(kernel), reference) +@pytest.mark.parametrize("mode", ("I;16", "I;16L", "I;16B", "I;16N")) +def test_consistency_i16( + size: tuple[int, int], weights: tuple[int, ...], mode: str +) -> None: + kernel = ImageFilter.Kernel(size, weights, 0.3) + reference = hopper("I").filter(kernel) + result = hopper(mode).filter(kernel) + assert result.mode == mode + assert result.size == reference.size + # Compare logical pixel values rather than raw bytes + # so the test is endianness-independent. + ref = list(reference.get_flattened_data()) + res = list(result.get_flattened_data()) + assert max(abs(float(a) - float(b)) for a, b in zip(res, ref)) <= 1 # type: ignore @pytest.mark.parametrize( diff --git a/src/PIL/ImageFilter.py b/src/PIL/ImageFilter.py index 823365af076..64dff0aa63f 100644 --- a/src/PIL/ImageFilter.py +++ b/src/PIL/ImageFilter.py @@ -54,10 +54,8 @@ def filter(self, image: _imaging.ImagingCore) -> _imaging.ImagingCore: class Kernel(BuiltinFilter): """ - Create a convolution kernel. This only supports 3x3 and 5x5 integer and floating - point kernels. - - Kernels can only be applied to "L" and "RGB" images. + Create a convolution kernel. + This only supports 3x3 and 5x5 integer and floating point kernels. :param size: Kernel size, given as (width, height). This must be (3,3) or (5,5). :param kernel: A sequence containing kernel weights. The kernel will be flipped diff --git a/src/libImaging/Filter.c b/src/libImaging/Filter.c index b5f971d40c2..8ec9231a4ec 100644 --- a/src/libImaging/Filter.c +++ b/src/libImaging/Filter.c @@ -154,17 +154,16 @@ ImagingFilter3x3(Imaging imOut, Imaging im, const float *kernel, float offset) { } out[x] = in0[x]; } - } else { + } else if (im->type == IMAGING_TYPE_SPECIAL) { + // Check for I;16 mode once, not per pixel int bigendian = 0; - if (im->type == IMAGING_TYPE_SPECIAL) { - if ( - im->mode == IMAGING_MODE_I_16B + if ( + im->mode == IMAGING_MODE_I_16B #ifdef WORDS_BIGENDIAN - || im->mode == IMAGING_MODE_I_16N + || im->mode == IMAGING_MODE_I_16N #endif - ) { - bigendian = 1; - } + ) { + bigendian = 1; } for (y = 1; y < ysize - 1; y++) { UINT8 *restrict in_1 = (UINT8 *)im->image[y - 1]; @@ -173,31 +172,35 @@ ImagingFilter3x3(Imaging imOut, Imaging im, const float *kernel, float offset) { UINT8 *restrict out = (UINT8 *)imOut->image[y]; out[0] = in0[0]; - if (im->type == IMAGING_TYPE_SPECIAL) { - out[1] = in0[1]; - } + out[1] = in0[1]; for (x = 1; x < xsize - 1; x++) { float ss = offset; - if (im->type == IMAGING_TYPE_SPECIAL) { - ss += kernel_i16(3, in1, x, &kernel[0], bigendian); - ss += kernel_i16(3, in0, x, &kernel[3], bigendian); - ss += kernel_i16(3, in_1, x, &kernel[6], bigendian); - int ss_int = ROUND_UP(ss); - out[x * 2 + (bigendian ? 1 : 0)] = clip8(ss_int % 256); - out[x * 2 + (bigendian ? 0 : 1)] = clip8(ss_int >> 8); - } else { - ss += KERNEL1x3(in1, x, &kernel[0], 1); - ss += KERNEL1x3(in0, x, &kernel[3], 1); - ss += KERNEL1x3(in_1, x, &kernel[6], 1); - out[x] = clip8(ss); - } + ss += kernel_i16(3, in1, x, &kernel[0], bigendian); + ss += kernel_i16(3, in0, x, &kernel[3], bigendian); + ss += kernel_i16(3, in_1, x, &kernel[6], bigendian); + int ss_int = ROUND_UP(ss); + out[x * 2 + (bigendian ? 1 : 0)] = clip8(ss_int % 256); + out[x * 2 + (bigendian ? 0 : 1)] = clip8(ss_int >> 8); } - if (im->type == IMAGING_TYPE_SPECIAL) { - out[x * 2] = in0[x * 2]; - out[x * 2 + 1] = in0[x * 2 + 1]; - } else { - out[x] = in0[x]; + out[x * 2] = in0[x * 2]; + out[x * 2 + 1] = in0[x * 2 + 1]; + } + } else { + for (y = 1; y < ysize - 1; y++) { + UINT8 *restrict in_1 = (UINT8 *)im->image[y - 1]; + UINT8 *restrict in0 = (UINT8 *)im->image[y]; + UINT8 *restrict in1 = (UINT8 *)im->image[y + 1]; + UINT8 *restrict out = (UINT8 *)imOut->image[y]; + + out[0] = in0[0]; + for (x = 1; x < xsize - 1; x++) { + float ss = offset; + ss += KERNEL1x3(in1, x, &kernel[0], 1); + ss += KERNEL1x3(in0, x, &kernel[3], 1); + ss += KERNEL1x3(in_1, x, &kernel[6], 1); + out[x] = clip8(ss); } + out[x] = in0[x]; } } } else { @@ -314,17 +317,16 @@ ImagingFilter5x5(Imaging imOut, Imaging im, const float *kernel, float offset) { out[x + 0] = in0[x + 0]; out[x + 1] = in0[x + 1]; } - } else { + } else if (im->type == IMAGING_TYPE_SPECIAL) { + // Check for I;16 mode once, not per pixel int bigendian = 0; - if (im->type == IMAGING_TYPE_SPECIAL) { - if ( - im->mode == IMAGING_MODE_I_16B + if ( + im->mode == IMAGING_MODE_I_16B #ifdef WORDS_BIGENDIAN - || im->mode == IMAGING_MODE_I_16N + || im->mode == IMAGING_MODE_I_16N #endif - ) { - bigendian = 1; - } + ) { + bigendian = 1; } for (y = 2; y < ysize - 2; y++) { UINT8 *restrict in_2 = (UINT8 *)im->image[y - 2]; @@ -336,39 +338,46 @@ ImagingFilter5x5(Imaging imOut, Imaging im, const float *kernel, float offset) { out[0] = in0[0]; out[1] = in0[1]; - if (im->type == IMAGING_TYPE_SPECIAL) { - out[2] = in0[2]; - out[3] = in0[3]; - } + out[2] = in0[2]; + out[3] = in0[3]; for (x = 2; x < xsize - 2; x++) { float ss = offset; - if (im->type == IMAGING_TYPE_SPECIAL) { - ss += kernel_i16(5, in2, x, &kernel[0], bigendian); - ss += kernel_i16(5, in1, x, &kernel[5], bigendian); - ss += kernel_i16(5, in0, x, &kernel[10], bigendian); - ss += kernel_i16(5, in_1, x, &kernel[15], bigendian); - ss += kernel_i16(5, in_2, x, &kernel[20], bigendian); - int ss_int = ROUND_UP(ss); - out[x * 2 + (bigendian ? 1 : 0)] = clip8(ss_int % 256); - out[x * 2 + (bigendian ? 0 : 1)] = clip8(ss_int >> 8); - } else { - ss += KERNEL1x5(in2, x, &kernel[0], 1); - ss += KERNEL1x5(in1, x, &kernel[5], 1); - ss += KERNEL1x5(in0, x, &kernel[10], 1); - ss += KERNEL1x5(in_1, x, &kernel[15], 1); - ss += KERNEL1x5(in_2, x, &kernel[20], 1); - out[x] = clip8(ss); - } + ss += kernel_i16(5, in2, x, &kernel[0], bigendian); + ss += kernel_i16(5, in1, x, &kernel[5], bigendian); + ss += kernel_i16(5, in0, x, &kernel[10], bigendian); + ss += kernel_i16(5, in_1, x, &kernel[15], bigendian); + ss += kernel_i16(5, in_2, x, &kernel[20], bigendian); + int ss_int = ROUND_UP(ss); + out[x * 2 + (bigendian ? 1 : 0)] = clip8(ss_int % 256); + out[x * 2 + (bigendian ? 0 : 1)] = clip8(ss_int >> 8); } - if (im->type == IMAGING_TYPE_SPECIAL) { - out[x * 2 + 0] = in0[x * 2 + 0]; - out[x * 2 + 1] = in0[x * 2 + 1]; - out[x * 2 + 2] = in0[x * 2 + 2]; - out[x * 2 + 3] = in0[x * 2 + 3]; - } else { - out[x + 0] = in0[x + 0]; - out[x + 1] = in0[x + 1]; + out[x * 2 + 0] = in0[x * 2 + 0]; + out[x * 2 + 1] = in0[x * 2 + 1]; + out[x * 2 + 2] = in0[x * 2 + 2]; + out[x * 2 + 3] = in0[x * 2 + 3]; + } + } else { + for (y = 2; y < ysize - 2; y++) { + UINT8 *restrict in_2 = (UINT8 *)im->image[y - 2]; + UINT8 *restrict in_1 = (UINT8 *)im->image[y - 1]; + UINT8 *restrict in0 = (UINT8 *)im->image[y]; + UINT8 *restrict in1 = (UINT8 *)im->image[y + 1]; + UINT8 *restrict in2 = (UINT8 *)im->image[y + 2]; + UINT8 *restrict out = (UINT8 *)imOut->image[y]; + + out[0] = in0[0]; + out[1] = in0[1]; + for (x = 2; x < xsize - 2; x++) { + float ss = offset; + ss += KERNEL1x5(in2, x, &kernel[0], 1); + ss += KERNEL1x5(in1, x, &kernel[5], 1); + ss += KERNEL1x5(in0, x, &kernel[10], 1); + ss += KERNEL1x5(in_1, x, &kernel[15], 1); + ss += KERNEL1x5(in_2, x, &kernel[20], 1); + out[x] = clip8(ss); } + out[x + 0] = in0[x + 0]; + out[x + 1] = in0[x + 1]; } } } else {