wagtail / Willow

A wrapper that combines the functionality of multiple Python image libraries into one API
https://willow.wagtail.org/
BSD 3-Clause "New" or "Revised" License
274 stars 53 forks source link

Greyscale images with ICC color profiles that are output as AVIF renditions display incorrectly in Firefox #153

Open coredumperror opened 2 months ago

coredumperror commented 2 months ago

This one's a really obscure issue, and I'm not actually sure why the combination of greyscale + color profile causes this, but I have example files to show it in action.

The issue appears to be that Firefox still doesn't support grid-based AVIF files in its AVIF renderer, despite the bug being active for almost 4 years. The ultimate source of that problem is that Firefox uses mp4parse-rust as its AVIF renderer, and that doesn't support grid-based AVIFs. This issue on pdn-avif may also shed some light on the underlying problem.

To see Firefox's grid issue in a non-Wagtail environment, go to https://0xc0000054.github.io/pdn-avif/using-image-grids.html and click the "Image grid example.avif" link in the second section. In Chrome, it'll display the same 1/2/3/4 image seen below the link. In Firefox, you'll get an error message.

I've also attached three files to this issue that can be uploaded to a Wagtail site to show off how greyscale + color profile makes Willow render the AVIF in a way that breaks in Firefox.

This image, which is a greyscale jpeg with an ICC Color Profile of "Dot Gain 20%" triggers the render bug: Tagore_Caputh-Greyscale

This image, which is a greyscale jpeg with no ICC profile, does not trigger the bug: Tagore_Caputh-Greyscale-no-profile (Note that, if you flip between the above images, you'll see that the one with no profile is dramatically darker)

This image, which is an RGB jpeg with an sRGB color profile, does not trigger the bug: Tagore_Caputh-RGB (This image looks identical to the original one with the color profile intact)

Upload these three to a Wagtail site that's configured to display renditions as AVIF, and you'll see that the first one displays wrong in Firefox: CleanShot 2024-09-05 at 11 22 20

I'm not 100% sure that this problem is actually with grid-based AVIFs, because I don't know how to determine if the broken one is actually a grid-based AVIF, but it seems likely.

My one clue about a potential fix for this is that pdn-avif issue that I linked. Apparently, using a "very slow" preset for the AVIF rendering step would prevent the image from being generated as a grid, and thus prevent the image corruption. Maybe that's something that Willow could do when it runs into a greyscale+color profile image that it's being asked to render as AVIF?

coredumperror commented 2 months ago

Further testing shows that greyscale PNGs with color profiles also cause the problem in Firefox. Converting the PNG to RGB fixes it, like it did with the jpegs.

coredumperror commented 2 months ago

I was writing a monkey patch to PillowImage.save_as_avif() when I discovered that simply converting greyscale images with color profiles to sRGB colorspace, like what that function already does for CMYK images, actually fixes the problem perfectly.

I changed line 440 (in v1.8.0) from:

            if image.mode == "CMYK":

to

            if image.mode in ("CMYK", "L"):

And now the image renditions aren't corrupted when viewed in Firefox, and they still look the same as the original.

Would this be a viable patch to Willow?

zerolab commented 2 months ago

cc @Stormheg as I think this relates to #142 and it may affect your projects too

coredumperror commented 2 months ago

Looks like there needs to be a small tweak to my "patch", because there are actually two different image modes that cover greyscale:

            if image.mode in ("CMYK", "L", "LA"):

EDIT: Oh, maybe this one isn't a good idea... Still testing, and getting some strange results.

coredumperror commented 2 months ago

OK, so it turns out that doing an sRGB colorspace conversion on a greyscale image with an alpha channel gives you really borked results. I ended up having to convert the image to non-transparent via the method described here: https://stackoverflow.com/a/33507138/464318, before doing the sRGB colorspace conversion. Now my patch is:

        if image.mode in ("CMYK", "L", "LA"):
            if image.mode == "LA":
                background = PILImage.new("RGBA", image.size, (255, 255, 255))
                rgb_image = image.convert("RGBA")
                self.image = PILImage.alpha_composite(background, rgb_image)
Stormheg commented 2 months ago

Interesting case! Looks like you have gone down a similar rabbit hole as I did in #142, have fun 😄

I wouldn't be opposed to converting obscure formats like lightness to the more common sRGB, like I did with CMYK. It would probably avoid a bunch of equally obscure render bugs in browsers.

Happy to review patches 👍