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

[css-color-4] Matrices for sRGB to XYZ and back were (slightly) wrong #5922

Closed
svgeesus opened this issue Feb 3, 2021 · 12 comments
Closed
Labels
Closed Accepted as Obvious Bugfix Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-color-4 Current Work

Comments

@svgeesus
Copy link
Contributor

svgeesus commented Feb 3, 2021

TLDR; very small change, no visible effect, but imprecise round-trip was annoying. Non-normative sample code updated. Paywalled standards are bad and lead to errors.

What was wrong

The normative definition of sRGB in CSS Color 4 is correct. In the non-normative sample code, the matrix to go from linear-light sRGB to CIE XYZ, and the inverse matrix to go from CIE XYZ to linear-light sRGB, was wrong starting at the fourth decimal place.

Why

How did this happen? The official definition of sRGB

IEC 61966-2-1:1999/AMD1:2003
Amendment 1 - Multimedia systems and equipment - Colour measurement and management - Part 2-1: Colour management - Default RGB colour space - sRGB

is a 16 page PDF which you buy from the IEC webstore for 70 CHF. I don't have a copy.

The standard is different from the original definition of sRGB because roundoff errors to 4 decimal places in the original proposal meant that the linear and curved parts of the transfer function were not continuous (although the effect made no difference, at 8 bits per component). The IEC standard fixed that.

So, like others I used the conversion matrices calculated by Bruce Lindbloom in the sRGB sample code.

The impact of this was fairly subtle. If you converted sRGB white, #FFFFFF to display-p3 you got color(display-p3 1.0000557 0.99999258 0.99990441) which is color(display-p3 1.000 1.000 1.000) to four significant figures, but color(display-p3 1.0001 1.0000 0.9999) to five significant figures, and this should be exactly 1 1 1 because both sRGB and display-p3 use a D65 white.

This bugged me.

Converting from some other D65-using colorspace, like color(rec2020 1 1 1) to display-p3, gave a nice satisfying round-trippable color(display-p3 1 1 1). So this imprecision was related to sRGB.

Wikipedia has the conversion matrices, to 8 significant figures which, it claims, come from the earlier version of the standard, IEC 61966-2-1:1999. Which is a 51 page PDF and costs 175 CHF. I don't know whether you need both this and the later standard, or just the later one. I don't have a copy of that, either.

What did I change

Today I recalculated the sRGB ones, based on the sRGB primary chromaticities and white point. When rounded to 8 decimal places, they are exactly identical to the ones that Wikipedia has. This gives me renewed confidence in those figures.

var M = [
		[ 0.41239079926595934, 0.357584339383878,   0.1804807884018343  ],
		[ 0.21263900587151027, 0.715168678767756,   0.07219231536073371 ],
		[ 0.01933081871559182, 0.11919477979462598, 0.9505321522496607  ]
	];
var Minv = [
		[  3.2409699419045226,  -1.537383177570094,   -0.4986107602930034  ],
		[ -0.9692436362808796,   1.8759675015077202,   0.04155505740717559 ],
		[  0.05563007969699366, -0.20397695888897652,  1.0569715142428786  ]
	];

Here are what Wikipedia claims are the official ones, for the 1999 version of the standard:

var M = [
		[ 0.41239080,  0.35758434,   0.18048079  ],
		[ 0.21263901,  0.71516868,   0.07219232 ],
		[ 0.01933082 , 0.11919478,   0.95053215 ]
	];
var Minv = [
		[  3.24096994,  -1.53738318,  -0.49861076 ],
		[ -0.96924364 ,  1.87596750,   0.04155506 ],
		[  0.05563008,  -0.20397696   1.05697151  ]
	];

It seems that, while I calculated the conversion matrices for the other predefined RGB spaces, I copied the sRGB ones from Lindbloom. And that was the source of the error.

And here are the Lindbloom ones:

var M = [
		[0.4124564,  0.3575761,  0.1804375],
		[0.2126729,  0.7151522,  0.0721750],
		[0.0193339,  0.1191920,  0.9503041]
	];
var Minv = [
		[ 3.2404542, -1.5371385, -0.4985314],
		[-0.9692660,  1.8760108,  0.0415560],
		[ 0.0556434, -0.2040259,  1.0572252]
	];

What difference does this actually make

