Color PartialEq is broken when comparing different variants

[JMS55]

Bevy version

0.11.2

Bug

Color PartialEq is implemented via derive. This means that different variants won't be equivalent, even if logically they should be.

Color::BLACK is Rgba(0, 0, 0, 1). RgbaLinear(0, 0, 0, 1) should be equivalent, but it is currently not.

We should write a by-hand PartialEq impl that converts all color variants to Rgba (or whatever we expect to incur the least conversion overhead), and then checks equivalence.

[bushrat011899]

This is resolved in #9698, as a note for reviewers.

[ameknite]

@bushrat011899 I found that conversions don't work very well with palette.

If I do this with the current implementation I get:

    let sea_green = Color::SEA_GREEN;
    let new_sea_green = sea_green.as_rgba_linear().as_rgba();

Rgba { red: 0.18, green: 0.55, blue: 0.34, alpha: 1.0 }
Rgba { red: 0.18, green: 0.55, blue: 0.34, alpha: 1.0 }

but with your PR if I do this, I get:

    let sea_green = Color::SEA_GREEN;
    let new_sea_green = Color::from_space(sea_green.as_rgba_linear());

Color { r: 0.18, g: 0.55, b: 0.34, a: 1.0 }
Color { r: 0.17999998, g: 0.54999995, b: 0.33999997, a: 1.0 }

So because of this, when you do a comparison it doesn't work.

[bushrat011899]

@bushrat011899 I found that conversions don't work very well with palette.

That can be true for some color values, but I think it is a separate issue to what's being discussed here. This exact test will fail on current Bevy, and pass on my PR:

let a = Color::RED;
let b = Color::hsl(0., 1.0, 0.5);

assert_eq!(a, b);

What you've identified is (in my opinion) more to do with the design flaw in comparing floating point types. If we do the comparison in a more robust way, we get the expected result (at the cost of some compute):

let original = Color::SEA_GREEN;
let converted = Color::from_space(original.as_rgba_linear());

assert!(original.as_lch().difference(converted.into_color()).abs() < 1e-3);

More specific to my PR, I could replace the PartialEq implementation with one that uses the above difference method, but then == becomes more expensive, and has more assumptions (is 1e-3 a reasonable tolerance, what space to compare in, etc.).

[alice-i-cecile]

This should be fixed now, with the addition of bevy_color.