search

graypegg / chromatism

:rainbow: A simple set of utility functions for colours.
1.78k stars 37 forks source link

Conversion of HSV fails when V = 0 #35

Open bdoss opened 6 years ago

bdoss commented 6 years ago

This appears to be caused by division by zero, resulting in the saturation component evaluating to NaN.

Line 12 seems to be where the division by zero occurs: https://github.com/toish/chromatism/blob/488020a1553d5aa0a0ffb1fbd98fc19a6be6503d/src/conversions/hsv.js#L9-L17

bdoss commented 6 years ago

Only seems to impact the HSV case.

HSL conversion:

let { hex } = chroma.convert({ h: 0, s: 0, l: 0 });
// hex = #000000

HSV conversion:

let { hex } = chroma.convert({ h: 0, s: 0, v: 0 });
// hex = #NaNNaNNaN
TehShrike commented 6 years ago

Good catch!

TehShrike commented 6 years ago

Fix published upstream in chromatism2@3.0.2. Can be cherry-picked/merged from https://github.com/TehShrike/chromatism/commit/436d8b09bc435845c3d31aff004cf77b83d91cc4

bdoss commented 6 years ago

Thanks for the quick fix!

graypegg commented 6 years ago

Awesome! Thanks for the patch @TehShrike, and sorry for the radio silence on my end, this will be in a patch release as soon as I can get to my laptop today.

graypegg commented 6 years ago

Oh wait, @TehShrike, when you’re able to, would you be able to make a pull request? Thanks!