getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.25k stars 166 forks source link

Thumbnails don’t include ICC color profile #2537

Closed fabianmichael closed 2 years ago

fabianmichael commented 4 years ago

Describe the bug
When generating thumbnails from images, the color profile is stripped from the file. This causes problems on wide gamut-monitors such as highend graphic design monitors, but also more consumer-grade devices like Apple products with P3 color gamut (e.g. some iPad models, 5k iMac, MacBook Pro …), as colors of thumbnails appear over-saturated in Mozilla Firefox.

For the ImageMagick driver, there's a hardcoded method, that always adds the -strip parameter to the imagemagick cli command. https://github.com/getkirby/kirby/blob/master/src/Image/Darkroom/ImageMagick.php#L136

This does not only remove unwanted metadata, but also any color profile attached to the image. Some browsers like Safari do color correction on wide gamut displays by assuming, that image files without profiles are in sRGB color space, but Firefox just displays the image in the full gamut. I can understand the decision why this is present, because it makes thumbnail files a bit smaller, but it produces weird results, especially when you use the original image file in a layout, because its thumbnail will show a noticable color shift.

The simplest solution would be to not strip any metadata or at least providing an option to configure this. A more advanced solution would be, to ensure that every thumbnail includes a color profile. According to this post, the default sRGB profile adds about 2.5 kB to the thumbnail file.

The more advanced solution, as used by large image hosters (e.g. Facebook) would be to accept a small amount of inaccuracy by replacing the original profile data with a minimal sRGB profile, that only weights around 0.5 kB (e.g. https://pippin.gimp.org/sRGBz/). However, I don’t know if the latter approach would be worth it to be included into core.

To Reproduce
Steps to reproduce the behavior:

  1. Upload an image file (I tested with a JPEG) with an sRGB profile included to any page. I used the ImageMagick driver for this test.
  2. In the corresponding template file, fetch the image via the API and embed the original file and a thumbnail of that file next to each other.
  3. Compare the 2 images, the thumbnail should have over-saturated colors. This effect becomes way more noticable on wide gamut displays, such as the Apple iMac 5k.

Expected behavior
The thumbnail’s colors should match the color values of the original file in all browsers, that support color management in some way.

Screenshots
spaghetti On the left, you can see a version of the image with a sRGB profile embedded. The colors appear the same as in color-managed applications such as Adobe Lightroom. On the right: Image without ICC profile, colors appear over-saturated in Mozilla Firefox.

Kirby Version
3.3.4

Desktop:

WebMechanic commented 4 years ago

FWIW if ImageMagick::strip() would support an $option['strip'] like many of its sister methods this call could be made optional / configurable.

This issue also applies to GD2 which has no support for color profiles whatsoever but might be the only option on many hosters. In this case one has to save/convert all images with/to sRGB to avoid color shifts and there's probably nothing Kirby could do "out of the box". This particular drawback of GD should be noted in the Guide.

Firefox is (partially) color managed. It does not support the "rendering intend" stored in an image (see below), so: avoid. Images must be tagged with a v2 profile for Firefox to color manage or it uses sRGB "as it should" - which is the root cause here. Unfortunately sRGB is the "default" color space for the Web so either dull or over saturated images are the default -- by design :) To check compliance visit http://www.color.org/version4html.xalter

Workarounds

This are possible workarounds until a valuable solution is available.

Photo Editor vs. OS

Keep in mind that image and photo editors ship with their own custom set of (fancy & licenced) color profiles, whereas browser don't. They might depend on what the OS provides so you should probably stick to "classics" like AdobeRGB; ProPhoto RGB or 16bit (HDR) profiles might cause trouble as well as (new) ICCv4 profiles no matter the screen type and quality. Photo editors may also choose to only refer to the profile by its name in the metadata OR actually embed the ICM data, making the file slightly bigger - and what --strip apparently destroys :)

WebP

AFAIK WebP always applies and uses color profiles during conversion. Might be worth trying that format and file sizes can be dramatically smaller than JPEGs and PNGs.

CSS or SVG (embed with <image>)

