diff --git a/Tests/images/hopper_emboss.bmp b/Tests/images/hopper_emboss_3x3.bmp similarity index 100% rename from Tests/images/hopper_emboss.bmp rename to Tests/images/hopper_emboss_3x3.bmp diff --git a/Tests/images/hopper_emboss_more.bmp b/Tests/images/hopper_emboss_5x5.bmp similarity index 100% rename from Tests/images/hopper_emboss_more.bmp rename to Tests/images/hopper_emboss_5x5.bmp diff --git a/Tests/test_image_filter.py b/Tests/test_image_filter.py index 4fa24e2f9c5..b93ccd264e1 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,38 +170,43 @@ def test_kernel_not_enough_coefficients() -> None: ImageFilter.Kernel((3, 3), (0, 0)) -@pytest.mark.parametrize( - "mode", ("L", "LA", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") -) -def test_consistency_3x3(mode: str) -> None: - matrix = ( +EMBOSS_MATRIX = { + 3: ( -1, -1, 0, -1, 0, 1, 0, 1, 1, - ) # fmt: skip - with Image.open("Tests/images/hopper.bmp") as source: - with Image.open("Tests/images/hopper_emboss.bmp") as reference: - kernel = ImageFilter.Kernel((3, 3), matrix, 0.3) - assert_image_equal(source.filter(kernel), reference) - - -@pytest.mark.parametrize( - "mode", ("L", "LA", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK") -) -def test_consistency_5x5(mode: str) -> None: - matrix = ( + ), + 5: ( -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: skip + ), +} # fmt: skip + + +@pytest.mark.parametrize("size", (3, 5)) +def test_consistency(size: int) -> None: + kernel = ImageFilter.Kernel((size, size), EMBOSS_MATRIX[size], 0.3) 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), matrix, 0.3) + with Image.open(f"Tests/images/hopper_emboss_{size}x{size}.bmp") as reference: assert_image_equal(source.filter(kernel), reference) +@pytest.mark.parametrize("size", (3, 5)) +@pytest.mark.parametrize("mode", ("I;16", "I;16L", "I;16B", "I;16N")) +def test_consistency_i16(size: int, mode: str) -> None: + kernel = ImageFilter.Kernel((size, size), EMBOSS_MATRIX[size], 0.3) + reference = hopper("I").filter(kernel) + result = hopper(mode).filter(kernel) + assert result.mode == mode + assert result.size == reference.size + for x in range(size): + for y in range(size): + assert result.getpixel((x, y)) == reference.getpixel((x, y)) + + @pytest.mark.parametrize("mode", ("I;16", "I;16L", "I;16B", "I;16N")) def test_consistency_i16_high_byte(mode: str) -> None: # Exercise filters with a 16bpc image that has content in the high byte, too. diff --git a/src/PIL/ImageFilter.py b/src/PIL/ImageFilter.py index ffdb7ffca10..7fa65339a1d 100644 --- a/src/PIL/ImageFilter.py +++ b/src/PIL/ImageFilter.py @@ -53,10 +53,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 41568a99e2e..a46dd00cf79 100644 --- a/src/libImaging/Filter.c +++ b/src/libImaging/Filter.c @@ -161,17 +161,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]; @@ -180,32 +179,36 @@ 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); - // NOT rounding here because `offset` already has a +0.5 bias. - int ss_int = clip16(ss); - out[x * 2 + (bigendian ? 1 : 0)] = (UINT8)(ss_int & 0xff); - out[x * 2 + (bigendian ? 0 : 1)] = (UINT8)(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); + // NOT rounding here because `offset` already has a +0.5 bias. + int ss_int = clip16(ss); + out[x * 2 + (bigendian ? 1 : 0)] = (UINT8)(ss_int & 0xff); + out[x * 2 + (bigendian ? 0 : 1)] = (UINT8)(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 { @@ -322,17 +325,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]; @@ -344,40 +346,47 @@ 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); - // NOT rounding here because `offset` already has a +0.5 bias. - int ss_int = clip16(ss); - out[x * 2 + (bigendian ? 1 : 0)] = (UINT8)(ss_int & 0xff); - out[x * 2 + (bigendian ? 0 : 1)] = (UINT8)(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); + // NOT rounding here because `offset` already has a +0.5 bias. + int ss_int = clip16(ss); + out[x * 2 + (bigendian ? 1 : 0)] = (UINT8)(ss_int & 0xff); + out[x * 2 + (bigendian ? 0 : 1)] = (UINT8)(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 {