Color maths 3

[lynn-lumen]

Objective

• Adds arithmatic operations for colors so they can be used with splines, suggestion of Add color animation support #12202 and incorporating feedback from Color maths 1 #12460 and Color maths 2 #12534

Solution

• This PR adds a LinearConvexSpace trait as suggested by @alice-i-cecile in this comment. This trait has everything needed for splines including arithatic operations such as addition or subtraction.
• LinearConvexSpace does not implement std::ops::Add and friends because those operations are not well defined for some types (such as colors) allthough those types may be used to define a linear convex space.
• LinearConvexSpace<Scalar> is generic over some Scalar type so that LinearConvexSpace may also be used for f64 or u32 based types such as DVec2. These types cannot be used with splines but since LinearConvexSpace does not suggest only being used with splines, this might be useful in the future, allthough I am happy to remove this if wanted.
• I have added a impl_linear_convex_space!(Type, Scalar) macro that will automatically implement LinearConvexSpace<Scalar> for Type using the std::ops::* traits.
• Color types implement LinearConvexSpace<f32> using a macro impl_color_linear_convex_space local to bevy_color. All operations on color types are componentwise with no special rules for the alpha channel.

---

Migration Guide

• The Point trait from cubic_splines.rs has been removed; as such any type that is supposed to be used with splines should implement LinearConvexSpace<f32> now. This can be done automatically for any custom type previously used with splines using the impl_linear_convex_space macro.

Additional information

Mathematical operations on colors are a controversial topic. This is not the first PR attempting to add them and there has been lots of feedback and discussions on some previous PRs. If you want more information for the reasoning behind this proposal, please check #12534 out. For more information on why we don't implement std::ops::Add etc. for Colors or why arithmetic operations on colors may still be useful, please check out #12460.

[SolarLiner]

Between the current implementation and @IQuick143's comment on the previous PR here (which I still to ruminate over, but I think wrt. his argument over the color space issue that he is ultimately correct; though I don't like the typetag approach he proposes personally), I'm beggining to be seriously convinced that the better thing to do is to have an impl for Srgba and forget about all this added complexity, and, ironically, going back to the first implementation of this PR (again, thank you @solis-lumine-vorago for your patience and work, it might feel like a waste of time but all those attempts are still very valuable!).

cc @alice-i-cecile @NthTensor @bushrat011899 @viridia

[NthTensor]

I think this is an improvement over approach 2. Thank you again for your tireless iteration on this issue, and your willingness to engage with feedback. I really do appreciate it.

LinearConvexSpace does not implement std::ops::Add and friends because those operations are not well defined for some types (such as colors) allthough those types may be used to define a linear convex space.

The core math traits provide a notational convenience, pure operator overloading, with no predefined semantics. We are talking about notation here, not well definedness.

Either we are adding math or we aren't. If we are, we should use the math operators. Hiding math operators behind a less obvious api doesn't make anything any more or less correct.

Please review my comments to the previous PR. Not letting us use standard math notation in bevy_math remains a blocker for me.

[IQuick143]

Math nitpick:
This is not exactly what linearity means, as it goes backwards, a line is usually defined using operations on a linear space.

The properties that actually make for a Linear space (also known as a Vector space) are:

(a and b are arbitrary scalars, x and y arbitrary vectors.)
A) Addition is commutative
B) Addition is associative
C) 0 * x produces an identity element (0).
D) 1 * x = x
E) Scalar multiplication associates with number multiplication (ab)*x = a*(b*x)
F) Scalar multiplication distributes over addition (a+b)*(x+y) = a*x + a*y + b*x + b*y

