System.Drawing.Color has methods for Hue/Sat/Brightness, but actually calcs in HSL

[ctolkien]

As per the HSL and HSV wiki:

HSL stands for hue, saturation, and lightness (or luminosity), and is also often called HLS. HSV stands for hue, saturation, and value, and is also often called HSB (B for brightness).

(Emphasis my own)

System.Drawing.Color has methods for GetHue(), GetSaturation(), and GetBrightness().

One might assume that these methods would then return values using the HSV/HSB format, however they do not. They are using HSL.

This is not really intuitive (especially as this doesn't seem to be documented). Appreciate changing how they work is unlikely to fly due to back compat.

[alexperovich]

Are you wanting us to rename the GetBrightness() function?

[ctolkien]

Are you wanting us to rename the GetBrightness() function?

Changing GetBrightness() to GetLightness() or GetLuminosity() would be more correct than what it is currently.

That would help in making a slightly clearer distinction as to what color representation these methods represent. If you're going to break the API however, I'd suggest going further.

If you look at a call to GetSaturation() (for instance) in isolation - how are you to know what color representation this is using?

Should these be called GetHslSaturation() or GetSaturation(ColorRepresentation = ColourRepresentation.HSL) - that being, the method has a default value to use the HSL algorithm?

[alexperovich]

So this API change

namespace System.Drawing
{
    public struct Color
    {
-       public float GetBrightness()
+       public float GetLightness()
    }
}

Part of the reason Color even exists in .NET core is compat. I don't think there is much chance of this change happening. I don't know if we have any usage of GetBrightness though. @terrajobst can comment on that.

[mellinoe]

The best we can do here is provide better documentation about the potential points of confusion. Changing the name would be a major breaking change, and like @alexperovich said, the only reason this type is in .NET Core is because of compatibility.

[alexperovich]

So we need to document that the GetBrightness() function actually returns the Luminosity using HSL format.

[ctolkien]

The best we can do here is provide better documentation about the potential points of confusion

Yep, figured changing the API would be a non-starter.

So we need to document that the GetBrightness() function actually returns the Luminosity using HSL format.

Also need to document that the GetHue() and GetSaturation() methods are also in HSL. With my limited understanding of color spaces - the value of hue is the same in both representations for a given color (but it should still be documented for clarity). Saturation however has entirely different definitions depending on HSL or HSV/B.

And that's how I came across this issue - we spent an awful long time trying to figure out why our saturation levels weren't like we would expect!

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.