New issue
Advanced search Search tips

Issue 880648 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Colors oversaturated on wide gamut monitor, compared with Photoshop/GIMP

Reported by macquari...@gmail.com, Sep 5

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3497.81 Safari/537.36

Example URL:
https://webkit.org/blog-files/color-gamut/

Steps to reproduce the problem:
1. Visit https://webkit.org/blog-files/color-gamut/
2. Download sRGB & P3 Images
3. Open in Photoshop/GIMP

What is the expected behavior?
Photos should look the same in Chrome as they do in Photoshop when using a calibrated display

What went wrong?
Photos are over-saturated. the sRGB image in Chrome looks more like the P3 image in Photoshop, and the P3 image in Chrome looks oversaturated compared with the P3 image in Photoshop.

Does it occur on multiple sites: Yes

Is it a problem with a plugin? No 

Did this work before? N/A 

Does this work in other browsers? No
 Firefox 61.0.2

Chrome version: 68.0.3440.106  Channel: stable
OS Version: 10.0
Flash Version: 

Monitor was calibrated with i1Profiler. Interestingly, Chrome and Firefox both show the same oversaturation.
 
sRGB-Compare-Chrome-Photoshop.PNG
1.4 MB View Download
P3-Compare-Chrome-Photoshop.PNG
1.3 MB View Download
Shanes-DELL-U2711.icm
1.3 MB Download
Labels: Needs-Triage-M69
Cc: viswa.karala@chromium.org
Labels: Triaged-ET Target-71 M-71 FoundIn-71
Status: Untriaged (was: Unconfirmed)
Able to reproduce the issue on chrome reported version# 69.0.3497.81 and on latest chrome# 71.0.3542.0 using the URL provided in comment# 0. As this issue is seen from M-60(60.0.3112.0), hence considering this issue as Non-Regression and marking it as Untriaged.
Note: Compared the images('sRGB' and 'P3') in chrome with GIMP. Unable to download GIMP on Mac and Linux, hence updating only Windows behavior.

Thanks!
Components: -Blink Internals>Compositing Blink>Paint
Labels: Needs-Feedback
Owner: ccameron@chromium.org
Status: Assigned (was: Untriaged)
macquarietextiles@, Could you add information on your color profiles? What is Chrome's "Force color profile" setting in chrome://flags?
Force color profile is set to Default, as I want it to use the calibrated profile provided by Windows. There doesn't seem to be any way to populate that option with a custom profile.
The attached profile lacks [rgb]XYZ tags. Perhaps Photoshop is getting gamut information from another tag?
More investigation: I think Photoshop is probably getting the gamut information from the A2B tag(s). Note that this particular profile uses a Lab PCS instead of the more common XYZ.
I reproduced [rgb]XYZ tags not being produced by i1Profiler when generating a table-based profile.

As a workaround, if you still have the calibration device you could try making a matrix-based profile instead of a table-based profile, or using a different profiling software such as DisplayCAL.

Technically these tags are required for RGB display-class profiles under ICC v2, but are not required under v4. In any case, given that this is a pretty common profiling software I think there is an argument to be made for increasing support for A2B.
I noticed probably the same behavior. When opening this website: https://www.flickr.com/photos/pva1964/29538718337/in/datetaken/ 
The monkey has blue fur, while the same website is shown correctly in InternetExplorer.

I have an EIZO cs2420 monitor calibrated using the i1 display pro, using ColorNavigator 6 software.

Strangely, only the sRGB profile gives problems, the AdobeRGB works correctly (not blue pixels)
monkey_01.png
187 KB View Download
CS2420(22688036)sRGB 100cd 6500K Clipped.icc
3.6 KB Download
CS2420(22688036)aRGB 100cd (0.3127, 0.3290) Clipped.icc
3.7 KB Download
Cc: brianosman@google.com mtkl...@google.com
Owner: brianosman@chromium.org
->brianosman/mtklein for ICC changes
Reproduced on my machine (with a different display). According to Flickr the EXIF declares Adobe RGB gamut. 

Both profiles have [rgb]XYZ tags, so that's not the issue in this case. The [rgb]TRC have nonzero black level, which would be unusual for a manufacturer-provided profile, but is not unusual for profiles created by calibration software. The curve doesn't look particularly strange otherwise. From a brief look the two curves seem to differ only by black level. When I replaced [rgb]TRC with a canonical sRGB curve (copied from an Apple profile) the blue part disappears. So I'd check what we are doing with the TRC especially when they have non-zero black level and in the presence of significant gamut correction.
Possible workaround for now: try unchecking "Reflect black level in tone curve" in ColorNavigator when creating the profile.
Hypothesis:

* AdobeRGB is a larger gamut than sRGB. Therefore, when converting colors in AdobeRGB that are outside the sRGB gamut to the sRGB gamut, we will end up with negative linear sRGB values.
* The sRGB profile has a gamma-like curve with nonzero black point. When this is inverted, there is going to be an extremely sharp negative section near black (see shaky-graph attached). In fact, if it ends up as a pure gamma the inverse is not even defined left of the x-intercept (but AFAIK the code always fits a linear section at the beginning?)
* Putting a negative value through this sort of function is going to make things even worse.
* Somehow the implicit clamping of fragments colors to [0, 1] doesn't seem to be rescuing us here. Not sure if this is not happening somehow, or if we are getting NaNed.

If this is indeed the issue, I can think of two (not mutually exclusive) measures:

