Skip to content

[css-typed-om] Do we need separate classes per color space? #1034

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

Open
LeaVerou opened this issue May 12, 2021 · 5 comments
Open

[css-typed-om] Do we need separate classes per color space? #1034

LeaVerou opened this issue May 12, 2021 · 5 comments

Comments

@LeaVerou
Copy link
Member

Given that the entire set of numeric types is represented by a single CSSUnitValue class, I was surprised to see the class hierarchy of CSSColorValue. Is this complexity and rigidity needed? Note that this requires adding new classes and new methods on every CSSColorValue for every single color space supported (and it makes it harder to support custom color spaces).

Why not just

[Exposed=(Window, Worker, PaintWorklet, LayoutWorklet)]
interface CSSColorValue : CSSStyleValue {
	constructor(CSSKeywordish colorspace, sequence<CSSNumberish> coords, optional alpha);

	attribute CSSKeywordish colorspace;
	attribute sequence<CSSNumberish> coords;
	attribute CSSNumberish alpha;

    CSSColorValue to(CSSKeywordish colorspace);

    [Exposed=Window] static CSSColorValue parse(USVString cssText);
};

Though I'm not sure how to represent the difference between e.g. color(srgb...) and rgb(...). Perhaps an additional type attribute.

@tabatkins
Copy link
Member

The unit types don't need separate classes because they're all the exact same thing - a single number paired with a single short string for the unit. There's nothing about their functionality that requires different behavior between them, either - all you want for them is math, and they all act identically there.

Colors are pretty strongly different, imo. Each color function has a different shape; they're generally all three/four-argument, but the arguments represent different things, and have different types. I don't think treating them identically as a list of args helps authors - in color.r vs color.coords[0], the latter seems substantially worse in understandability.

This feels similar to trying to unify the transform functions into a single class with a list of args, which would similarly be bad for authors with dubious benefit, if any, to specs and impls.

Though I'm not sure how to represent the difference between e.g. color(srgb...) and rgb(...). Perhaps an additional type attribute.

I think this is a code smell that suggests the merging wouldn't be a good thing. ^_^

Note that this requires adding new classes and new methods on every CSSColorValue for every single color space supported

It does, but is this a bad thing?

(I'm also not quite sure what you mean by "on every CSSColorValue" - each new color class would add one method to the superclass; instances would see it by inheritance, as usual. Is there some multiplication of methods I'm missing?)

(and it makes it harder to support custom color spaces)

I don't understand this bit - what about the design makes it harder? Custom color spaces is just through color(), right? That already has the generic design you're suggesting (by necessity; it's the clearest, most direct way to represent color() values). Or are you talking about a different sort of custom color feature, maybe something like I sketched in my earlier attempt at Color JS stuff a few years back?

@LeaVerou
Copy link
Member Author

LeaVerou commented May 13, 2021

Each color function has a different shape; they're generally all three/four-argument, but the arguments represent different things, and have different types.

In terms of Typed OM, they are all CSSNumericValue, aren't they?

I don't think treating them identically as a list of args helps authors - in color.r vs color.coords[0], the latter seems substantially worse in understandability.

That's a strawman, I wasn't suggesting color.coords[0] to be the only way to get a coordinate, but color.coords to get all coords as an array is useful.
In terms of getting coordinates by name, there can be convenience syntax, e.g. color.get("r"). But right now, if authors want to get the list of coords generically, the only way is to manually check all possible getters, which is ...not great DX.

Also note that this design does not allow for named getters for custom color spaces even if they are named in the @color-profile rule, since we are moving away from exotic objects in the Web Platform. Whereas a maplike design would allow for named getters for any supported color space via .get() and .set(), and you could even allow setting/getting coords of different color spaces (e.g. color.get("lch.l") without the overhead of converting to that color space and back.

In Color.js we actually support things like color.lch for the array of coords and e.g. color.lch.lightness for coords (both read-write) for every supported color space, but not sure if that would be feasible (in terms of Web IDL) or even desirable in a native API. However, a .get()/.set() syntax definitely is.

Though I'm not sure how to represent the difference between e.g. color(srgb...) and rgb(...). Perhaps an additional type attribute.

I think this is a code smell that suggests the merging wouldn't be a good thing. ^_^

I could argue the same for CSSUnitValue#unit being "number" when you have a number.

Note that this requires adding new classes and new methods on every CSSColorValue for every single color space supported

It does, but is this a bad thing?

(I'm also not quite sure what you mean by "on every CSSColorValue" - each new color class would add one method to the superclass; instances would see it by inheritance, as usual. Is there some multiplication of methods I'm missing?)

Several reasons:

a) It's currently very difficult to find out the color space of a color:

let space = color instanceof CSSColor? color.colorspace : Object.prototype.toString.call(color).match(/\[object CSS(\w+)\]/?.[0];

and that's assuming these subclasses are reflected in these objects' [[Class]] property (and assuming the fix in #1031 actually gets pushed). If not, it's even more complicated.

You could add a property to reflect the color space, but then you'd have multiple sources of truth.

b) It makes it very clunky to make manipulations of colors without changing a color's color space:

// Find color space
let space = color instanceof CSSColor? color.colorspace : Object.prototype.toString.call(color).match(/\[object CSS(\w+)\]/?.[0];

color = color.toLCH(); 
color.l *= 1.2; // actual manipulation

// Back to original color space
if (("to" + space) in color) {
	color = color["to" + space](); 
}
else {
	color = color.toColor(space);
}

This should really be one line, and is one line in literally every color manipulation library out there.

Also, converting to an arbitrary color space is really clunky too, as we can see from the example above.

c) It makes it harder for authors to polyfill Typed OM for future color functions since they cannot create objects that have a custom [[Class]] property.

d) It privileges the color functions over color() (see also my point above about named getters and custom color spaces)

@tabatkins
Copy link
Member

In terms of Typed OM, they are all CSSNumericValue, aren't they?

Yeah, I meant CSS types. (Sorry, the terminology gets confusing since there's not a 1:1 relationship there.)

That's a strawman, I wasn't suggesting color.coords[0] to be the only way to get a coordinate, but color.coords to get all coords as an array is useful.
In terms of getting coordinates by name, there can be convenience syntax, e.g. color.get("r").

Its not a strawman, it's literally what you suggested, according to your sample IDL. ^_^ If you meant to imply that there'd also be some sort of maplike interface to interact with the channels by name, it didn't come thru in your message, sorry.

But right now, if authors want to get the list of coords generically, the only way is to manually check all possible getters, which is ...not great DX.

I'm not sure I understand the concern here. What sort of code is being written that wants to interact with arbitrary color objects and manipulate their channels without any knowledge of what the color space is? I don't think there's any math you can meaningfully perform using an arbitrary list of channels, which might even be a mix of percentage and angle values.

Could you elaborate?

Also note that this design does not allow for named getters for custom color spaces even if they are named in the @color-profile rule, since we are moving away from exotic objects in the Web Platform. Whereas a maplike design would allow for named getters for any supported color space via .get() and .set(), and you could even allow setting/getting coords of different color spaces (e.g. color.get("lch.l") without the overhead of converting to that color space and back.

Yeah, a maplike interface giving a way to address the coords of a color() function by name (when there's an appropriate @color-profile providing names for them) definitely makes sense.

I could see us potentially moving that method to the superclass, I suppose. I think it'd need a little more justification.

a) It's currently very difficult to find out the color space of a color:

Ah, that's reasonable to fix. I already do this for CSSNumericValue - the operation subclasses are separate subclasses, but also expose their corresponding function name in a property, to make it easier to respond to. I can see moving ".colorspace" to the superclass as a readonly property, and making it writable in CSSColor.

I just filed #1036 to remind me to take care of this.

(Maybe we should do this for the transform subclasses too...)

d) It privileges the color functions over color() (see also my point above about named getters and custom color spaces)

I don't think it does - the color() function is inherently more arbitrary and less well-defined in its author-exposed interface, because it has to be generic over all possible color space definitions.

That said, you're right that CSSColor is somewhat anti-privileged - it should really be using indexed properties, rather than a separate .channels property. That's my bad; I was just converting the old/weird design I needed for the earlier fallback ability down to a single variant, and what I wrote was the most straightforward way to do it. I've filed #1037 to fix this.

c) It makes it harder for authors to polyfill Typed OM for future color functions since they cannot create objects that have a custom [[Class]] property.

In general, the Typed OM isn't currently being designed to be author-extensible. You can't add new units, etc. either. Nobody seemed to like my efforts to design an author-extensible set of color classes back in Color 4, either. :(

That said, there's definitely room to improve here. I think color functions definitely are a good excuse to be author-extensible. I wonder if we can get away with something quite simple, actually - I filed #1038 to explore this topic.

b) It makes it very clunky to make manipulations of colors without changing a color's color space:

This should really be one line, and is one line in literally every color manipulation library out there.

I explored this in my earlier attempt in Color 4, and thought that implicitly invoking a bunch of colorspace conversions wouldn't be a great idea. And since this is a CSS API, which color function you "really" are is important to preserve.

Also, converting to an arbitrary color space is really clunky too, as we can see from the example above.

Ooh, that's fair. If I fix #1036 as I want, then we can change the conversion method to a generic .to() that can spit out any of the color subclasses. I've added a comment along those lines.

If we do that, then your code collapses down to:

let space = color.colorspace;
color = color.to("lch");
color.l.value *= 1.2;
color = color.to(space);

which seems straightforward and simple while maintaining the important invariants.

Is this sort of "temporarily switch to another colorspace for modification, then switch back" a common thing in your experience? If so, we can make it easier - say:

color.modifyIn("lch", ({l})=>{return {l:l*1.2}});

That is, specify what space you're going to do the modifications in, then give a callback function that takes the color in that destination space, and returns an object with the properties you wanted to modify. (Unspecified props are left alone.) It then converts back to its original color space.

I'd want at least a little evidence that this is something common enough to be worth the additional API surface area, tho.

@svgeesus
Copy link
Contributor

Is this sort of "temporarily switch to another colorspace for modification, then switch back" a common thing in your experience?

Obviously it can be done as you do above, where you create a second object, fiddle with it, then put the results back into the first object. It is nicer for the creation and deletion to happen out of sight though.

Places where these implicit colorspace conversions happen (not necessarily mutating the original):

  • composite c1 over c2 in XYZ or in sRGB
  • interpolate c1 to c2 in colspace to make a gradient
  • mix c1 with c2 in proportions x:y in colspace (for example, in LCH)
  • calculate D65 relative luminance of c1 and c2, compare to find the lighter, then compute (lighter +.05) / (darker +.05) (WCAG 2.1 contrast)

In all cases c1 and c2 need not be in the same colorspace, and need not be in the computation colorspace.

The calculation should not need to special case if c1 or c2 happen to have been specified be in a legacy sRGB format (HSL, hex, etc)

@tabatkins
Copy link
Member

Thanks!

If you're not mutating the original colors, I think all of these cases would involve calling a conversion function to get a fresh object, right? So that feels similar to what we already have, especially if we switch to a generic .to() function.

I suppose if you were just reading the color in another colorspace, and doing so in a sufficiently simple way, then a more direct "give me this channel in this colorspace" API (like Color.js's color.xyz.x access) could be reasonable, but I think all the cases you've given want to read all the channels anyway, and you don't want to redo the conversion multiple times, right? It could memoize under the hood, I suppose, to avoid repeated conversions of the same color.

I don't think that in any of these cases you're switching back to the original color space, right? Well, in all of your examples there are two colors, and as you say they can be in different color spaces anyway, so I'm not even sure what "back" would mean there.

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

No branches or pull requests

3 participants