Those may come to the rescue (for some folks) to apply CSS or (more powerful) SVG filters after the fact:

A sprinkle of PHP

There's https://cimage.se/doc/introduction that does server side color management but also depends on Imagick to do so. It's a bit older but worth a peek for inspiration to put in a Kirby Plugin.

Imagick

Imagick comes with a bundled sRGB profile (.icm) that can be embedded, so there's no reason to eventually ship one with Kirby. However you need to determine the Imagick version to get its path ($version = $image->getVersion();)

Firefox gfx config & ICCv4

Firefox on Android is color managed and does support ICCv4 profiles by default. Firefox (Mac, Win) should also support color management for tagged images using ICC v2 as of 2007 by default but you can also force this via about:config so it's only good for testing. Open about:config and search for gfx.color_management:

Enjoy & stay healthy.

manspeterson commented 3 years ago

Also agree it would be nice if there was an option to preserve the color profile.

Another simple solution to preserve the color profile, yet stripping all other profiles, using ImageMagick is to "remove" the "-strip" parameter and replace all "-resize" parameters with "-thumbnail".

You can demo it by simply modifying three lines in "src/Image/Darkroom/ImageMagick.php"; comment out line 132 (the "strip" stage) and replace "-replace" with "-thumbnail" on line 181 and 200.

fabianmichael commented 3 years ago

Some updates to this issue:

Firefox 89.9 is now fully color-managed on macOS, which is great news, because most (all?) Apple computers feature wide-gamut displays these days. From the release notes:

Colors in Firefox on macOS will no longer be saturated on wide gamut displays, untagged images are properly treated as sRGB, and colors in images tagged as sRGB will now match CSS colors. – https://www.mozilla.org/en-US/firefox/89.0/releasenotes/

@bastianallgeier I found a solution, that could easily be incorporated into Kirby by changing a tiny portion of the ImageMagick driver. Instead of throwing out all meta metadata by using -strip for IM, it also supports selectively throwing out profiles with a glob-like syntax. The following should delete all metadata (EXIF, IPTC etc.), but keep the ICC profile, if present.

protected function strip(string $file, array $options): string
    {
        return '+profile "!icc,*"';
    }

https://github.com/getkirby/kirby/blob/master/src/Image/Darkroom/ImageMagick.php#L226

This can be tested, by setting up a simple plugin:

# site/plugins/colorz/index.php
<?php

use Kirby\Cms\App as Kirby;
use Kirby\Image\Darkroom;
use Kirby\Image\Darkroom\ImageMagick;

class CustomImageMagickDriver extends ImageMagick
{
    protected function strip(string $file, array $options): string
    {
        return '+profile "!icc,*"';
    }
}

Kirby::plugin('fm/colorz', [
    'hooks' => [
        'system.loadPlugins:after' => function () {
            Darkroom::$types['im'] = \CustomImageMagickDriver::class;
        }
    ]
]);

Caveat: This obviously only works with the ImageMagick driver. But given the fact, that it’s available on most shared hosts these days (at least from my experience), it could provide an easy fix for most of us. This might cause slight color shifts to existing sites, that did not create to color-managed thumbnails before and will also increase file sizes of thumbnails ever so slightly. But in my opinion, these are all minor trade-offs.

Update: Here is the demo plugin mentioned above as ZIP file, ready to be dropped into your plugin dir: colorz.zip

fabianmichael commented 3 years ago

