diogotr7 / OpenRGB.NET

C# client for the OpenRGB SDK
MIT License
40 stars 16 forks source link

Color.FromHsv is broken for negative hue values #23

Closed wfowler1 closed 1 year ago

wfowler1 commented 1 year ago

Color.FromHsv does some wacky stuff when a negative value is passed for hue. Ideally, passing a value like -30 should act the same as passing 330. Instead it goes bonkers. This is easily fixed with a small change.

        public static Color FromHsv(double hue, double saturation, double value)
        {
            if (saturation < 0 || saturation > 1)
                throw new ArgumentOutOfRangeException(nameof(saturation));
            if (value < 0 || value > 1)
                throw new ArgumentOutOfRangeException(nameof(value));

            int hi = Convert.ToInt32(Math.Floor(hue / 60)) % 6;
            if (hi < 0)
            {
                hi += 6;
            }
            double f = (hue / 60) - Math.Floor(hue / 60);

            value *= 255;
            var v = Convert.ToByte(value);
            var p = Convert.ToByte(value * (1 - saturation));
            var q = Convert.ToByte(value * (1 - (f * saturation)));
            var t = Convert.ToByte(value * (1 - ((1 - f) * saturation)));
            switch (hi)
            {
                case 0:
                    return new Color(v, t, p);
                case 1:
                    return new Color(q, v, p);
                case 2:
                    return new Color(p, v, t);
                case 3:
                    return new Color(p, q, v);
                case 4:
                    return new Color(t, p, v);
                default:
                    return new Color(v, p, q);
            }
        }

The change is the if (hi < 0) block. After the modulo operation on the previous line, this ensures the value is once again in the range [0, 5]. I'd make a pull request but my fork has all kinds of other nonrelevant changes.

diogotr7 commented 1 year ago

Hey, i'm looking into OpenRGB.NET to integrate segments support, so going through issues. Do you want to PR this change? (and any other change you've since made?)

I think that code was copy pasted from Aurora, so it can probably do with some improvement. I'm unsure if people actually use these Color class utilities, I certainly do not. I'll be making some major changes to the library since i'm switching to net6.0, so I'm considering removing the methods, or at least moving them to some ColorUtils class.