Added HSV

[brewmook]

Hopefully this addresses #20. It's just a copy of the HSL code with relevant modifications. I'll try and get to the interpolation soon.

[mbostock]

Thanks for the PR.

I should have mentioned this on the other issue, but I’m not totally convinced it’s worth supporting HSV. It’s a fairly trivial mapping from HSL, and it has the same problem of not being perceptually-uniform that RGB and HSL have (unlike Lab* and HCL).

If we are to support it, I still would not support string parsing HSV colors as we do for RGB and HSL. String parsing is intended to match CSS behavior, and CSS does not support HSV.

A nice benefit of not supporting string parsing is that you can define the HSV color space in its own file (like cubehelix.js), rather than needing to fit it in color.js.

In fact, you could even define HSV in a separate module, like d3-scale-chromatic.

[brewmook]

I've pulled out the code to hsv.js and removed the parsing as suggested.

I don't mind if you reject the PR, it's been a useful exercise for me anyway, and I can now see how easy it would be to handle HSV in my own codebase.

[mbostock]

Thanks. Let’s try this: I went ahead and created a d3-hsv repository and invited you as a collaborator. It’ll be a good test to see what (if anything) we need to export from d3-color to allow anyone to implement new color spaces. I copied over the code in this pull request, but it doesn’t quite work because it relies on a few internals to d3-color:

• define and extend aren’t really needed; you can just inline the subclass definition.
• Color and Rgb can be replaced with import {color, rgb} from "d3-color";
• darker and brighter are just numeric constants and you can copy those, too.

I’m glossing over a few things like you’ll need to say rgb(…) instead of new Rgb(…).

[mbostock]

Okay well I couldn’t resist getting it working in d3/d3-hsv@f5e4ab28051e7334b45178b5442e39183f6bc5b5. Sorry! But yay? 😁

[brewmook]

Sweet, I can go to bed then! Cheers. :)

[mbostock]

Thank you! 👍

[mbostock]

I took a crack at implement d3.interpolateHsv and d3.interpolateHsvLong, but then I realized the behavior will always be identical to d3.interpolateHsl and d3.interpolateHslLong, since HSV is just a simple transformation of HSL. So we could provide aliases for these, but I don’t think it’s necessary to dedicate separate implementations.

[mbostock]

Okay, not identical, but almost identical; see the interpolate branch. I’m not sure it’s worth it.

I also caught a bug where we were reporting a NaN saturation for white, which is correct for HSL, but it should be zero for HSV.

[brewmook]

Sorry, I'll probably not get a chance to check the interpolate branch out until the weekend. Cheers for doing the heavy lifting though. :)