Skip to content
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.

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

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

Open
coredumperror opened this issue Sep 5, 2024 · 6 comments

Comments

@coredumperror
Copy link

coredumperror commented Sep 5, 2024

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
Copy link
Author

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
Copy link
Author

coredumperror commented Sep 5, 2024

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
Copy link
Collaborator

zerolab commented Sep 6, 2024

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

@coredumperror
Copy link
Author

coredumperror commented Sep 10, 2024

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
Copy link
Author

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
Copy link
Member

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants