Skip to content

[css-color] Changes to CSS color spec #557

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

Merged
merged 16 commits into from
Nov 13, 2016
Merged

[css-color] Changes to CSS color spec #557

merged 16 commits into from
Nov 13, 2016

Conversation

cabanier
Copy link
Member

I gathered some feedback from our color people and changed the spec per their recommendations

@cabanier cabanier changed the title Change to CSS color spec [css-color] Changes to CSS color spec Sep 30, 2016
Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the source for this 1/2.4 gamma? Apple uses 1/2.2 for their Display P3 while Samsung and Dell use 1/2.6 (and call it either pr, or DCI P3, or "Cinema".

What is the source for the chromaticities?

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Rrec 2020 transfer function is copied from the BT.2020 specification.

Why do you propose changing it to "the same as sRGB"?

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks better than what we had on video, thanks!

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 2020 transfer function is from the 2020 specification. Please clarify why you want it changed to 1/2.4 and what your source is for that.

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useful changes on the JS, thanks

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@svgeesus
Copy link
Contributor

Wait, I thought I was approving changesets individually not the entire PR :(

I need responses on the source of the chromaticities for P3 and on the transfer function for Rec.2020 before approving.

@@ -1714,7 +1717,7 @@ Profiled, Device-dependent Colors</h2>
or an RGB colorspace (such as ProPhoto <!-- ref -->, <!-- would be good to mention AdobeRGB 1998 here too, need to check trademark and copyright issues first --> widely used by photographers), or any other color or monochrome output device which has been characterized.
In addition, for convenience,
CSS provides two predefined RGB color spaces:
DCI P3 [[!DCI-P3]],
p3d65 [[!DCI-P3]],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK the closest standard definition of "P3D65" is in Annex D of SMPTE ST 2067-21:2016, where it is specified as:

  • the color primaries specified in SMPTE RP 431-2, and
  • the illuminant D65 specified in SMPTE RP 177.

That definition does not include a transfer function. Moreover, the COLOR.6 colorspace defined in the same specification combines P3D65 colorimetry with the PQ transfer function.

I would therefore not use "P3D65", and instead use "P3D65Gamma_X_X".

Perhaps more importantly, I do not expect content to be distributed using this particular combination of colorimetry and transfer function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that Apple, at least, wanted the P3 value for the color gamut media query specifically so they could distribute wide-gamut still imagery in their Display P3 colorspace.

I'm waiting for an ICC profile from @grorg to see what exactly their Display P3 space is like.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was told that these are the values that are currently used by Apple in the p3 profile.
I can send that profile to the group if wanted so you can double-check

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cabanier My primary comment is that the name should not be "p3d65" since it refers to something else already. A secondary question is whether the group intends to document specific display capabilities and/or encoding colorspaces. For example, sRGB is currently the encoding colorspace for the web even though not all displays can necessarily display sRGB completely.

Copy link
Member Author

@cabanier cabanier Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@palemieux the spec intends to document popular compositing/encoding spaces.

As for the name, we can call it anything else (p3rgb?) since it's just Apple's implementation of the p3 colorspace.
Ideally, we should keep it short.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@palemieux: yes, for now it's to tag input colors. There's a note/todo in the spec that we need to tag the compositing space but that will likely happen in a later spec.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grorg I renamed p3d65 and updated the PR

Copy link
Contributor

@Crissov Crissov Oct 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t know where the P3 part comes from, but a more useful keyword for authors would probably be cinematic (or digital-cinema, movie-theater, film-projector …), alongside uhdtv, hdtv, sdtv, ntsc, pal/ebu, … (table at Wikipedia)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Crissov Displays are marketed as being "P3" (see Apple iMac and Ipad pro).
Also, there's already a P3 media query: https://drafts.csswg.org/mediaqueries-4/#color-gamut

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment equally applies to MQ4 gamuts then, I guess.

@@ -1872,18 +1890,8 @@ Predefined colorspaces: ''srgb'', ''dci-p3'', and ''rec2020''.</h3>
<tr><th>Green chromaticity</th><td>x=0.170 y=0.797</td></tr>
<tr><th>Blue chromaticity</th><td>x=0.131 y=0.046</td></tr>
<tr><th>White chromaticity</th><td>x=0.3127 y=0.3290 (D65)</td></tr>
<tr><th>Transfer function</th><td>see below</td></tr>
<tr><th>Transfer function</th><td>1/2.4</td></tr>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not telling the full story. Rec. 2020 specifies an OETF with an exponent of 0.45, but notes that

in typical production practice the encoding function of image sources is adjusted so that the final
picture has the desired look, as viewed on a reference monitor having the reference decoding
function of Recommendation ITU-R BT.1886

BT.1886 specifies an EOTF with a gamma of 2.4.

I recommend referencing Rec.2020, instead of attempting to replicate normative text here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the normative reference should be Rec.2020. The "see below" transfer function with the high precision coefficients was copied from that specification. Just saying that is is aboyt 1/2.4 is just as wrong as saying that sRGB is "about 1/2.2".

Copy link
Member Author

@cabanier cabanier Sep 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked over our internal discussion wrt 2020:

The W3C transfer function characterizes reference cameras, not displays.
In actual Rec2020 video production, the W3C transfer function is never used.
Never used in production cameras, never used in 2020 displays.
BTW, the W3C curve is an OETF, not an EOTF.

The writer ignored this statement in Rec. ITU-R BT.2020-2 :
4) In typical production practice the encoding function of image sources is adjusted so that the final picture has the desired look, as viewed on a reference monitor having the reference decoding function of Recommendation ITU-R BT.1886, in the reference viewing environment defined in Recommendation ITU-R BT.2035.

W3C needs to follow the 2020 note or else 2020 videos will look washed out in browsers, the way QTP looks washed out.
The transfer function (1886) for reference Rec.2020 displays is gamma 2.4.
EBU has more details on reference monitors.

For a computer in an office, Recommendation ITU-R BT.2035 does not apply. The sRGB curve was designed for this purpose.
So W3C should consider either gamma 2.4 for dark surrounds or sRGB for average surrounds, but not the current pick.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Rik. That sort of detailed rationale would have been very helpful along with the original PR, as tab notes. I now agree with specifying the sRGB curve for normal viewing environments, and the spec should provide the specific rationale you excerpted above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svgeesus how should we add that rationale? Do you want me to update the PR?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI. See ITU Rec.2087 for recommendations on "Colour conversion from Recommendation ITU-R BT.709 to Recommendation ITU-R BT.2020"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@svgeesus I added a note for rec2020
@palemieux why did you post that link?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cabanier ... because ITU Rec.2087 provides two data points that can be useful to implementers and authors:

(a) conversion from Rec 2020 to Rec 709
(b) discusses the two distinct cases, each of which use a different EOTF: "where the goal is to preserve colours seen on a Rec. 709 display" and "In the case where the source is a direct camera output and the goal is to match the colours of a direct Rec. 2020 camera output"

Copy link

@palemieux palemieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See inline comments.

@tabatkins
Copy link
Member

Note that dropping a single large PR with a bunch of unrelated changes and little explanation is not good etiquette. This would have been vastly better as a set of PRs, one for each set of related changes, with an actual explanation of what each PR was trying to do and why.

@svgeesus
Copy link
Contributor

Agreed. For some of them I can see (and agree with) the rationale; for others I asked questions which will need answers before merging.

@cabanier
Copy link
Member Author

The changes are mostly in the definition of the profiles so it's not as big of a change as it seems.

I'm tracking down where this transfer function for 2020 is coming from.

@svgeesus
Copy link
Contributor

If you pulled the chromaticity values out of an ICC profile and it is a v.4 profile, be aware that those chromaticities are already adapted to D50 and the adaptation applied is in the chad table. For this spec we need the unadapted primaries, as chromatic adaptation to D50 is already performed in the XYZ to Lab step.
The primaries and transfer function currently in the spec are directly from the BT.2020 spec, which is freely available.

@cabanier
Copy link
Member Author

I commented earlier on the transfer function. I didn't pull the values out of the profile; they were given to me (and different from the ones in the profile)

@@ -1717,7 +1717,7 @@ Profiled, Device-dependent Colors</h2>
or an RGB colorspace (such as ProPhoto <!-- ref -->, <!-- would be good to mention AdobeRGB 1998 here too, need to check trademark and copyright issues first --> widely used by photographers), or any other color or monochrome output device which has been characterized.
In addition, for convenience,
CSS provides two predefined RGB color spaces:
p3d65 [[!DCI-P3]],
display-p3 [[!DCI-P3]],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per #557 (comment) , this colorspace is for "targeting input (colors, images, videos, etc) and not displays", so why call it "display-p3"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@palemieux you are correct! Sorry. I will fix this asap

@@ -1717,7 +1717,7 @@ Profiled, Device-dependent Colors</h2>
or an RGB colorspace (such as ProPhoto <!-- ref -->, <!-- would be good to mention AdobeRGB 1998 here too, need to check trademark and copyright issues first --> widely used by photographers), or any other color or monochrome output device which has been characterized.
In addition, for convenience,
CSS provides two predefined RGB color spaces:
p3d65 [[!DCI-P3]],
display-p3 [[!DCI-P3]],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RP 431-2 specifies a transfer function (gamma 2.6 and absolute XYZ coordinates) and white point there are no consistent with the use in this document. Also note that the term "p3" is not used once in RP 431-2.

At the very least, only Annex C.1 of RP 431-2 should be referenced. Ideally, RP 431-2 would not be normatively referenced, and instead the document would merely mention that the selected primaries happen to be identical to those specified in RP 431-2.

@cabanier
Copy link
Member Author

cabanier commented Oct 7, 2016

Should I pull something out or can it be merged as-is? (and we can address minor issues later)

@cabanier
Copy link
Member Author

What should I do? break it up or remove features or can the pr be merged?

@svgeesus
Copy link
Contributor

I still don't see a source for the different chromaticities (apart from "they were given to me"). Breaking this PR into smaller chunks would certainly help; some parts of the current PR could be merged as is while other parts are blockers.

@cabanier
Copy link
Member Author

Sorry that I didn't see your latest comment. Let me look into the source of the chromaticities and get back to you.
I didn't actually make that many changes. It looks like a lot because it caused changes in several areas.

@cabanier
Copy link
Member Author

The chromicities come from SMPTE ST 2067-21:2016
Apple’s display profile for the sRGB curve.

@cabanier
Copy link
Member Author

cabanier commented Nov 8, 2016

This PR has been open for a while now. What can we do to get it merged?
I taled to the people at ICC and they also want to see these changes in the document.

Copy link
Contributor

@svgeesus svgeesus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the Display P3.icc from Apple, and unadapting the primaries back from D50 to D65, I get these chromaticities:

Apple Display P3: (note negative Z on red due to adaptation)
red XYZ 0.51512146, 0.24119568, -0.00105286 => 0.6820413379009531 0.31935269068993966
green XYZ 0.29197693, 0.69224548, 0.04188538 => 0.2845480103021146 0.6746323210352004
blue XYZ 0.15710449, 0.06657410, 0.78407288 => 0.15589606631881173 0.06606202221664831

I appreciate the spec reference, but it is not freely available (it costs money to get the spec, so developers won't have it).

I think that keeping the display-p3 name and using the chromaticities above (rounded to 4 significant figures) will end up being more compatible with what is actually being used on the web.

@@ -2653,9 +2667,6 @@ Issue: The code below is only for sRGB, and duplicates the
Otherwise, set the channel's value to
<code>((channel + 0.055) / 1.055) ^ 2.4</code>.

Note: This reverses the logarithmic power scaling of the sRGB gamut,
so the value of the channel is linear with respect to the amount of light produced.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointing out that the value is linear wrt the light produced (i.e., it is additive) still seems valuable.

@svgeesus svgeesus merged commit 216f037 into w3c:master Nov 13, 2016
@cabanier
Copy link
Member Author

I don't see the changes reflected in the public draft. Is there a problem with the publishing process?

@svgeesus
Copy link
Contributor

I don't either, but I do see

CSS Color Module Level 4
Editor’s Draft, 14 November 2016

I see them in the Overview.bs on GitHub.

@cabanier
Copy link
Member Author

something must have gone wrong in the auto-publishing process.
Who should we contact?

@plinss
Copy link
Member

plinss commented Nov 15, 2016

If there's a problem, ping me. In this case, the server was showing the most recent push which was an accidental branch from some time back. I merged the branch and it's now showing the latest version (will look at updating the server to always show the tip).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants