linebender / color

Color in Rust.
Apache License 2.0
39 stars 5 forks source link

Implement `Hash`, `PartialEq` for `DynamicColor` #75

Closed waywardmonkeys closed 14 hours ago

waywardmonkeys commented 21 hours ago

Also, impl Hash on ColorSpaceTag, Missing in support.

This is useful for using DynamicColor within a cache key or for memoizing, but does not match the behavior of Rust float types.

raphlinus commented 15 hours ago

I'm ok with this, it's simple enough, but it feels incomplete. It seems to me we should impl Hash and Eq on DynamicColor to make that use case work. If we decide that DynamicColor is a bit container in which the bits for the components are to be interpreted as f32, then doing those traits in terms of to_bits makes sense.

I understand resistance, as the Leibniz rule for equality breaks down. You can have a case where color1 == color2 and color1.components[0] != color2.components[0] (NaN) and the reverse (0.0 == -0.0). But Leibniz doesn't hold for floats anyway.

waywardmonkeys commented 15 hours ago

I was more concerned about tiny differences due to conversion that aren’t perceptible, but guess we could ignore that.

raphlinus commented 15 hours ago

Yeah, that's an interesting question, but I think quantization to improve cache effectiveness should probably be outside the scope of this library. My feeling is that if we're caching gradient ramps, it's unlikely it'll make a practical difference.

waywardmonkeys commented 15 hours ago

Right, but I'm ending up putting this into a general purpose library where I don't control all of the consumers ... but I'll do this for now and we can revisit in the future as needed.