In practical terms, none. The deltaE2000 between color(display-p3 1.0000557 0.99999258 0.99990441) and color(display-p3 1.0000000 1.0000000 1.0000000) is 0.015, and a deltaE2000 of less than 1 is difficult to see.

However, given the problems that sRGB already had from excessive roundoff error, it is satisfying that with this change there is no roundoff error to 8 decimal places. This does affect the values in one WPT test, but does not affect the pass/fail since the difference is very small, invisible to the eye, and hard to measure accurately even with a good spectrophotometer.

matrixmaker.html is blank

Thanks for noticing. To change what matrix is calculated, edit the source. To see the result, open the console.

@svgeesus svgeesus added Closed Accepted as Obvious Bugfix Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. labels Feb 3, 2021
svgeesus added a commit that referenced this issue Feb 3, 2021
@svgeesus svgeesus added the css-color-4 Current Work label Feb 3, 2021
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this issue Feb 8, 2021
…t spec

https://bugs.webkit.org/show_bug.cgi?id=221533

Reviewed by Alex Christensen.

Source/WebCore:

Update values to keep in sync with w3c/csswg-drafts#5922.

Updates ExtendedColor API test.

* platform/graphics/ColorConversion.cpp:
Update values to keep in sync with the values used in the CSS Color 4 spec. This
doesn't have any user visible effect, but would reduce errors if round tripping
through this ever became necessary.

Tools:

* TestWebKitAPI/Tests/WebCore/ExtendedColorTests.cpp:
(TestWebKitAPI::TEST):
Update values to values in P3 <-> sRGB conversion test and update to
use EXPECT_FLOAT_EQ so the values can be seen in the output when thigs
fail.


Canonical link: https://commits.webkit.org/233812@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@272498 268f45cc-cd09-0410-ab3c-d52691b4dbfc
bertogg pushed a commit to Igalia/webkit that referenced this issue Feb 9, 2021
…t spec

https://bugs.webkit.org/show_bug.cgi?id=221533

Reviewed by Alex Christensen.

Source/WebCore:

Update values to keep in sync with w3c/csswg-drafts#5922.

Updates ExtendedColor API test.

* platform/graphics/ColorConversion.cpp:
Update values to keep in sync with the values used in the CSS Color 4 spec. This
doesn't have any user visible effect, but would reduce errors if round tripping
through this ever became necessary.

Tools:

* TestWebKitAPI/Tests/WebCore/ExtendedColorTests.cpp:
(TestWebKitAPI::TEST):
Update values to values in P3 <-> sRGB conversion test and update to
use EXPECT_FLOAT_EQ so the values can be seen in the output when thigs
fail.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@272498 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@svgeesus
Copy link
Contributor Author

Good to see these changes migrating into the implementations. Closing, since the spec is updated and everyone seems to be happy.

@svgeesus
Copy link
Contributor Author

svgeesus commented May 4, 2021

Just for completeness, I see that the Wikipedia sRGB article has suffered an edit war (people reverting each other's edits); and that one edit by an anonymous account replaced the 8-digit matrix values with 4-digit ones which aren't even a correct rounding of the 8-digit values.

@astearns
Copy link
Member

Wow. This is unacceptable, no surprise just how wrong you are

This is crazy.

@ValZapod comments like these are not welcome here. Please read through our Code of Ethics and Professional Conduct to find out why.

https://www.w3.org/Consortium/cepc/

@kainino0x
Copy link
Contributor

FYI to other participants in this thread, there was a short parallel discussion over in #7320 (comment)
in which I changed the decimal values landed in this PR, to rational forms of the same values. Since the values weren't changed, discussion of whether these values are correct is best done here, or in a new issue.

@kainino0x
Copy link
Contributor

I'll note that I'm not familiar with the relevant specs, but I have a vague recollection there are at least two specs that define sRGB (IEC vs ITU-R or something??), and they differ slightly due to varying precision tradeoffs of the target systems. CSS probably picked one over the other; is it possible that the discrepancy is just due to there being two "competing" specs?

@kainino0x
Copy link
Contributor

OK, I probably misremembered. I only heard about the spec differences secondhand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted as Obvious Bugfix Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-color-4 Current Work
Projects
None yet
Development

No branches or pull requests

4 participants
@kainino0x @astearns @svgeesus and others