-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add transform_colorspace_to_srgb
operation and use it to fix inaccurate colors when saving specific image files
#142
Add transform_colorspace_to_srgb
operation and use it to fix inaccurate colors when saving specific image files
#142
Conversation
I've added a new image for the test suite: 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 screenshot comparing the original JPEG on the left with an unpatched WEBP version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one @Stormheg
Left a couple of notes
@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? |
@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 😄 |
CMYK is not supported by PNG, WEBP, AVIF and HEIC Pillow encoders. When a CMYK image is encoded, it is converted to RGB but this conversion results in inaccurate colors because Pillow ignores the ICC profile when performing the conversion. As a workaround, we manually force an accurate conversion to RGB _before_ encoding the image. This results in a much more accurate representation of the original CMYK image.
d6c24a7
to
0d3dd06
Compare
The loose check appears to not be necesary. There do not appear to be differences between systems.
0d3dd06
to
4cf47e1
Compare
Nice one @Stormheg. Let's discuss the suggestion from @andre-fuchs separately |
Please see #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
andpng
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.