(I hope I didn't miss anything.)

These properties make up a vector space, which is what API consumers expect. (I believe these properties could form their own trait and convexity would be a separate marker trait. But that's maybe a next-PR task.)

As far as convexity goes, you're right, although we could note that the line between points A and B is defined as all the points A*t + B*(1-t) because it might not be imideatly obvious what is a line segment in an abstract space.

[alice-i-cecile]

Yep, I think I'd prefer to include this style of definition.

[lynn-lumen]

Could someone write this as a suggestion please? I am not a native speaker and I don't think I could write good docs for what is described above although I agree with the spirit. :/

[IQuick143]

I see the Default constraint, is it needed/should we constrain it to have a pre-defined value? (like the 0 vector?)

[lynn-lumen]

It is needed within the splines. Constraining it to be the 0 vector might be a good idea though. I'll get to that tomorrow.

[lynn-lumen]

The core math traits provide a notational convenience, pure operator overloading, with no predefined semantics. We are talking about notation here, not well definedness.

Either we are adding math or we aren't. If we are, we should use the math operators. Hiding math operators behind a less obvious api doesn't make anything any more or less correct.

I believe the reasoning for this was laid out by @viridia in the first PR #12460. It boils down to the fact that rust operators should be well defined and have an obvious and agreed upon definition.

Componentwise operations make sense for colors but are not an immediately obvious nor an agreed upon standard for all color spaces. As such they do not meet the "style guide" requirements a type should fulllfill for std::ops::* to be implemented.

As someone has already said: Intuitively, adding red and yellow should produce a valid orange. Yet no addition proposed so far and none that I can think of produce that result. By not allowing let orange = red + yellow; we can make users more aware of the unintuitive yet useful nature of mathematical operations on colors.

[IQuick143]

Not allowing red + yellow != orange, is why I proposed the color wrapper struct that defines its component-wise addition. (Either as a newtype over colors or as a generic as I detailed earlier.)

[NthTensor]

By not allowing let orange = red + yellow; we can make users more aware of the unintuitive yet useful nature of mathematical operations on colors.

An uncharitable reading of this argument might call it opting for an intentionally obtuse api in the hopes that beginners won't try to use it. Little prevents them from assuming red.add(yellow) == orange, it's just verbose and a tad harder to find.

After three PRs, I don't think any of us would disagree that color math is hard to get right. New users are going to make mistakes and misunderstand things, and I think that's unavoidable. We are going to have to write really great docs to help them learn. I grant using add instead of + means we can associate documentation slightly closer to the probable point of failure but at the same time, it really hamstrings complex color math. I am sure it will frustrate experts who do know what they are doing, and want to do more than a few operations in a single line.

I'm sorry, on the one hand I feel like I am derailing your PRs, on the other hand I think this is a serious issue. This represent a noticeable decrease in quality for the spline code, which is a complex feature prone to subtle mathematical errors. I am actively writing additional tests for splines and a PR that adds closed splines, and this will make them difficult to read and correct.

[lynn-lumen]

After three PRs, I don't think any of us would disagree that color math is hard to get right. New users are going to make mistakes and misunderstand things, and I think that's unavoidable. We are going to have to write really great docs to help them learn. I grant using add instead of + means we can associate documentation slightly closer to the probable point of failure but at the same time, it really hamstrings complex color math. I am sure it will frustrate experts who do know what they are doing, and want to do more than a few operations in a single line.

I think what you are referring to is the first PR #12460 which would be pretty much what you are arguing for here. It is most certainly the most obvious in terms of using the API. But as discussed in #12460 a big portion of people don't think that std::ops::* operators should be implemented for colors. If we agree with them, this PR is a logical next step. If we don't, the first PR might be worth a second thought though.

I personally prefer this PR, as we could still implement a wrapper type to make cubic splines less complex and because

• this moves the docs closer to the source of confusion,
• this conforms with Rust's guides for implementing std::ops::* and
• this will prevent a good portion of user errors although the API is (at the moment) more difficult.

In the end this is up for debate though and I will do what most agree upon. These PRs are suggestions for what such an API could look like and intend to enable a more practical view on the real life consequences of these implementations. Please feel free to critique or support any of them.

[viridia]

Going to put forth an odd suggestion: for non-linear color spaces like HSL, it might make sense to think of spline animations in terms of separate animation tracks for each color component rather than a combined animation track for a color object. Interpolating hue or saturation, by itself, is fairly intuitive. Animating alpha, by itself, makes logical sense in any color model. Unfortunately, this creates its own set of problems when it comes time to actually apply the result to a material, since at some point you have to combine the channels together and convert to something that can actually be drawn. Setting the color parameter to HSL requires that you have signals for all three components, H, S, and L, although I suppose some of these can be constants.

[IQuick143]

That is a valid way of doing it, but it's effectively just splitting a single spline into 4 independent ones and then having to recombine them later. (The only benefit I see is the freedom in control point placement, but I'm not super sure how helpful is that, I'd imagine you want to line up your control points in channels.)

[lynn-lumen]

Not allowing red + yellow != orange, is why I proposed the color wrapper struct that defines its component-wise addition. (Either as a newtype over colors or as a generic as I detailed earlier.)

I like this idea in principle but the cons are a bit too harsh here, I think. Having to call .into() or from every single time you interact with splines is very cumbersome. E.g. the fairly small cubic_curve example would require 6 .into()s if you wanted to animate color similarly to position. I don't think this strikes the balance between correctness and ease of use we shoulod be going for.

[lynn-lumen]

This represent a noticeable decrease in quality for the spline code, which is a complex feature prone to subtle mathematical errors. I am actively writing additional tests for splines and a PR that adds closed splines, and this will make them difficult to read and correct.

If your concern is about code within cubic_splines only, you might like the wrapper type idea to fix the difficulties with the current operations. This would involve creating a wrapper type Point<LinearConvexSpace>> that is not externally visible but implements Add, Sub, etc. and is used for all internal operations in cubic splines. We could then just .into() the result of working with that type when we return it out of bevy_math.

This would still be more complex than having Add and Sub for colors but should be fairly legible though.

[SolarLiner]

but it's effectively just splitting a single spline into 4 independent ones and then having to recombine them later

FWIW it's how Blender and Godot do it, when you create an animation it creates 3/4 lanes for each component, and each can be controlled independently.

[NthTensor]

Maybe math in color spaces isn't the way to go at all. If I wanted to produce a color spline with the current implementation, I would define the spline on Vec4, then map the samples into LinearRgba. If I wanted, I could also define four separate splines for each color channel, or a color spline on Vec3 and an alpha spline.

This seems good enough. I'm fine with the idea that once you turn numbers into colors, you can't do math on them any more, so if you want to do color math you have to keep around the coordinates in Vec4 form.

This approach is not going to be obvious to users, but I think this a small explanation in the book it would be fine. It seems basically good enough for most cases.

[SolarLiner]

Color math is still useful in areas other than splines, and some color spaces are designed specifically with the idea of being able to do math in them (ie. Oklab being a perceptually-linear space designed for taking relative distances, comparing colors, and performing additive synthesis). Which is why my original stance on this issue was "math on some color types".

Yes, you could for all use-cases extract the components as a vector and do your math on that, but that defeats the purpose of providing type safety in the first place (one could also push the argument ad absurdum and say that vector types are strictly unnecessary, they're just a bunch of numbers that we could pass as a tuple on all operations, anyway -- it's a type of primitive obsession, I think).

[NthTensor]

Color math is still useful in areas other than splines.

It does seem like most of those are covered by Mix, lighten and so on. Not all, I grant you, but most.

Which is why my original stance on this issue was "math on some color types".

I am generally sympathetic to this, but our spaces are not pure Oklab, they are Oklab+Alpha and alpha is super weird. Any math we define on the Oklab type is going to need to encode some opinionated compositing logic. I'd like to see a design for alpha blending before we try to support addition on our linear spaces, but maybe that's just me?

After some consideration, I don't think we need math on Color.