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 7 commits into
base: main
Choose a base branch
from
+145 −103
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d009ac9
Deduplicate modes list in test_image_filter
akx Jul 3, 2026
1920bcc
Remove inaccurate docstring text
akx Jul 3, 2026
7594f68
Rename lying test
akx Jul 3, 2026
bc1202b
Remove unused parametrization from test_consistency_*
akx Jul 3, 2026
5778ef1
Refactor test_consistency; add 16bpc test
akx Jul 3, 2026
14c9690
Split I16 ImageFilter loops so they are optimized by compiler
akx Jul 3, 2026
bb738b9
Merge branch 'main' into faster-filter-still
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
97 changes: 66 additions & 31 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,36 +170,73 @@ def test_kernel_not_enough_coefficients() -> None:
ImageFilter.Kernel((3, 3), (0, 0))


EMBOSS_3x3 = (
-1, -1, 0,
-1, 0, 1,
0, 1, 1,
) # fmt: skip

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: skip


@pytest.mark.parametrize(
"mode", ("L", "LA", "I", "I;16", "I;16L", "I;16B", "I;16N", "RGB", "CMYK")
"size, matrix, 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:
matrix = (
-1, -1, 0,
-1, 0, 1,
0, 1, 1,
) # fmt: skip
def test_consistency(
size: tuple[int, int], matrix: 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), matrix, 0.3)
with Image.open(expected) as reference:
kernel = ImageFilter.Kernel(size, 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")
"size, matrix",
[
pytest.param((3, 3), EMBOSS_3x3, id="3x3"),
pytest.param((5, 5), EMBOSS_5x5, id="5x5"),
],
)
def test_consistency_5x5(mode: str) -> None:
matrix = (
-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
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)
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], matrix: tuple[int, ...], mode: str
) -> None:
kernel = ImageFilter.Kernel(size, matrix, 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.
assert (
max(
abs(a - b) # type: ignore[operator]
for a, b in zip(
reference.get_flattened_data(), result.get_flattened_data(), strict=True
)
)
<= 1
)


@pytest.mark.parametrize("mode", ("I;16", "I;16L", "I;16B", "I;16N"))
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 @@ -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
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