Remove ambiguous Color math implementations

[Sliman4]

Objective

The implementations are very ambiguous, and it's easy to make bugs using them, especially Hsla variant. They treat Color as Vec4, which isn't quite right. Color values should be clamped to [0.0,1.0], but there are no checks. It adds/multiplies the first 3 values, regardless of the color's variant (There's no reason why you should multiply 3 values of Hsl at once). Also, I (and maybe some other people) initially thought Color::GREEN * 0.5 would result in semi-transparent green, but it makes it darker instead, so it makes it hard to read. Mul<f32> example (but the same errors apply to every other implementation):

// 2 different representation of one color. If you pass them to a function,
// the function may not handle it correctly, just like this:
let green = Color::GREEN;
let other_green = Color::Hsla {
    hue: 120.0,
    saturation: 100.0,
    lightness: 50.0,
    alpha: 1.0,
};
assert_ne!(green * 0.5, other_green * 0.5);

// as in iter_combinations.rs example
let emissive_green = green * 2.0;
// now emissive_green.g() is 2.0, which is not valid
// hsla fails to convert this value and the left value will be Color::WHITE
assert_ne!(emissive_green.as_hsla().as_rgba(), emissive_green);

Solution

Remove it! It's only used by 1 example (btw it's multiplying ORANGE_RED by 2 making .r() == 2.0, which is a bug, but the renderer somehow handles it).

[james7132]

Color values should be clamped to [0.0,1.0]

Quick note, haven't read the code here just yet. How would this translate for HDR colors? Or is this strictly talking about HSLA?

This was briefly discussed regarding setting up linear interpolation for color for animation, where these operations across color spaces is ill-defined and lossy. See #1870.

[alice-i-cecile]

I would very much like a proper color interpolation solution. These impls however are inconsistent and largely incorrect. I agree that these should be removed to avoid confusing users.

[Sliman4]

Color values should be clamped to [0.0,1.0]

Quick note, haven't read the code here just yet. How would this translate for HDR colors? Or is this strictly talking about HSLA?

I'm not very familiar with how HDR is implemented in bevy, but I made no logic changes to Color, just removed some ambiguous implementations that may not work well in user's code. There are no values greater than 1.0 in tests and constants, and .as_hlsa() returns WHITE for such values, so I think these values are invalid, which may be a wrong assumption. I personally think that interpolation should implement such things privately without exposing APIs like this to users, who may (and will) break it.

[superdump]

Values > 1.0 are allowed for linear colours and there is tonemapping code in the shaders that handles mapping HDR values where the colour value is > 1.0 to within 0.0 to 1.0. The bloom PR also specifically benefits from values > 1.0 and it will be common to set emissive values > 1.0 I think, in order for the light to bloom to the surroundings. That is to say that I've used the Mul implementation for Color at least.

[alice-i-cecile]

Thanks for the context. Closing this out, in favor of a new issue.