uploadcare / pillow-simd

The friendly PIL fork
https://python-pillow.github.io/pillow-perf/
Other
2.16k stars 85 forks source link

Possible bug in `ImagingFilter3x3f_u8`? #104

Closed kleisauke closed 2 years ago

kleisauke commented 2 years ago

What did you do?

Applying a identity filter to a 1-channel image with an offset of 1.

What did you expect to happen?

Same result as Pillow.

What actually happened?

There's a visible border at the top and bottom of the image.

What are your OS, Python and Pillow versions?

Reproduction

$ python3 -c "from PIL import Image, ImageFilter; Image.open('x.png').filter(ImageFilter.Kernel((3, 3), (0, 0, 0, 0, 1, 0, 0, 0, 0), 1, 1)).save('pillow-simd-identity.png')"
from PIL import Image, ImageFilter;

img = Image.open('x.png')
img = img.filter(ImageFilter.Kernel((3, 3), (0, 0, 0, 0, 1, 0, 0, 0, 0), 1, 1))
img.save('pillow-simd-identity.png')

Using this test image: x

Which was generated with:

# Prepare test image
$ vips identity t.v
$ vips replicate t.v x.png 1 256
$ rm t.v

Pillow-SIMD 7.0.0.post3: pillow-simd-identity

Pillow 8.4.0: pillow-identity

homm commented 2 years ago

Thanks, I'll look into it soon

kleisauke commented 2 years ago

Great! For this particular image, I was able to workaround it this way:

--- a/src/libImaging/Filter.c
+++ b/src/libImaging/Filter.c
@@ -138,7 +138,7 @@ ImagingFilter(Imaging im, int xsize, int ysize, const FLOAT32* kernel,
     if (xsize == 3) {
         /* 3x3 kernel. */
         if (im->image8) {
-            ImagingFilter3x3f_u8(imOut, im, kernel, offset);
+            ImagingFilter3x3f_u8(imOut, im, kernel, offset - 1.0f);
         } else {
             if (fast_path) {
                 ImagingFilter3x3i_4u8(imOut, im, norm_kernel, norm_offset);

As a result, the average of the image is the same before and after applying the identity filter (and there are no edges visible).

$ vips avg x.png
127.500000
$ python3 -c "from PIL import Image, ImageFilter; Image.open('x.png').filter(ImageFilter.Kernel((3, 3), (0, 0, 0, 0, 1, 0, 0, 0, 0), 1, 1)).save('pillow-simd-identity.png')"
$ vips avg pillow-simd-identity.png
127.500000
kleisauke commented 2 years ago

Oh, I accidentally used an offset of 1 in the above commands (hence the above patch fixed it), the correct test script should be:

$ python3 -c "from PIL import Image, ImageFilter; Image.open('x.png').filter(ImageFilter.Kernel((3, 3), (0, 0, 0, 0, 1, 0, 0, 0, 0), 1, 0)).save('pillow-simd-identity.png')"
from PIL import Image, ImageFilter;

img = Image.open('x.png')
img = img.filter(ImageFilter.Kernel((3, 3), (0, 0, 0, 0, 1, 0, 0, 0, 0), 1, 0))
img.save('pillow-simd-identity.png')

So this bug manifests only when offset != 0.0f.

homm commented 2 years ago

Oh, I accidentally used an offset of 1

Yeah, I noticed this from the start and thought this was the issue )

Anyway, fixed in https://pypi.org/project/Pillow-SIMD/7.0.0.post4/ Yes, still significantly behind the upstream. Sorry.