`Color.GetHue()`, `Color.GetSaturation()`, and `Color.GetBrightness()` are incorrectly implemented as HSV

[CoryCharlton]

Library/API/IoT binding

nanoFramework.Graphics.Core

Visual Studio version

No response

.NET nanoFramework extension version

No response

Target name(s)

No response

Firmware version

No response

Device capabilities

No response

Description

The documentation for Color.GetHue(), Color.GetSaturation(), and Color.GetBrightness() (and the .NET full implementation) say they should return HSL values but they actually return HSV values.

How to reproduce

var color = Color.FromArgb(38, 196, 103);
Console.WriteLine($"color: {color} - Hue: {color.GetHue()} - Saturation: {color.GetSaturation()} - Light: {color.GetBrightness()}");

nanoFramework output:

color: #FF26C467 - Hue: 144.683548 - Saturation: 0.80612242 - Light: 0.76862746

.NET Core 8.0 output:

color: Color [A=255, R=38, G=196, B=103] - Hue: 144.68355 - Saturation: 0.6752137 - Light: 0.45882353

Expected behaviour

Either HSL values should be returned or the documentation should be updated to note they are HSV values (I prefer the latter but it differs from .NET full 🤷 )

Screenshots

No response

Sample project or code

No response

Aditional information

https://www.rapidtables.com/convert/color/rgb-to-hsv.html https://www.rapidtables.com/convert/color/rgb-to-hsl.html

[CoryCharlton]

As usual I'm more that willing to send over a PR just want to confirm which direction we want to go.

[TerryFogg]

The original Color.cs imported from .Net Microframework was much simpler and did not have these functions. This is a much-improved class, and it looks like @Ellerbach has created this new one from code (maybe from Microsoft). Hopefully, he can assist. Terry

[josesimoes]

As usual, we should favor alignment with the full .NET API (despite we all occasionally disagree on some bits). Let's wait for @Ellerbach take on this. And, yes please, @CoryCharlton do take of this. 👍🏻

[Ellerbach]

@CoryCharlton and @josesimoes yes, code is mainly coming from Microframework and .NET and the existing functions in the nano Graphics. Those explains as well disconnection on some of the names for the function and as some were used also in the IoT repo. If there are names to align, go for it! If there are maths to align, go for it as well! Just be careful with renaming quite a lot of functions are used in the nano Graphic libs and in the IoT repo.

[CoryCharlton]

Sounds good. I sent over a PR to fix the math involved in Color.GetHue(), Color.GetSaturation(), and Color.GetBrightness(). I used the implementation from .NET Core 8.0.