1. Clamp negative values so that we are never trying to work with negative colors.
2. Apply some sort of black point compensation to display TRCs so that source black maps to destination black and source white to destination white. This could be as simple as a uniform vertical scale on the TRC so that TRC(0) = TRC.f = 0 and TRC(1) = 1. In principle we might want to condition this on rendering intent (and we might want to do so regardless of this particular issue, in order to use as much of the display's capability as possible/avoid black crush), but this might be tricky given that rendering intent belongs to the content but the TRCs in question belong to the display, and there may be some compositing in the middle that forms an information barrier.
inversion_problem.png
20.5 KB View Download

Comment 15 Deleted

Comment 16 Deleted

Maybe related to https://bugs.chromium.org/p/chromium/issues/detail?id=875650 ? Though there was no black point involved there.
Components: -Blink>Paint -Internals>Compositing Internals>Skia
Project Member

Comment 19 by bugdroid1@chromium.org, Sep 26

The following revision refers to this bug:
  https://skia.googlesource.com/skcms/+/0eb21d4ecaddfc3c439be8962d480f03795f9c34

commit 0eb21d4ecaddfc3c439be8962d480f03795f9c34
Author: Mike Klein <mtklein@google.com>
Date: Wed Sep 26 18:43:17 2018

dump inverse Best tf (i.e. what we use for destinations)

This was mildly useful when looking at the profiles
in the second half (Comment 9+) of the attached bug.

Bug:  chromium:880648 

Change-Id: Ib8a3df5b6de82e373fa1043d2d68a152ad624bce
Reviewed-on: https://skia-review.googlesource.com/157240
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>

[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/AdobeRGB.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/mobile/iPhone7p.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/BenQ_GL2450.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/ThinkpadX1YogaV2.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/Apple_Color_LCD.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/XPS13_9360.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/mobile/sRGB_parametric.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/AdobeColorSpin.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/color.org/Lower_Right.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/test_only.c
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/color.org/Lower_Left.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/SM245B.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/sRGB_HP.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/Dot_Gain_20_Grayscale.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/mobile/Display_P3_parametric.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/Gray_Gamma_22.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/mobile/Display_P3_LUT.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/sRGB_HP_2.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/sRGB_Calibrated_Heterogeneous.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/Color_Spin_Gamma_18.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/Kodak_sRGB.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/color.org/sRGB2014.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/BenQ_RL2455.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/HD_709.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/Generic_RGB_Gamma_18.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/sRGB_Facebook.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/mobile/sRGB_LUT.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/fuzz/inverse_tf_not_invertible.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/sRGB_black_scaled.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/sRGB_Calibrated_Homogeneous.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/sRGB_lcms.icc.txt
[modify] https://crrev.com/0eb21d4ecaddfc3c439be8962d480f03795f9c34/profiles/misc/DisplayCal_ASUS_NonMonotonic.icc.txt

Any luck on a fix?
So it should be available in a near future version.

Just browsing through the code: iccdump.c

I don't know if this code is called often, or just in a unit test:
static uint16_t read_big_u16(const uint8_t* ptr) {
    uint16_t be;
    memcpy(&be, ptr, sizeof(be));
#if defined(_MSC_VER)
    return _byteswap_ushort(be);
#else
    return __builtin_bswap16(be);
#endif
}
static uint32_t read_big_u32(const uint8_t* ptr) {
    uint32_t be;
    memcpy(&be, ptr, sizeof(be));
#if defined(_MSC_VER)
    return _byteswap_ulong(be);
#else
    return __builtin_bswap32(be);
#endif
}

But why memcpy (ptr) to an intermediate value be, just to push it on the stack and get a new value in return.

This could save you a full copy on every call:
static uint32_t read_big_u32(const uint8_t* ptr) {
#if defined(_MSC_VER)
    return _byteswap_ulong(*reinterpret_cast<uint32_t*>(ptr));
#else
    return __builtin_bswap32(*reinterpret_cast<uint32_t*>(ptr));
#endif
}
If I'm reading this right it's just moving the first point in the curve to zero---wouldn't this effectively cause the raw curve to appear as if it starts at zero at the first point, and then jump straight up to the black point at the second point?

I'd consider fitting f != 0 as before, and then rescaling the curve after the fact. The simplest way would to be to offset and scale; i.e. given fitted curve T(x) defined by 

cx + f if x < d else (ax + b)^g + e

produce output T'(x) defined by

offset = -T(0) = -f
scale = T(1) / (T(1) - T(0)) = (a + b)^g / ((a + b)^g - f)

a' = a * scale^(1/g)
b' = b * scale^(1/g)
c' = c * scale
d' = d
e' = (e + offset) * scale
f' = f + offset = 0
g' = g

(This choice of scale leaves white where it is, in case the profile is trying to adjust the white point.)
Or is memory alignment required for the byteswap to work? And the stack variable is properly aligned?
The memcpy() mentioned in 22+24 is for alignment, yes.  It's kind of notional... it'll be compiled to a single unaligned load instruction on pretty much every platform by pretty much every compiler.  Those reinterpret casts would allow compilers to assume the pointer is 4-byte aligned, which we don't know to be true there.

We considered rescaling the curve after the fact, but that would leave all the other points a little less well fit than if we just chose to believe f!=0 is a bad idea and fix that one point.  With these profiles being so close to black at zero anyway, I don't think it makes much of a difference one way or another, but that affine scale is definitely the next thing we'd try if this doesn't work well enough.
I noticed this issue is tagged with 'Needs-Feedback'. Do I or someone else need to give feedback?
NextAction: says -----, so this should be solved then?
Tried the link of the monkey, and was incorrectly drawn in my current chrome version:
Google Chrome is up to date
Version 71.0.3578.98 (Official Build) (64-bit)
Labels: -Needs-Feedback
Status: Fixed (was: Assigned)
No action needed. The fix for this didn't make it into version 71, but is present in version 72. You can verify this by running a Canary build of Chrome, if you'd like.
I installed the Canary build and it works as advertised :-)
Thanks for the fix!

Sign in to add a comment