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

Preserve image ICC profiles #2957

Merged
merged 4 commits into from Feb 2, 2018

Conversation

@mchapman87501
Copy link
Contributor

@mchapman87501 mchapman87501 commented Jan 27, 2018

This improves appearance of images on systems with wide gamut displays, e.g., Retina iMacs.

Pull Request Checklist

  • I’ve read the guidelines for contributing.
  • I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
  • I tested my changes.

Description

Images served by 'nikola serve -b' looked flat on my iMac with retina display in comparison to the corresponding original images. Interactive testing with Pillow's Image.open and Image.save revealed that it did not automatically copy 'icc_profile' image info.

If it would be better for me to open an issue in the python-pillow/Pillow project, please let me know. Thanks!

See the mention of available options for https://pillow.readthedocs.io/en/latest/reference/Image.html#PIL.Image.Image.save.

…with wide gamut displays, e.g., Retina iMacs.
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 27, 2018

@mchapman87501
Copy link
Contributor Author

@mchapman87501 mchapman87501 commented Jan 28, 2018

Current recommended practice seems to be to embed color profiles when exporting images for the web. Most desktop web browsers now support ICC profile management. See the links below.

The images of concern to me are JPEG photos that embed either sRGB (e.g., "sRGB IEC61966-2.1") or wide-gamut ("Display P3") color profiles. I've done follow-up tests to better understand cases in which such images render poorly. In case it's of use to anyone, here's what I've found.

The tests encompassed:

  • A single RAW photo exported from macOS Photos as JPEGs, embedding sRGB and Display P3 profiles
  • "No profile" copies of the images above, created using PIL.open() followed by PIL.save()
  • iMac Retina display and laptop displays (presumably covering the sRGB gamut)
  • Safari on macOS; and Firefox and Chrome, on linux and macOS

In all cases, one image looked flat in comparison to the others: the "no profile" image created from the "Display P3" image.

So, at least for my use case, a workaround for the current release of Nikola is to use only JPEG images exported with an embedded sRGB profile.

Links

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 28, 2018

This sRGB PNG bug is still present in Firefox, and there is also an unrelated example of a JPEG that doesn’t work properly in Firefox at the end of this thread: https://bugzilla.mozilla.org/show_bug.cgi?id=621474

Removing the color profile fixes the sample PNGs shown in the thread. (They work fine in Chrome and Safari. Compared with Digital Color Meter.app.) While this might work better for JPEG and wide color gamuts, it might also hinder standard gamut PNG support in some cases.

@mchapman87501
Copy link
Contributor Author

@mchapman87501 mchapman87501 commented Jan 28, 2018

Here is a bit more detail about those test cases:

The Firefox JPEG image cited (https://drive.google.com/file/d/0B_yEbP1sy9ScX09CYS1UaGxyWWs/view?usp=sharing) contains a CanonEOS5DMk2-Generic color profile.

On macOS 10.13.3, Digital Color Meter.app 5.11 shows the PNG image data (https://bugzilla.mozilla.org/show_bug.cgi?id=621474) are presented in the same way by Safari 11.0.3, Chrome 64.0.3282.119 and Firefox 58.0, with one exception: in Firefox, the sample values for the "none" (i.e., untagged) PNG image data exactly match the original hex color values - Firefox renders them without applying the system display profile.

@ralsina
Copy link
Member

@ralsina ralsina commented Jan 30, 2018

Hmmm ... so, executive summary?

This is a good idea for jpeg but not for png?

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 30, 2018

Some browsers (Firefox) still have issues with color profile support. (That didn’t change since about 2011.) At the same time, a color profile can improve the appearance of images on wide color gamut displays (such as the ones found in newer Apple hardware).

@mchapman87501
Copy link
Contributor Author

@mchapman87501 mchapman87501 commented Jan 31, 2018

Your executive summary and Chris's follow-up look good to me.

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Jan 31, 2018

Perhaps this could be an optional setting? @mchapman87501, can you make it so ICC profiles are preserved only when the user asks for it via settings?

@ralsina
Copy link
Member

@ralsina ralsina commented Jan 31, 2018

Making it an option should not be terribly hard. If @mchapman87501 can't/won't I can hack it

@mchapman87501
Copy link
Contributor Author

@mchapman87501 mchapman87501 commented Feb 1, 2018

I'll be happy to make the changes. It may be a day or two before I have time, if that's ok.

@ralsina
Copy link
Member

@ralsina ralsina commented Feb 1, 2018

@getnikola getnikola deleted a comment Feb 2, 2018
Copy link
Member

@Kwpolska Kwpolska left a comment

Looks good to me!

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Feb 2, 2018

Thanks for contributing, and especially for improving test coverage! 🎉

You may want to add the e-mail used for the original commit (***-8.com) to your GitHub profile so it will appear as yours.

@Kwpolska Kwpolska merged commit e091954 into getnikola:master Feb 2, 2018
3 checks passed
@mchapman87501
Copy link
Contributor Author

@mchapman87501 mchapman87501 commented Feb 3, 2018

Thanks! I wish I had checked my git config before that first commit - the fig8 email account was retired just at the beginning of the year.

@mchapman87501 mchapman87501 deleted the preserve-icc-profiles branch Feb 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants