Refactor XYZ color space into its own module.

d3 / d3-color

Color spaces! RGB, HSL, Cubehelix, CIELAB, and more.

https://d3js.org/d3-color

ISC License

398 stars 91 forks source link

Refactor XYZ color space into its own module. #84

Open DCtheTall opened 3 years ago

DCtheTall commented 3 years ago

For #51

Refactors the implementation of the XYZ color space (i.e. CIE XYZ D50) out of src/lab.js into its own module, src/xyz.js.

mbostock commented 3 years ago

This looks like a worthwhile refactoring and I’m in favor of exporting d3.xyz. 👍

DCtheTall commented 3 years ago

@mbostock the PR is ready for review

mbostock commented 3 years ago

Per @danburzo’s comment https://github.com/d3/d3-color/issues/51#issuecomment-720063360, what do you think about the choice of illuminant (D65 vs. D50)? One possibility is that the XYZ class has a field which stores the illuminant ("d65" or "d50"), and we export constructors for both (d3.xyzd65 and d3.xyzd50).

Also, does this PR affect the performance of converting between Lab and RGB because it now makes explicit the extra chromatic adaptation step rather than combining it into the matrix multiplication?

DCtheTall commented 3 years ago

what do you think about the choice of illuminant (D65 vs. D50)?

See my comment on #51

Also, does this PR affect the performance of converting between Lab and RGB because it now makes explicit the extra chromatic adaptation step rather than combining it into the matrix multiplication?

Is there a benchmark you use for this? How have you tested performance impact in the past?

DCtheTall commented 3 years ago

@mbostock friendly ping

danburzo commented 2 years ago

In a recent addition to the css-color-4 spec, @svgeesus has made the following adjustments as per https://github.com/w3c/csswg-drafts/issues/6722:

xyz refers to XYZ with the D65 white-point;
xyz-d50 refers the D50-relative XYZ;
xyz-d65 is an alias for xyz.

These are all predefined color profiles for the color() syntax and are expected to show up as identifiers in the upcoming Color API.

xyzd50 and xyzd65 sound reasonable as method identifiers on the d3 object. (In Culori, I opted a while back for xyz65 and xyz50 and will maintain it for backwards compatibility, although now I prefer the inclusion of the full illuminant name)