casesandberg / react-color

:art: Color Pickers from Sketch, Photoshop, Chrome, Github, Twitter & more
http://casesandberg.github.io/react-color/
MIT License
11.95k stars 914 forks source link

Wrong Brightness Coefficients #184

Open lgg opened 8 years ago

lgg commented 8 years ago

https://github.com/casesandberg/react-color/blob/master/modules/tinycolor2/index.js#L76

For now there are coefficients for NTSC, not sRGB. And i think that w3c is little bit outdated.

Also see this demo: http://codepen.io/dom1n1k/pen/LZWjbj

See sRGB table: http://www.brucelindbloom.com/index.html?WorkingSpaceInfo.html Or point 9 here: http://www.poynton.com/notes/colour_and_gamma/ColorFAQ.html

lgg commented 8 years ago

The formula using to calculate relative luminance (or brightness): Y = 0.299 R + 0.587 G + 0.114

But it is correct in NTSC (1953) color space only. This formula is deprecated and legacy. Actual color space sRGB significantly different: chromacity diagram.

Correct formula is: Y = 0.2126 R + 0.7152 G + 0.0722 B

The proof of numbers Source — Bruce Lindbloom site See also — Charles Poynton Color FAQ

A lot of work or refactoring is not required. We can simply replace this coefficients.

casesandberg commented 7 years ago

Oh great! I will look into replacing this soon

casesandberg commented 6 years ago

I want to make sure we get the color conversion as accurate as possible in the new v3.0.0. Do you have any suggestions on packages or resources that would allow us to drop tinycolor altogether?

I was also talking to @chrisgervang about this so maybe he has ideas / can help out.

lgg commented 6 years ago

@casesandberg hmm, i can't find any Brightness Coefficients in react-color code right now. tinycolor is another package or you changed smth else?

lgg commented 6 years ago

https://github.com/casesandberg/react-color/blob/b0317ffff96494bcb4b3f8e76ec2eef61bba5073/modules/tinycolor2/index.js

chrisgervang commented 6 years ago

I'm not exactly sure what can be done about the Y coefficients. Is this about the Y in CMYK? If so, sRGB doesn't have 100% overlap with CMYK, so converting between the two spaces will always be lossy (depending on the color).

We could weight each color, but doing that on the 24-bit RGB (255, 255, 255) that comes with the browser would knock some colors outside of this range if you were to convert it between CMYK and RGB, right?

Regarding tinycolor2, the color conversion package used in react-color, I'm not sure how it is at fault if we're talking about CMYK. However, the hsl <-> rgb conversion doesn't produce a linear perceived color change, so functions like lighten, brighten, darken, desaturate, and saturate may not act fantastically.

In this example I compared 3 npm packages:

screen shot 2017-10-17 at 1 15 48 am
chrisgervang commented 6 years ago

Oh wait, is this the function you're talking about @lgg? https://github.com/casesandberg/react-color/blob/669bf040955d2b8ff82b491e1044d28fe80d06b5/src/helpers/color.js#L62 If so, I don't see any harm in changing the coefficients.

@casesandberg wrt to removing tinycolor, it looks like its only used in this file https://github.com/casesandberg/react-color/blob/669bf040955d2b8ff82b491e1044d28fe80d06b5/src/helpers/color.js#L2 I don't actually see a reason to remove it because react-color isn't using any the functions I mentioned above that have issues. But, if you were interested in adding hsluv in addition to hsl, which can be used to make different "shades" of the same color WAY easier, that would be cool.

hsluv and hsl have the same parameters. But hsl produces shade as seen in the shader npm package in my example, whereas hsluv produces a "truer" shade. I noticed material color uses something like hsluv to produce their shades.

lgg commented 6 years ago

@chrisgervang yeah, about this coefficients: https://github.com/casesandberg/react-color/blob/669bf040955d2b8ff82b491e1044d28fe80d06b5/src/helpers/color.js#L62

Myndex commented 5 years ago

HI @chrisgervang

I'm not exactly sure what can be done about the Y coefficients. Is this about the Y in CMYK?

The Y is Luminance as in CIEXYZ, linear relative luminance, not related to Yellow in CMYK.

Unfortunately in colorimetry we have a lot of confusingly similar terms, which exacerbates an often confusing and abstract subject.

Luminance: A linear measure of LIGHT. L measured in cd/m^2 or Y for a relative measure, 0.0 - 1.0 as in CIEXYZ colorspace. Note that it is spectrally weighted per human vision, but NOT lightness-weighted. This is the reason for the coefficients.

Perceptual Lightness: The way we perceive light, L* (Lstar) not to be confused with L !! L* models the non-linear aspect of human vision (it is an attempt at a linear measure of perception so is NON linear regarding light).

Luma: a non-linear function used for certain video encoding schemes. It is denoted (Y prime), it has a gamma curve and is not linear, and should not be confused with linear luminance.

Y´IQ: The video encoding method used in NTSC. If you are doing Y´IQ then I assume your are trying to emulate NTSC, and then should use the NTSC coefficients (as you are), and those are used in a gamma encoded space (this results in inaccuracies, but is the accepted method). If going from sRGB to NTSC/Y´IQ it is "best practices" to first transform sRGB to NTSC RGB, then convert to Y´IQ.

sRGB: The computer and internet standard. It uses the Rec709 coefficients, and used them IN LINEAR SPACE. That is first linearize sRGN to linear RGB, then apply the Rec709 coefficients (Y = R 0.2126 + G 0.7152 + B * 0.0722).

Y = linear Y´= nonlinear (nominally gamma 1/2.2) L = linear L* = nonlinear (approximately gamma 1/2.4)

However, the hsl <-> rgb conversion doesn't produce a linear perceived color change,

HSL is not even remotely perceptually uniform. It's just a model that makes it "easy" to choose colors within software.

For a more perceptually uniform model, try CIELAB LCh

If you are wanting to write software so that a change of n is perceptually the same where ever n is applied, then use a perceptually uniform model such as CIELAB or CIELUV.

If you want to model how LIGHT acts in the real world, then use a light-linear model such as CIEXYZ, or linearized RGB.

I hope this clarifies and not obfuscate the issue...

indo-dev-0 commented 4 years ago

TinyColor's codebase hasn't been updated in 3 years, I would investigate Chroma.js or d3-color as a replacement. AFAICT, Chroma.js is about the same size and has nearly equivalent functionality. It also has a (more) accurate contrast formula.

stefanKuijers commented 4 years ago

Came to this issue as an OWASP check gives warnings about vulnerabilities cause tinycolor2 uses jQuery v1.9.1