Skip to content

Speed up Image.filter() for I;16* #9760

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.

Sign up for GitHub

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

Open
akx wants to merge 10 commits into
base: main
Choose a base branch
from
+111 −101
Open

Speed up Image.filter() for I;16* #9760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
183d6ba
Deduplicate modes list in test_image_filter
akx Jul 3, 2026
c008a7a
Remove inaccurate docstring text
akx Jul 3, 2026
e48212f
Rename lying test
akx Jul 3, 2026
54c9499
Remove unused parametrization from test_consistency_*
akx Jul 3, 2026
9eeec34
Refactor test_consistency; add 16bpc test
akx Jul 3, 2026
08548eb
Split I16 ImageFilter loops so they are optimized by compiler
akx Jul 3, 2026
b726bfa
Use size to refer to test images
radarhere Aug 15, 2026
3debcfe
Combine matrix constants
radarhere Aug 15, 2026
1189ca1
Move kernel outside image opens
radarhere Aug 15, 2026
80b9256
Simplified code
radarhere Aug 15, 2026
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
0 Tests/images/hopper_emboss.bmp → Tests/images/hopper_emboss_3x3.bmp
View file
Open in desktop
File renamed without changes.
0 Tests/images/hopper_emboss_more.bmp → Tests/images/hopper_emboss_5x5.bmp
View file
Open in desktop
File renamed without changes.
61 changes: 32 additions & 29 deletions Tests/test_image_filter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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:
Expand All @@ -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(
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions src/PIL/ImageFilter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
145 changes: 77 additions & 68 deletions src/libImaging/Filter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

@radarhere radarhere Aug 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// Check for I;16 mode once, not per pixel

Once this is merged, I don't think it's obvious what the comment is referring to

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];
Expand All @@ -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 {
Expand Down Expand Up @@ -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

@radarhere radarhere Aug 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
// 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];
Expand All @@ -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 {
Expand Down
Loading