d3-color icon indicating copy to clipboard operation
d3-color copied to clipboard

Refactor XYZ color space into its own module.

Open DCtheTall opened this issue 4 years ago • 6 comments

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.

DCtheTall avatar Oct 31 '20 21:10 DCtheTall

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

mbostock avatar Oct 31 '20 21:10 mbostock

@mbostock the PR is ready for review

DCtheTall avatar Nov 02 '20 15:11 DCtheTall

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?

mbostock avatar Nov 02 '20 15:11 mbostock

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 avatar Nov 02 '20 16:11 DCtheTall

@mbostock friendly ping

DCtheTall avatar Nov 16 '20 18:11 DCtheTall

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)

danburzo avatar Oct 12 '21 11:10 danburzo