-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add color spaces for Luv, LCHuv, HSLuv and HPLuv. #339
Conversation
Hi @lloydk
HSLuv and HPLuv are both clamped to sRGB, which is a thing that is useful-but they need a different set of constraints for other spaces. So, for sRGB, no gamut check should be required—but if it's to be used for P3, then the HSLuv allows for saturated colors, and therefore the saturation correlate is relative. HPLuv is clamped so that the saturation correlate is absolute (aka intended to be orthogonally independent) but as a result the clamp only allows pastel, desaturated colors. The hue is pretty stable and orthogonally independent. CIELuv*, LChuv, Lshuv on the other hand are essentially unbounded, and therefore definitely do need the gamut check. LChuv is particularly a nuisance because C is all over the place relative to various hue settings—max C for red is ~179, but that is far out of gamut for all other hues. |
Wow, thanks for this amazing work!! 😍
The thing is, the bigger the PR, the harder it is to review it, so that’s a bit cyclical 😕
Hmm, the issue is in the spec only predefined @svgeesus now I even wonder if
Of course!
Note that since color spaces are just instances of If we introduce a new property to make this simpler, I'd rather not have something that's either a keyword or a
|
Instead of review I probably should have said discussion. I mostly wanted to show how the changes for points 1 & 2 are being used by the new color spaces. Consider a thorough review as optional at this point. |
How about:
|
So as you point out this requires wider changes across more spaces, and adds to the burden of specifying a polar space. I'm missing a discussion on the advantages of this alternative approach? |
By accordingly I'm assuming you mean isPolar. If that's the case then I don't think we have enough information to handle the HPLuv color space which is polar and will have a null gamutSpace because it's a subset of sRGB. So it's not clear to me how the getter would be implemented. I'll try your proposed changes to see if I can make it work. |
So, lets first examine the goals and desirability of adding these spaces. CIE LUV has the advantage over CIE Lab that it provides a 2D chromaticity space, which CIE Lab does not, and that the uv chromaticity diagram is less bad than the older xy chromaticity diagram. But both are bad, they are not at all perceptually uniform, and since 1976 computers have become a bit more advanced so we don't need to calculate light mixtures on a printed chart and measuring with a ruler; we can calculate them in 3D in either a linear-light space or a perceptually uniform space depending on what problem we are solving. CIE LUV has all the disadvantages of CIE Lab in terms of radial hue uniformity, chroma stretching for higher chroma colors, and hue curvature. It also has an additional disadvantage, in that the results of chromatic adaptation in LUV are especially bad, often predicting non-physical colors. There are lots of better ways to predict corresponding colors nowadays. None the less, CIE LUV is, or was, fairly widely used and we already have utility functions to calculate uv chromaticity coordinates so we should probably add it. For one thing, if using a dataset where the colors are given in LUV or in Yuv, it would make transforming to XYZ and thence to some other color space easier. Moving on to CIE LCHuv - this has all the disadvantages of LCHab, and then some, which is why it is rarely used in practice nowadays. It might be worthwhile to add it but frankly I am in no rush. Both CIE LUV and CIE LCHuv have no gamut restrictions. HSL (and HSV, and HWB) are terrible formulations that promise (but do not deliver) usability benefits and perceptual uniformity. They are implemented in color.js solely because HSL is widely used in practice, notably in CSS preprocessors such as SASS, and they are part of CSS Color. There is usually a better way to solve a given problem than using HSL. HSL used to be restricted to the sRGB gamut but in CSS Color we changed that, relatively recently, avoiding a gamut mapping step to prioritize lossless round-tripping of colors outside the sRGB gamut. HSLuv is an unholy union of CEI LUV and HSL, combining the disadvantages of both by adding a non-perceptual saturation to LCHuv, just so that it forms a square shape which is convenient for color pickers. It is also restricted to the sRGB gamut and thus a strange anachronism in a world where the major RGB color spaces are Display P3 and BT.2020/2100. I would want to see some more justification for adding this, especially if doing so involves refactoring other color spaces. |
get gamutSpace() {
return this.isPolar ? this.base : this;
} But if a So HPLuv can just provide |
@LeaVerou wrote:
Using a custom color function and thus, a dashed ident as the colorspace name, is probably the way to go.
I have wondered that for a while, currently in CSS Color 5 pointing to an ICC profile is the only way to add an implementation of a custom color space which is certainly needed for CMYK and suchlike spaces; for additive RGB spaces it is needless overhead and what is needed is primary and white chromaticities plus a transfer curve (EOTF) formula. We might prototype that in color.js and it might even make it's way into CSS Color or the Color API as an extensibility point. |
It doesn't help that the website for HSLuv not only uses the term "RGB" to mean
|
I think that's a fair question to ask. I'm fine with maintaining these color spaces in my fork. |
I wonder how literal this is meant...to have these exact functions and names as such. As an implementer of these spaces in my library, if it is super literal, I'm in violation 🙃. It doesn't fit into my API at all, though the spirit of it is there, the ability to convert from these spaces to sRGB and back. I don't see these requirments explicitly stated in the license, but yeah, it does say this on their website. I guess if the author hunts me down and says I must have these exact functions with these exact signatures, I guess I could reconsider and add functions that no one will ever use, or just remove HSLuv and HPLuv 🤷🏻. With all of that said, I can definitely see the argument that as we move into a wider gamut world that many of these HSL-ish spaces seem less useful. Though, I can also understand that there will always be applications stuck with sRGB and people wanting something more perceptually uniform, even if those solutions are imperfect. Regardless, it doesn't mean every color library is obligated to directly support and maintain them. |
@svgeesus The only reason I can think of for not adding a color space is that it's not popular enough. These color spaces are very popular, and their lacking in color.js has been noticed by many. Color.js is a tool where we can experiment with every popular color space, and has plenty of other spaces we don't plan to add natively to CSS. It's useful for comparisons, as well as to see what needs different color spaces have so that the Color API is flexible enough. It's the same reason why you implemented a bunch of contrast algorithms, some of which are very clearly suboptimal: so we can use them in comparisons. 😊 |
There are currently six color spaces that are polar:
Only three of those spaces (hsl, hsv, hwb) have a base space with coordinates that have range values. The Of the three new polar spaces in this PR none of them would do any gamut checks using the base space:
I don't think it's a burden when defining polar spaces. It's extra line for new polar spaces that have a base space with range values that aren't gamut checked in some other space. For the three new spaces in this PR there is no burden as none of the spaces are gamut checked in the base space. Same for OKhsl and OKhsv should those spaces get added. If there are other unbounded polar spaces that get added there would be no extra work to specify those spaces. Only three of the existing color spaces would need to specify a I think checking |
As someone who has implemented both HSLuv and HPLuv, the only way to provide a default gamut check/gamut map for HPLuv is to have its default be itself, so if HPLuv is meant to be in color.js, there would have to be a way for it to do that if no gamut space is provided as the base conversion space is simply not sufficient for a default gamut mapping. I personally agree that it would be cleaner to use something like |
The current heuristic works across all six of them though 😄 For the first three because they have a base space with coordinate ranges, and for the last three trivially, because they have no gamut, and their base space has no ranges. The base space always returns true, because that's the correct answer!
I think there may be a misunderstanding here. The design I proposed only checks Reading @facelessuser's comment I'm even more inclined to think that you both misunderstood what I was proposing. Hopefully now that I explained it better it's clearer? |
It is likely I misunderstood and didn't read close enough. |
Actually, it's possible I did! I think I just realized what you were saying @facelessuser: there is no way to override the smart default if the gamut space is the space itself, but the space is also polar, because we don't have a reference to the space until after it's created, so nothing to pass to Hmm hmm. So the options are:
let foo = new ColorSpace(...);
foo.gamutSpace = foo; I think out of those, (1) is the least bad. Not sure if there are any other options? |
As gently as possible (that's my vain attempt to not be controversial):
Absolutely agree, I also agree that LChuv has a wacky-way-out Chroma correlate. And we have much better color spaces now, and fairly mature LUT pipelines, so does LUV/LChuv/Lshuv have relevance? I'll suggest it does for certain applications.
Lshuv is markedly better in hue distribution and freedom from the blue/purple shift. CIELUV is substantially more uniform in hue than CIELAB. This comparison is using polar on the top row, cartisian on the bottom row. Top row second from left is LUV polar with the saturation instead of chroma . Lshuv with sat correlate substantially tames the chroma bronco-bull. There are modifications that can be applied to reign in some issues. E.g. I don't think L* is the right correlate for lightness to build everything around either, and accounts for a healthy chunk of LUV's MacAdams ellipse issues. L* is Munsell value, created around diffuse reflecting tiles in a defined env.
LUV should never be used for chromatic adaptation, it's not fit for that. As it happens, the things LUV is good for should not require that: start and end in D65, and working space should be the destination space when practical.
No disagreement here... though I don't think @lloydk is doing anything with those?
Probably inevitable... is there a flag/meta for the color space?
Well, the main reason to employ HSLuv (or Lshuv), is for the purpose of picking colors, and making gradients, etc. In this role, it performs well. It's computationally inexpensive, intuitive to use, much more uniform than HSL, no significant purple shift like LAB, and can be clamped to a given space, preventing out of gamut clipping.
sRGB is still the default, that's "major" yea? And there are some accessibility reasons that sRGB and Adobe98 are preferred over BT.2100 etc (those issues can be remedied with universal user personalization). The means to calculate the constraints for HSLuv to be clamped to a particular space is in the repo. It does not look terribly difficult to generate the constraint matrix for any given space.
OpenColorIO and LUTs... We use LUTs in film/TV because they are fast, easy, stackable, and in addition to transformations from a space to a space or device, it's easy to create a look and apply it across media, easy(er) to do gamut mapping, etc. Moreover: easy to create. For instance Davinci Resolve has a free version that is usefully capable. Once you get a look or grade you like, Resolve can bake it into a LUT, or bake several LUTs together, and send it wherever needed, in a click. But then, there is Open Color IO, open source complete color management system. Highly configurable, the full configuration can output many flavors of LUT or other transform methods, including ICC profiles. My thought is not to use the full implementation, just enough of the API for connecting a LUT for in and out. Sort of an OCIO lite—with all the new color spaces, and the many to come, and the fact that OCIO LUTs work well with 32bit float and half float, it feels like the direction to examine.... Y 4 R There LUV
Well, HSLuv is popular in DataViz, in part because of the saturation correlate, even if relative, it is still somewhat stable (better than the C). As I mentioned, Hsluv is best for color picking, gradients, etc. It may not be perfect, but it's well suited to the job of color selector/setter. |
Still not a big fan of option 1. If feels like we're having to jump through hoops in order to avoid adding this line to three color spaces:
With option 2:
|
Not sure where we're at as far as proceeding with adding these new color spaces. If they're not going to be added then I'll just close this PR. If they're going to be added the only thing I have to add regarding the For the coord grammar change I know that it's not spec compliant but even if you restrict the grammar to numbers and percentages a grammar is still required on the Luv color space to handle negative reference range values. If angles aren't allowed then it's not really clear to me how Hue should be defined in the LCHuv HSLuv and HPLuv color spaces (and OKhsl/OKhsv if they were to be added) |
I think the consensus among maintainers is that they should be added, but @svgeesus has some reservations, that may require more discussion. Definitely don't close the PR though!
These are not the same. Not specifying it may or may not resolve to self, depending on
The option you are advocating is not backwards compatible, requires changes to existing spaces, and adds more boilerplate to defining a new space. I'm happy to discuss alternative designs, but making the gamut space default cover fewer use cases than it currently does and requiring more overrides is a non-starter.
@svgeesus This is actually excellent feedback for how the |
Hi @lloydk, Thank you for your patience!
As an update, @svgeesus and I just discussed it, and we have consensus that:
|
Oh, and for tests you'd likely want to use the new format: https://github.com/LeaVerou/color.js/blob/main/tests/conversions.js (would love feedback on it too!) |
"name": "Luv", | ||
"id": "luv", | ||
"url": "https://en.wikipedia.org/wiki/CIELUV", | ||
"description": "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does need a description, but that can be added later
refRange: [0, 100], | ||
name: "Lightness" | ||
}, | ||
c: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LeaVerou LCHuv, unlike LCHab has a saturation as well. Saturation is chroma / lightness. How should that be exposed, as a utility method? Or can lchuv.s
be made to work somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MVP won't have it, and you (or someone else) needs to open a new issue to discuss how to handle this in the API. 🥲
Hi everyone, I just got reminded of this from this tweet. |
I'm waiting for #369 and #370 to be merged before proceeding with PR's for the color spaces. Was also potentially waiting for the parsing tests to be ported to the new format. |
Somehow these fell completely off our radar, so sorry @lloydk!! I just reviewed one and asked a clarifying question on the other. In general, I'm personally very keen to add support for these color spaces, so if you see any delays on our part, please don't be afraid to ping us, we're not ghosting you, we're just spread very very thin. 😊 |
This is a draft PR for adding the Luv, LCHuv, HSLuv and HPLuv color spaces.
It's probably to big for single commit so I plan on breaking it up into four smaller pull requests after it has been reviewed:
The Javascript reference implementation for HSLuv and HPLuv can be found here. The HSLuv and HPLuv color spaces are based on the reference implementation so I've add the license file from that repository to the source code for those two color spaces. I'm assuming that's ok.
Gamut Checking
I wasn't sure how to do gamut checking for HSLuv and HPLuv. The
ColorSpace
inGamut
function uses the base space if the current space is a polar space. This doesn't work for HSLuv and HPLuv because the base space has a larger gamut.I added a new property to the
ColorSpace
class calledgamutCheck
which is either a color space that should be used for gamut checking (sRGB in the case of HSLuv) or"self"
which means use the current color space for gamut checking. HPLuv uses"self"
because it has a gamut that is a subset of sRGB.