stewartlord/identicon.js

HSB/HSV vs HSL, Brightness vs Lightness

Closed this issue · 2 comments

McPo commented

This is a very minor issue, which depending on the change could be breaking. However I figured I'd at least document it in the issues list.

The parameters for the library are saturation and brightness with the hue being adjusted. The issue is that HSL (Hue, Saturation, Lightness) is being used not HSB/HSV (Hue, Saturation, Brightness/Value) (HSV == HSB, HSV != HSL, HSB != HSL).

Therefore I suggest renaming the brightness parameter to lightness.

The reason I ran into this issue was I was porting the colour extraction algorithm to python. I wasn't sure if HSL was being used or HSB. As the function call was hsl(h,s,b) with a note stating that it had been adapted from else where (Maybe it was adapted to HSB but they forgot to rename etc).

To make matters worst python doesn't have a built-in HSB function, it does have one for HSV (Which is the same thing). And to add to that the python function is for HLS as opposed HSL.

So far the above comments revolve around it being confusing to read the code / port. However It also has a real impact on users of the library. I may have selected a saturation and brightness value from photoshop or http://colorizer.org, that I like. However the library is actually using HSL not HSB/HSV therefore I would be getting different colours than I expected.

I can raise PR if requested, but like I said, I just wanted to document this incase somebody else ran into it.

Interesting! I wonder if this explains why it doesn't produce quite the same color as GitHub. When I originally coded this (cough) 5 years ago, I didn't know why the colors were slightly off. A PR would be welcome. It would be breaking in the sense that the colors would change. We could address that with a major version bump.

Closing due to inactivity