Skip to content

Commit 77bd329

Browse files
committed
Fix round-up error in 16-bit filtering
1 parent 981459d commit 77bd329

2 files changed

Lines changed: 21 additions & 11 deletions

File tree

Tests/test_image_filter.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -214,10 +214,11 @@ def test_consistency_5x5(mode: str) -> None:
214214
def test_consistency_i16_high_byte(mode: str) -> None:
215215
# Exercise filters with a 16bpc image that has content in the high byte, too.
216216
im = Image.new(mode, (8, 8), 1000)
217-
pixel = im.filter(ImageFilter.SMOOTH).getpixel((4, 4))
218-
assert isinstance(pixel, (int, float))
219-
# Allow for kernel rounding errors.
220-
assert abs(pixel - 1000) <= 1
217+
# Ensure repeated smoothing retains the exact same color value,
218+
# rather than drifting due to rounding errors.
219+
for _ in range(5):
220+
im = im.filter(ImageFilter.SMOOTH)
221+
assert im.getpixel((4, 4)) == 1000
221222

222223

223224
@pytest.mark.parametrize(

src/libImaging/Filter.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
#include <stdint.h>
2828
#include "Imaging.h"
2929

30-
#define ROUND_UP(f) ((int)((f) >= 0.0 ? (f) + 0.5F : (f) - 0.5F))
3130
#define INT32_MAX_F 2147483647.0F
3231

3332
static inline UINT8
@@ -38,6 +37,14 @@ clip8(float in) {
3837
return (UINT8)in;
3938
}
4039

40+
static inline int
41+
clip16(float in) {
42+
// Branchless clamp to [0, 65535].
43+
in = in < 0.0f ? 0.0f : in;
44+
in = in > 65535.0f ? 65535.0f : in;
45+
return (int)in;
46+
}
47+
4148
static inline INT32
4249
clip32(float in) {
4350
// Clamp the low bound branchlessly (maxss).
@@ -182,9 +189,10 @@ ImagingFilter3x3(Imaging imOut, Imaging im, const float *kernel, float offset) {
182189
ss += kernel_i16(3, in1, x, &kernel[0], bigendian);
183190
ss += kernel_i16(3, in0, x, &kernel[3], bigendian);
184191
ss += kernel_i16(3, in_1, x, &kernel[6], bigendian);
185-
int ss_int = ROUND_UP(ss);
186-
out[x * 2 + (bigendian ? 1 : 0)] = clip8(ss_int % 256);
187-
out[x * 2 + (bigendian ? 0 : 1)] = clip8(ss_int >> 8);
192+
// NOT rounding here because `offset` already has a +0.5 bias.
193+
int ss_int = clip16(ss);
194+
out[x * 2 + (bigendian ? 1 : 0)] = (UINT8)(ss_int & 0xff);
195+
out[x * 2 + (bigendian ? 0 : 1)] = (UINT8)(ss_int >> 8);
188196
} else {
189197
ss += KERNEL1x3(in1, x, &kernel[0], 1);
190198
ss += KERNEL1x3(in0, x, &kernel[3], 1);
@@ -348,9 +356,10 @@ ImagingFilter5x5(Imaging imOut, Imaging im, const float *kernel, float offset) {
348356
ss += kernel_i16(5, in0, x, &kernel[10], bigendian);
349357
ss += kernel_i16(5, in_1, x, &kernel[15], bigendian);
350358
ss += kernel_i16(5, in_2, x, &kernel[20], bigendian);
351-
int ss_int = ROUND_UP(ss);
352-
out[x * 2 + (bigendian ? 1 : 0)] = clip8(ss_int % 256);
353-
out[x * 2 + (bigendian ? 0 : 1)] = clip8(ss_int >> 8);
359+
// NOT rounding here because `offset` already has a +0.5 bias.
360+
int ss_int = clip16(ss);
361+
out[x * 2 + (bigendian ? 1 : 0)] = (UINT8)(ss_int & 0xff);
362+
out[x * 2 + (bigendian ? 0 : 1)] = (UINT8)(ss_int >> 8);
354363
} else {
355364
ss += KERNEL1x5(in2, x, &kernel[0], 1);
356365
ss += KERNEL1x5(in1, x, &kernel[5], 1);

0 commit comments

Comments
 (0)