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
273 stars 53 forks source link

Add `transform_colorspace_to_srgb` operation and use it to fix inaccurate colors when saving specific image files #142

Closed Stormheg closed 8 months ago

Stormheg commented 8 months ago

Please see https://github.com/wagtail/Willow/issues/139 for a detailed description of what this is trying to accomplish. Took me longer than expected to prepare a PR, writing tests turned out to be quite time consuming!

I've expanded the automatic conversion discussed in #139 to include png files too as they are also affected by the color issue described in #139. Technically, GIF files are also affected but I've not bothered to fix them because there isn't much to gain when the colors are butchered to fit in a palette of 256 colors.

The automatic conversion during save of avif, heic, webp and png images only happens when we encounter an image that is affected by the issue (CMYK mode, with embedded ICC profile). For all other images, the new code path is not taken and there should be no impact.

I'd appreciate reviews from anyone! (not necessarily Willow maintainers only).

I've done my best to explain how and why in the docs and comments but I'm no expert on the subject of color spaces and gamuts and such. Please let me know if there are any inaccuracies or ambiguities.

Stormheg commented 8 months ago

I've added a new image for the test suite: dog_and_lake_cmyk_with_icc_profile.jpg. I've modified this image specifically to be affected. The image has been heavily downsized to make its size as small as possible but the embedded ICC profile still takes up a lot of space. It is a little over 1MB in size.

I took the image a couple years ago during a hike with my dog. Willow has my permission to use the downsized image file for its test suite.

Here is a full-sized image for your viewing pleasure. Try opening and saving it as webp or avif with an unpatched version of Willow and this patch to see the difference. zaar-lake_iso_coatedv2.jpg.zip

image

screenshot comparing the original JPEG on the left with an unpatched WEBP version.

andre-fuchs commented 8 months ago

@Stormheg Very cool! I suggest to add a system wide setting to convert all images to sRGB. Two setting variables would desirable, I think. A boolean type to turn this conversion on or off. And another one for the rendering intend (could be both an integer or char type). This is mainly an update of Wagtail, but I guess that Willow also would need some preparation for that. Does this make sense?

Stormheg commented 8 months ago

@andre-fuchs lets discuss automatically converting to sRGB in a separate issue 👍

Since most images and devices consuming the web are only capable of displaying sRGB this should be mostly fine; the space saving would be worth the cost in most cases I think. I could imagine that a photography site might not want to perform such conversions however. Saturated sRGB colors look a bit bland compared to what is possible with P3 and Adobe RGB.

Maybe there could be a middle ground where we provide functionality to detect what color gamut the original image is in? And if it is in a color other than sRGB, Wagtail could output something akin to the snippet below. This will only load the wide gamut with original ICC profile for devices that support displaying that color gamut.

<picture>
  <source media="(color-gamut: p3)" srcset="photo-wide.jpg">
  <img src="photo-srgb.jpg">
</picture>

Somewhat niche functionality maybe, but with the number of devices that are capable of recording and displaying wide-gamut images increasing (in particular recent iPhones are capable of this) it would be good future proofing for Wagtail to be aware of wide gamut images.

Anyway, let's discuss separately from this PR 😄

zerolab commented 8 months ago

Nice one @Stormheg. Let's discuss the suggestion from @andre-fuchs separately