w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 661 forks source link

[css-color-5] Consider using the same identifiers for color-adjust() and relative color syntax #6142

Closed weinig closed 3 years ago

weinig commented 3 years ago

As I realize they are kind of competing with each other, I think it would make it a bit easier to compare color-adjust() (https://drafts.csswg.org/css-color-5/#coloradjust) and relative color syntax (https://drafts.csswg.org/css-color-5/#relative-colors) if they used the same keywords for color channels.

For example, color-adjust() uses red, green, and blue, while relative color syntax uses r, g', andb`.

I think using the more verbose but clear keywords from color-adjust() would be preferable, but consistency either way would be nice.

tabatkins commented 3 years ago

Ah, so the reason I initially wrote color-adjust() to use the more verbose words was because it wants to auto-detect the colorspace in use, so the adjusters need to be unique, or at least shared only by spaces that give them the same meaning. If we do as you suggest in #6141 and make the adjustment colorspace required, then this is no longer necessary, and we can have, say, b represent blue (rgb), black (hwb), and "b" (Lab) simultaneously, since it's no longer ambiguous.

I prefer the shorter keywords from relative colors, because (a) they match the character already used in the function name, so it's intuitively clear what they mean, and (b) it produces shorter code overall, especially vs keywords like lightness; these are already capable of producing pretty long values.

(Tangent, but actually I think I made a mistake anyway; HSL and LCH/Lab lightness aren't identical, right? hsl(0 0% 50%) is RGB 50% gray, but lch(50% 0 0) is not.)

weinig commented 3 years ago

Consistency is my biggest goal here, so if the shorter ones are more preferred, that would be ok.

LeaVerou commented 3 years ago

Ah, so the reason I initially wrote color-adjust()

@una proposed it and specced it. I think what you are referring to is the old color-mod() but color-adjust() is not really based on that.

to use the more verbose words was because it wants to auto-detect the colorspace in use, so the adjusters need to be unique, or at least shared only by spaces that give them the same meaning. If we do as you suggest in #6141 and make the adjustment colorspace required, then this is no longer necessary, and we can have, say, b represent blue (rgb), black (hwb), and "b" (Lab) simultaneously, since it's no longer ambiguous.

Please note that we are making the color space mandatory for now. Eventually, the goal was to be able to omit it, although autodetecting it from the component names sounds suboptimal as well, so maybe we should keep it mandatory.

Another reason to use more verbose names in color-adjust(): by design, you only need to specify the coordinates you are modifying, whereas for RCS you need to specify all components, even those that remain unchanged, so brevity is more important. I am indifferent about changing the names in color-adjust(), but would oppose using more verbose names in RCS because that would render it a huge pain to use.

(Tangent, but actually I think I made a mistake anyway; HSL and LCH/Lab lightness aren't identical, right? hsl(0 0% 50%) is RGB 50% gray, but lch(50% 0 0) is not.)

👀 The whole point of Lab/LCH is that they are not identical! 50% HSL lightness doesn't reflect 50% perceptual lightness, even despite gamma correction, which brings it much closer than it would be with linear RGB. See also this example from the spec.

tabatkins commented 3 years ago

I think what you are referring to is the old color-mod()

Ah yeah, I am. They're closely related, tho; see https://drafts.csswg.org/date/2017-10-11T14:48:05/css-color-4/#modifying-colors which is very similar in overall syntax.

but color-adjust() is not really based on that.

It is, in fact; Una's proposal came out of discussions between her, me, and Adam Argyle about this topic, based on my earlier attempts in the space, and I believe the proposal was based directly off of reading my older proposal and modifying it to have less ()-nesting. I pushed for Una to handle spec edits as a nice First Spec to work on (and to make sure I wasn't overly burdened with Yet Another Spec to work on ^_^).

Eventually, the goal was to be able to omit it, although autodetecting it from the component names sounds suboptimal as well, so maybe we should keep it mandatory.

We should def make that decision now, since as shown here it'll influence the optimal names.

I agree, tho, that auto-detecting the colorspace from the used channel names isn't a great strategy, and so just keeping it mandatory is probably the right idea, particularly since that aligns it with the other color-modifying functions. Consistency is good, even if some usages could conceivably be simpler.

The whole point of Lab/LCH is that they are not identical! 50% HSL lightness doesn't reflect 50% perceptual lightness, even despite gamma correction, which brings it much closer than it would be with linear RGB.

Right. ^_^ So yeah, that definitely throws lightness as a reasonable name out the window; we'd have to name them hsl-lightness and lch-lightness and lab-lightness, and at that point, ugh, just let me say I'm working in lab and use a nice short l. If we add more spaces, these sorts of clashes between similarly-named channels will only increase; iirc "chroma" isn't too uncommon across multiple spaces either, and generally has a space-specific definition.

svgeesus commented 3 years ago

I agree, tho, that auto-detecting the colorspace from the used channel names isn't a great strategy, and so just keeping it mandatory is probably the right idea, particularly since that aligns it with the other color-modifying functions.

Strongly agree. It is the clearest, simplest, and most consistent way forward.

iirc "chroma" isn't too uncommon across multiple spaces either

Yes. LCH, JzCzHz and OKLCH all have Chroma.

LeaVerou commented 3 years ago

Yup, agreed that the color space should be mandatory.

Beyond that, is consistency with RCS the only reason to make the names single letter, or is it an independently good change?

tabatkins commented 3 years ago

Tangent: oh geez even hue isn't consistent across spaces. Obvious once I think about it, but didn't occur to me at first.

Beyond that, is consistency with RCS the only reason to make the names single letter, or is it an independently good change?

I think it's a good change on its own, for a few reasons:

  1. The channel names are right there in the function name. This is the same reason they were good to use in relative colors, because it's obvious right from our naming conventions what value is being taken from where. Same applies here - if you're adjusting in lch space (and the space name is mandatory, so it's definitely right there at the start of the function), then the h name has a very obvious source and meaning.
  2. Channel names aren't always obvious, and they're often slightly complex English words, or even specialized jargon not used in normal English. (a) In hwb(), you can't predict whether the second channel should be called white or whiteness, for example; in cmyk(), it's not clear whether k stands for black or key to someone not already steeped in color jargon (b) In hsl(), saturation and lightness are long. (c) in lch(), "chroma" is technical jargon. We avoid all of these by just using the single-letter names we're already using for each channel.
  3. Several spaces use single-letter channel names anyway, like the "a" and "b" channels of Lab, and it just feels weird to me that some spaces get to have these arbitrary short and easy names, while others have to spell out long names. This is definitely an aesthetic judgement, however.
LeaVerou commented 3 years ago

Yup, this is good reasoning, agreed. Especially the part about names being hard to remember. I'm in favor to make them single letter now.

Tangent: We should also add a descriptor to @color-profile for supplying coordinate names, then RCS and color-adjust() would be possible for arbitrary color spaces.

tabatkins commented 3 years ago

Oooh, yes, that would be nice.

svgeesus commented 3 years ago

Tangent: We should also add a descriptor to @color-profile for supplying coordinate names, then RCS and color-adjust() would be possible for arbitrary color spaces.

I believe @Crissov had previously suggested something on those lines, though I can't find it just now. IIR it went well beyond naming, though.

But yes, especially for multichannel colorspaces like CMYKOGV, using names rather than just position in the parameter list is a win.

LeaVerou commented 3 years ago

@una @argyleink are you ok with this proposed change?

argyleink commented 3 years ago

i'm good with it yep 👍🏻 for all the points mentioned above (tldr; less to remember because we're merging like items)

and, just so i'm understanding it all correctly, that would result in spec changes ~like this:

<srgb-adjuster> = r | g | b
<hsl-adjuster> = <> | s | l
<hwb-adjuster> = <> |  w | b
<xyz-adjuster> = x | y | z
<lab-adjuster> = l | a | b
<lch-adjuster> = l | c | <>

for use ~like this:

color-adjust(in hsl, peru l -20%);
color-adjust(in lab, hotpink b -20%);
LeaVerou commented 3 years ago

This is tangential, but I think the adjusters should be separated from the color via commas. That also allows us to extend the grammar to support multiple adjusters in the future.

tabatkins commented 3 years ago

@argyleink I'm not sure what the <>s in your comment are meant to be?

Crissov commented 3 years ago

I suggested a composition descriptor for @color-profile in #4488, which contained the CIE xy chromaticities of the primaries. It could be adapted for the purpose described here, if I understand it correctly.

@color-profile "sRGB" {
  composition: r 0.64 0.33, g 0.30 0.60, b 0.15 0.06;
}
tabatkins commented 3 years ago

This is tangential, but I think the adjusters should be separated from the color via commas. That also allows us to extend the grammar to support multiple adjusters in the future.

That's not strictly necessary, so long as the grammar for each adjuster stays in its current simple form. color-adjust(hsl, red h 20deg l 20%) is unambiguous. I admit it's not great to read, tho; I'd be fine with commas between them if necessary.

Hmm tho, now that I'm looking at color-adjust() again, I've got some thoughts I'll raise in another issue...

argyleink commented 3 years ago

@argyleink I'm not sure what the <>s in your comment are meant to be?

they're where the dynamic links normally are, but when i copied from the spec it was in a raw form / uncompiled. figured it didnt matter as the gist of the spec note was the letter instead of the word. sorry that was confusing.

svgeesus commented 3 years ago

@LeaVerou wrote:

I'm in favor to make them single letter now.

Does that include alpha? Because renaming it to a will clash with a in lab. I propose we keep alpha therefore.

tabatkins commented 3 years ago

Agree; that's consistent with what I'm doing in Typed OM right now too.

svgeesus commented 3 years ago

@LeaVerou said

We should also add a descriptor to @color-profile for supplying coordinate names,

Done

Then RCS and color-adjust() would be possible for arbitrary color spaces.

Also done although, as we discussed, depending on a descriptor which might get added or removed over the document lifetime gets us into invalid at computed value time, rather than parse time, errors.

svgeesus commented 3 years ago

Closed by WG resolution to drop color-adjust()