HSL constructor calculates saturation values in range [0-255] instead of [0-1]

[hgroenenboom]

Reproduction

When trying to use the juce::Colour::withMultipliedLightness method, the saturation is also affected and different colours come out then expected. I've traced this down to juce_Colour.cpp:

    struct HSL
    {
        HSL (Colour col) noexcept
        {
            auto r = (int) col.getRed();
            auto g = (int) col.getGreen();
            auto b = (int) col.getBlue();

            auto hi = jmax (r, g, b);
            auto lo = jmin (r, g, b);

            if (hi > 0)
            {
                lightness = ((float) (hi + lo) / 2.0f) / 255.0f;

                if (lightness > 0.0f)
                    hue = getHue (col);

                saturation = (float) (hi - lo) / (1.0f - std::abs ((2.0f * lightness) - 1.0f));
            }
        }

Suggested change:

There seems to be a missing division by 255.0f

saturation = ((float)(hi - lo) / 255.0f) / (1.0f - std::abs ((2.0f * lightness) - 1.0f));

Very basic tests I used

    juce::Colour c(201, 201, 54);

    // Multiplication by 1 doesn't affect colour
    jassert(c == c.withMultipliedLightness(1.0f)); 

    // Possible to return to same colour
    const auto c2 = c.withMultipliedSaturationHSL(0.5f).withMultipliedSaturationHSL(2.0f);
    jassert(c == c2);

    // Used googles colourpicker to create a specific test case (50% lightness -> 75% lightness)
    const juce::Colour clighter(228, 228, 154);
    const auto c1 = c.withMultipliedLightness(1.5f);
    jassert(c1 == clighter);

    // Updating a colour with its own lightness creates the same colour
    jassert(c == c.withLightness(c.getLightness()));

[ed95]

Thanks, this has been fixed in https://github.com/juce-framework/JUCE/commit/f9f83fe3faac599cb905c33f0940775109cbfd78.