Use `LinearRgba` for user-facing APIs, rather than `Color`
Objective
As discussed in https://github.com/bevyengine/bevy/issues/12056, widespread use of the Color type (which is an enum holding variants that map to all of the supported color spaces) is not ideal. Right now, it is used in virtually all user-facing configuration (e.g. gizmo builders, clear color, fog settings, BackgroundColor components). This decision was primarily made to ease the initial migration: it maps closely to the previous bevy_render::Color enum (but stores types instead of fields). By contrast, internal, rendering-focused abstractions store LinearRgba: this is the color format that users expect.
We should make a decision on this pattern before shipping the new color type to users. There will not be a perfect decision: there are very real trade-offs with each choice, but the choice should be deliberate, rather than incidental.
Fixes https://github.com/bevyengine/bevy/issues/12212.
Solution
This PR implements the simple and efficient solution 2, laid out in #12212, which simply uses LinearRgba as our standard user-facing color type.
Advantages: All color spaces can be directly converted into it. Save on conversion costs. Great for PBR operations.
Disadvantages: LinearRGB is the worst color space for most perceptual operations. Data about the original color space is thrown away.
Change Log
All uses of Color in the engine have been removed.
Instead, LinearRgba is consistently used to store information about colors across UI and rendering.
Migration Guide
All fields which previously held a Color now store a LinearRgba. The full list of types and fields is:
- TODO, populate list
When defining a color palette for your application, consider defining these in a human-friendly color space: such as Hsla, Labch or Srgba. When spawning objects in your game, convert the stored colors into LinearRgba using the From/Into traits.
Opened https://github.com/alice-i-cecile/bevy/pull/167, which migrates bevy_gizmos to use LinearRgba :)
I think this is more D-Straighforward than D-Trivial, because of the amount of files that need to be changed. (I'm fine with either, though. After all, you did write the description!)
Just as a small warning, this PR may make usage of the Color type significantly decrease. Instead of storing a Color constant for various things, I could see developers just hardcoding a LinearRgba instead.
One could argue that this is actually a good thing, because it forces developers to choose the color representation that bests fits their project. Either way, it's something to keep in mind.
Just as a small warning, this PR may make usage of the Color type significantly decrease. Instead of storing a Color constant for various things, I could see developers just hardcoding a LinearRgba instead.
Yep, reducing usage of Color is the intent. I'm considering submitting a follow-up PR removing the Color type completely: it doesn't have clear use cases and is a lot of work to maintain.
I'm not thrilled with all of the Into calls however.
I'm not thrilled with all of the
Intocalls however.
A few ideas for reducing this:
- Encourage calling
.into()once, then reusing the calculated result.- I'm thinking like lazy static here? My hope is that
rustcjust automatically does this for you.
- I'm thinking like lazy static here? My hope is that
- Convert all colors within
bevy_color::palettestoLinearRgba.- This is most definitely a follow-up PR thing.
- Add
impl Into<LinearRgba>as parameters.- This is a last resort in my opinion, because it can sacrifice efficiency for developer experience, specifically because of the extra monomorphization and potential of calling
color.into()withinUpdateoutside of the developer's control
- This is a last resort in my opinion, because it can sacrifice efficiency for developer experience, specifically because of the extra monomorphization and potential of calling
Putting this on hold until this thread on Discord resolves itself, deciding whether this is a good idea or not.
Initial stress testing showed no meaningful difference. I don't think there's serious performance gains here in real applications.
My current stance is that this PR should be closed, with ColorMaterial::emissive getting swapped to LinearRgba and everything else staying the same.
I agree with closing this. The PR seems like too much change and too little gain. Since Alice and I agree on this, I'm going to close it.