search

supersimple / chameleon

Chameleon Hex Package
Apache License 2.0
26 stars 11 forks source link

Add the HSV colorspace #2

Closed fhunleth closed 6 years ago

fhunleth commented 6 years ago

This adds the HSV colorspace. I had mistakenly thought that HSL would be close enough, but that didn't work out.

A few notes on this PR. I think these are all food for thought or for future PRs:

  1. I added the percent() and angle() types for this module. They probably should be refactored to a common location when they get used by other modules.
  2. I wonder if floating point values should be allowed. I didn't make this change since it would be inconsistent with other colors, but the restriction to use integers felt arbitrary.
  3. I separated out the HSV tests from the others since I wanted to unit test it against more colors and it felt too much for one test file. I think it would be nice to have a _test.exs for each supported color.
  4. I found out that "black" converts using Pantone black which is not RGB(0, 0, 0). I found this surprising. Since I don't use Pantone colors, I wasn't sure whether you'd find this surprising so I just commented out the test.
fhunleth commented 6 years ago

I updated the hsv2rgb function names to be consistent. Let me know if you need anything else.

supersimple commented 6 years ago

Thanks @fhunleth !