Okay, my optimism came a bit too early. According to a chat with iskrisis from the Kirby Discord chat and reading the proposed solution from the author of ImageOptim (see https://imageoptim.com/color-profiles.html), in theory we should:

Save images in the sRGB profile with gamma 2.2, but don't embed any profile in the image. That's the most compatible and most efficient solution.

That works, as long we don’t require our wide-gamut thumbnails on a website (very unlikely). But as tools such as ImageMagick and VIPS can be compiled without support for color management, we cannot even be sure that a server will handle these things correctly. Maybe just leave it open for later. This really topic seems to be a rabbit hole, not that color management alone was complicated enough … but as long as we cannot assume somewhat uniform support among web servers, it does not seem possible having a universal solution at the moment.

This is an alternative suggestion: https://makandracards.com/makandra/473154-always-convert-and-strip-user-provided-images-to-srgb

I’ve included screenshots of the discussion on Discord, so it does not get lost (sorry for including images as text, but I don’t know how to export from Discord):

Bildschirmfoto 2021-06-22 um 14 48 37 Bildschirmfoto 2021-06-22 um 14 48 44

lukasbestle commented 3 years ago

Given that Firefox now also assumes sRGB for images without embedded profile, doesn't this mean that all images exported as sRGB and then stripped from their color profile by Kirby's thumb engine should now be displayed properly in all browsers?

If that's the case, I think the solution is to always use sRGB images as @iskrisis proposed. This will ensure the highest compatibility with all server setups. As you wrote, it's pretty much impossible to implement profile conversion in a way that is compatible with all servers.

iskrisis commented 3 years ago

I think the issue is not what is the worst case scenario default - always just strip profile and hope people provided sRGB image but that it's not possible to control the behavior. Many im installations even on shared hosting will be able to do color conversion and some people will indeed want to keep the profile in place.

But maybe with im this could be handled by some kind of fallback setting for all other usecases. im driver just builds the output command as string and some kind of "manual" mode that only sets resolution and output paths and leaves rest to developer could be general last resort solution.

Without adding any new configuration... we could be check if im can color manage - if yes convert nonsrgb to srgb then strip. If no then just strip. This would probably help overall image quality without anyone having to change anything.

luismgarcia commented 3 years ago

It seems a sensible and functional idea. It will solve a complex problem for many people in a simple way. And maybe it will also help with the changes that are coming: JPEG XL, AVIF, etc. I hope!
https://jpegxl.info/ https://sneyers.info/

fabianmichael commented 3 years ago

Given that Firefox now also assumes sRGB for images without embedded profile, doesn't this mean that all images exported as sRGB and then stripped from their color profile by Kirby's thumb engine should now be displayed properly in all browsers?

@lukasbestle Yes and no. First of all there’s not one canonical sRGB profile, but rather 15-20 or even more (see https://ninedegreesbelow.com/photography/srgb-profile-comparison.html#addendum). Differences include different white points, slightly different curves and different settings for black point compensation. On top of that, there are also differences between v2 and v4 profiles. In practice, this should still look way better than uploading e.g. an image in the AdobeRGB color space and stripping its profile. But as we cannot now for sure which sRGB profile was applied to untagged images and also which sRGB profile a browser would use as a fallback, serving them without a profile is not ideal.

Then there’s also desktop software, when a user downloads an image. AFAIK, some apps assume sRGB as fallback for untagged images, others will just use monitor colors, which will lead to over-saturated colors on wide-gamut monitors, such as the iMac or any recent MacBook Pro (and countless devices from other manufacturers).

But maybe with im this could be handled by some kind of fallback setting for all other usecases. im driver just builds the output command as string and some kind of "manual" mode that only sets resolution and output paths and leaves rest to developer could be general last resort solution.

Without adding any new configuration... we could be check if im can color manage - if yes convert nonsrgb to srgb then strip. If no then just strip. This would probably help overall image quality without anyone having to change anything.

I like the idea of being able to modify the convert command directly. This would really open up new possibilities for advanced users, who don’t want to implement a custom driver, that could theoretically break with the next update. Support for color management in ImageMagick depends on LittleCMS, which identifies as lcms when requesting the version info of ImageMagick.

$ convert --version | fgrep lcms
Delegates (built-in): bzlib fontconfig freetype gslib heic jng jp2 jpeg lcms lqr ltdl lzma openexr png ps tiff webp xml zlib

So the profile commands could also be handles by the built-in driver, as this check should not have any notable performance drawbacks. But as I said earlier, after all what I’ve learned over the past days, I’d rather include a profile with every generated image, as a typical sRGB profile only adds about ~ 3 kB to each thumbnail. If that’s still too large, there are also smaller versions around 500 bytes, that sacrifice a bit of accuracy for smaller file size (e.g. http://pippin.gimp.org/sRGBz/).

Another thing, that might be interesting for Kirby here is the ImageMagick PHP module, which seemingly got more popularity over the last years and is also used by many popular CMS like WordPress. Not only does it usually come with LittleCMS, but also includes support for nice features such as generating PDF previews. A later version of Kirby could look into that as an alternative to the CLI version of ImageMagick, because it’s way nicer to handle in PHP and does not require any configuration, such as setting the correct path the the convert binary in config.

Big question is: Should this be fixed by now or is the whole image processing backend of Kirby prone for a more extensive update anyway to support additional file formats like WebP and AVIF. I’m personally still very happy with JPEG and PNG for most use-cases, but I can clearly see the benefits of these newer formats as well.

lukasbestle commented 3 years ago

I like the idea to dynamically decide on the convert command to use based on the available ImageMagick features.

If I understand it correctly, any ICC profile to be embedded would need to be shipped with Kirby, right? Or does ImageMagick itself come with a profile that can be used with a CLI flag? If we would need to ship one, then the tiny one you linked would be great because of its CC0 license.

In any case, we need help with the right convert commands.

Another thing, that might be interesting for Kirby here is the ImageMagick PHP module

Do you mean the native PHP extension? To be honest I've rarely seen that on shared hosting servers. They often provide the CLI, but not the PHP extension.

fabianmichael commented 3 years ago

I like the idea to dynamically decide on the convert command to use based on the available ImageMagick features.

👍

If I understand it correctly, any ICC profile to be embedded would need to be shipped with Kirby, right? Or does ImageMagick itself come with a profile that can be used with a CLI flag? If we would need to ship one, then the tiny one you linked would be great because of its CC0 license.

As far as my understanding goes, we need to provide our own profile. Using a v2 profile would ensure the best compatibility with devices. The official profile issued by the ICC can be found here: https://www.color.org/srgbprofiles.xalter#v2 (sRGB2014.icc). This is the most canonical version of sRGB I could think of, so maybe we should favor this? By the way, if an image in e.g. AdobeRGB colorspace is delivered to the browser with the correct profile, the browser would do the conversion instead. As these algorithms are not standardized, the result might differ a bit between different browsers, but should still look much better, than just stripping the profile. Added benefit: It will look gorgeous on wide-gamut displays, but that’s probably rather a niche use-case.

According to their terms, it also seems to be available under a very permissive license, but that should be double-checked: https://www.color.org/profiles2.xalter#license

In any case, we need help with the right convert commands.

My approach would be the following:

  1. Test, if imagemagick was compiled with LittleCMS: Execute convert --version and search the output for the string lcms
  2. If color management is supported, add the profile parameter to the convert command: convert "img.jpg" -profile "/path/to/profile.icc" […]. Important note here: Use -profile and not -set profile, because the latter would only assign a profile, but convert the colors.
  3. Ensure, that the stripping of metadata does not remove the profile. There are two ways of doing that. Either use -strip and apply the profile again after that (-strip -profile "/path/to/profile.icc"), or use a glob-like syntax for stripping all profiles, but the color profile: +profile "!icc,*".

The full command should look something like this:

convert "/path/to/img.jpg" -profile "/path/to/profile.icc" +profile "!icc,*" […] -limit thread 1 "/path/to/img.jpg"

See my experimental plugin for a working implementation (without the feature test): https://github.com/fabianmichael/kirby3-imagekit/blob/main/src/Image/Darkroom/ImageMagick.php

One thing I haven’t tested though is grayscale. But by default, grayscale images use the same non-linear colorspace as sRGB images, so it should be fine.

More information about color management in ImageMagick can be found in their docs. https://imagemagick.org/script/color-management.php

Please let me know, if you need any more help or if I should provide some code.

Do you mean the native PHP extension? To be honest I've rarely seen that on shared hosting servers. They often provide the CLI, but not the PHP extension.

Just tested on All-inkl.com, Uberspace and BioHost. All three of them provide the Imagick extension and it seems to my, like it became more popular recently. But that’s just an assumption, not scientifically proven. Maybe that’s due to the fact, that WordPress also uses Imagick, if available. But as our CLI-based implementation works fine, I also don’t see any reason for switching anytime soon.

distantnative commented 3 years ago

Reading all of this it sounds to my not-very-familiar ears as if a lots of presumptions have to be taken, style decisions made etc. which makes it look to me as if this is rather an opinionated plugin than something for the core?

fabianmichael commented 3 years ago

@distantnative Ignoring color management leads to washed-out colors in thumbnails, so I was trying to figure out a way of improving the situation. Unfortunately, there is no canonical way of getting things right, as you can see from the comments above. In my opinion, all possible solutions are somehow opinionated.

The most un-opinionated solution I could think of was leave the original profile, because that would not touch any of the image’s colors and would leave the task of displaying it correctly to the browser itself. But just stripping the profile (Kirby’s current behavior) will inevitably lead to dull colors in certain situations as described above.

Yes, it’s opinionated to convert images to sRGB, but that’s also the most pragmatic way of ensuring decent quality everywhere. As @iskrisis suggested, converting to some sRGB profile and stripping the profile afterwards will still produce better results, than doing nothing about it (that’s what most websites do). However, leaving the sRGB profile in the resulting thumbnail should lead to slightly more accuracy (because of the large amount of different sRGB profiles), but it not strictly necessary.

iskrisis commented 3 years ago

So i've tried to figure this out. And my conclusion is we should not be stripping the ICC. Even Kirby shouldn't do this by default.

Conclusion of author is

The two most important recommendations:

  1. Embed color profiles in all images. This lets color-managed systems know exactly how to treat the color data in your image.

  2. When preparing a photo for web display, use sRGB. On average, it's the closest color space to the average user's average monitor.... on average. Browsers that don't understand the embedded profile are all Color Stupid, so sRGB is the best you can guess.

My notes:

Overall it actually seems that the opinionated position is to strip the profiles. I think if anyone would specifically want to strip icc or automatically convert then that's advanced and they should probably tweak the behavior.

BTW this is a bit of surprise to me because in print including color profiles wouldn't be great and we would basically convert/strip everything to some baseline and tweak stuff with printer afterwards.

fabianmichael commented 3 years ago

@iskrisis Thanks for your research!

he most bulletproof solution would be to convert everything to sRGB and then it doesn't seem to matter if the profile is included or not. I didn't notice any differences with sRGB image without or with profile. But the automatic conversion can also lead to bad results. When photographers target sRGB they tweak on output but with automatic conversion from some profile to another its not perfect - underlined by many options to tweak (littleCMS has 7 algorithms to pick from). So even if you can automatically convert to sRGB you might get better results by not converting and just keeping the icc there.

The preferred way for the conversion algorithm (rendering intent) is usually Perceptual (ImageMagick’s default). Though that is not a standardized algorithm, this is probably what most browsers will do, when e.g. trying to display an AdobeRGB image on an sRGB monitor. This might not always look ideal, but will still look way better than the same image without any profile.

I agree, the simplest solution with best compatiblity would probably be to stick with the original profile by using +profile "!icc,*" instead of -strip. Unless the image is run through some falsely configured optimizer tools later on, this should yiel decent results for most people.

lukasbestle commented 3 years ago

Yeah, I think that makes the most sense. It doesn't require feature-checking (which reduces performance) and also increases behavioral consistency between different servers. I feel like the main reason we went for -strip is to remove all the other image metadata like EXIF data, GPS, exact date etc.

@fabianmichael If I understand +profile "!icc,*" correctly, it strips every profile except ICC profiles, right? Does this mean that everything that -strip strips is still stripped except the ICC profile or what is "profile" referring to in this option?

fabianmichael commented 3 years ago

@fabianmichael If I understand +profile "!icc,*" correctly, it strips every profile except ICC profiles, right? Does this mean that everything that -strip strips is still stripped except the ICC profile or what is "profile" referring to in this option?

@lukasbestle Yes, the +profile option removes all listed profiles and uses a glob-like syntax and IM seems to consider any kind of meta information as a "profile". See https://imagemagick.org/script/command-line-options.php#profile

lukasbestle commented 3 years ago

Awesome, that's a really simple fix then. As far as I can tell, our GDLib implementation is not affected by this, correct?

fabianmichael commented 3 years ago

@lukasbestle Yes, GD does not support any kind of color management as of today. In theory, you could extract the ICC profile from the headers of any image file in pure PHP and re-assign it after changing. But, I guess, you got more important things to do than digging through the specifications of different image formats. 🙃

I would, however, recommend testing this or even add a unit test. Throw in source images with EXIF/IPTC/XMP metadata and ICC profiles embedded and run it through the thumbnail engine. As far as I have tested and according to the IM docs, it really should remove everything but the ICC profile, but better double-check that if you got resources for it.

lukasbestle commented 3 years ago

I think you have much more experience with this than any of us. If you have the time and are in the mood for it, a PR would be very welcome!

fabianmichael commented 3 years ago

I think you have much more experience with this than any of us. If you have the time and are in the mood for it, a PR would be very welcome!

@lukasbestle Hope, I can create a PR. Just need to figure out a way of validating the results without requiring additional dependencies, such as ExifTool. But maybe, I can handle this using PHP’s built-in EXIF functions. But most important: should I create a PR for the 3.6 branch or should this still land in a 3.5 patch release?

lukasbestle commented 3 years ago

More 3.5 patch releases are unlikely at this point. A PR against develop would be best. :)

WebMechanic commented 2 years ago

Well, I for one believe you're opening Pandora's Box fiddling with ICC profiles in images for the most inconsistend platform ever: the Web.

This seems like another attempt trying to make Browsers and the Web do what designer want, when Browsers have always at best taken anything we do as a "hint" and eventually decide what to do anyway... like taking a user's hardware into account...

Also sRGB has been the default Color Space for the Web since the beginning and applies to all things colour, it's not a recent decision and given the market share of fancy Apple monitors compared to the rest...

This also still applies to CSS colours! As long as browsers do not support the new CSS colour sapce features you'd get in trouble if colour corrrected, and ICCv4 tagged images stand against a non-color corrected background colour still using shabby default sRGB.

Reality check: you might want to consider your audiences browsing the websites you make or have a look at the server logs.

A recent release of FF for Mac fixed the long-standing bug with oversaturated images, So ...

Just my 2ct.

iskrisis commented 2 years ago

Hi @WebMechanic what are your suggesting? I can't figure it out.

Well, I for one believe you're opening Pandora's Box fiddling with ICC profiles in images for the most inconsistend platform ever: the Web.

This change - keeping icc profiles by default is less "fiddling" than what is happening now - stripping the profile. It's doing less work not more.

This seems like another attempt trying to make Browsers and the Web do what designer want

You are saying that like it's a bad thing. Of course we want Web to do/look as designer wants.

You are right about CSS colors - we are in wierd territory here. But CSS Colors management are coming and hopefully will be soon in browsers. It's matter of time and then this would change would need to happen anyway. It's better default, you can keep striping ICCs if you want to. On the other side many people get in troubles for this because their photographer/artist client's images look slightly wrong.

Also i am not sure what you are trying to say about audiences. Like if people with Apple devices or screens with wider gamut than sRGB were some bizzare posh thing. Most current screens have wider gamut. Even the cheaper ones and especially phones they have great screens.

WebMechanic commented 2 years ago

Sorry for some misunderstanding: I wasn't actually refering to the PR -- which we'll find out if it works as "imagined" in the wild. I don't know if the behaviour is configurable; hope so.

TL;DR Just because your images contain ICC profiles (or are tagged to comply with a specific standard) and you have a perfectly calibrated, wide gamit, high rolution monitor doesn't mean others have, too. Fun fact: they don't.

The whole idea of using ICC profiles on the web = the dillusional idea of "having control" over user's devices, software choice and browsing habits. Web Devs never had and never will. That's the whole point of The Web to be open and flexible and accessible to everyone no matter the device -- and it's the crux with web design (and development) -- and the fun of it.

If you decide to add profiles to your site's images, you ARE opening Pandora's Box. That's all I'm saying. Using profiles doesn't even work safely and proerly in the print world where they are common for ages.

Like if people with Apple devices or screens with wider gamut than sRGB were some bizzare posh thing.

Well, I guess that's what I am saying then :)

Most current screens have wider gamut. Even the cheaper ones and especially phones they have great screens.

I understand that's what you like to believe from your personal experience and maybe your lucky audience does indeed fall into that category, and maybe your friends and colleagues and clients have and use such devices everywhere -- or you just think they do.

I also believe you're confusing screen resolution with color gamut - they usually don't go hand in hand. +10bit monitors are not common... yet. A large gamut doesn't imply proper color calibration. User's can still mess with their monitor settings; intentionally or by accident.

Have fun.

iskrisis commented 2 years ago

@WebMechanic i am not sure what are you trying to achieve here. If you went through the issue and the reasoning behind it you would find out that this wouldn't change anything for old devices.

When you upload sRGB image it works the same as now. It will show reasonably everywhere - win. When some of the editors uploads image in AdobeRGB (core of this issue) then device with no way to handle this will simply think it's sRGB just like if we stripped the profile. Zero difference (except that 2kb of embedded profile data).

We can't expect to do color conversion to sRGB by default because GD can't do it and in IM it depends on how its compiled - i think maybe this change we could try to give this as option if developer wants and knows their IM can do color conversion. Something that maybe could make sense for some occasions but i doubt even that after digging into this.

The problem is when we always strip the ICC it will look wrong for both "color unmanaged" users and the "color managed" users.

On the other hand for the devices that can understand embedded color profiles they will try to show the image as intended. It doesn't matter if the gamut of the display can't show all of the destination profile. AFAIK screens are pretty smart and try to show as much as possible and convert themselves.

We are not talking about 10bit here. I am not sure you realize this but when you use AdobeRGB on 8bit display it will work, it just makes the steps between colors bigger. There are many screens that show AdobeRGB pretty well that are 8bit - thats how pros have been doing it for a long time.

Test it yourself on your screen - you might be surprised.

Screens that can show more than sRGB are very common. From the stats you sent 24% of users are on Apple devices - those could show more (even if only a little) for a long time. Even my 2013 macbook pro i am writing this on shows bit more colors on AdobeRGB pictures. And with phones (not mentioning iphones from last 3 years are 10bit) from those 35% android users how many have some better one from recent years (Google Pixels, Sony, OnePlus etc) 5%-10% if i am lowballing it? Because all of those have crazy capable screens.

So thats what atleast 35%? We are robbing of nicer pictures for no reason?

Now let's take into consideration:

  1. Currently i can't turn off the stripping without using custom driver.
  2. Most users/devs are not aware about any of this ICC hustle and won't know to opt in.

What should be the default then? I was hesitant about this at first as well when @fabianmichael brought this up. But more and more i think it's the right change.

BTW ... confusing resolution with color gamut? Come on. Proper calibration? Sure if you fuck up your colors they will be fucked sRGB or not. We can only do our best and give the machines as much info as we can to work properly.

fabianmichael commented 2 years ago

@WebMechanic It took a lot discussions and back-and-forth to find a "proper" solution to this issue. I can just second what @iskrisis said: with this patch, we are trying to fiddle less with profiles, than before. If images are being uploaded in anything but sRGB*, a huge color/contrast shift could appear, when the profile is just removed. Our first attempt has been to convert colors to sRGB, but as ImageMagick can be compiled without color management support for some reason, this would not have worked everywhere. This is not perfect, but IMHO the best thing we can do to avoid confusing color shifts in thumbnails.

*) Actually, there is not one canonical sRGB color space, but plenty of them. They are similar, but not equal. Let’s assume, someone uses e.g. krita to create a colorful concept art image and uses the built-in sRGB profile, which differs a bit from the one e.g. Photoshop uses. When we strip that profile, the browser will just pick its default sRGB color profile and colors would still be off. This default color profile might be different on proprietary and open source operating systems, or even be provided by the browser. But including the profile with our image allows the OS/browser to display the image a bit closer to its intended look – even on an uncalibrated 90% sRGB monitor.

bastianallgeier commented 2 years ago