bevyengine / bevy

A refreshingly simple data-driven game engine built in Rust
https://bevyengine.org
Apache License 2.0
35.2k stars 3.47k forks source link

Bevy Colors V2 #10986

Closed viridia closed 6 months ago

viridia commented 9 months ago

There's been some discussion about rethinking the way Bevy represents colors. This ticket is a brief summary of the issues; a formal specification will come later.

Statement of the problem

The current representation of Color is an enum which provides an enumeration of various color spaces (SRGB, HSL, etc.) Unfortunately, this flexibility comes at a cost:

Proposed solution

A new bevy crate will be introduced, named bevy_color. This will have concrete types for several different color spaces:

The conversion between color spaces will be implemented via the From trait. These conversions will never fail; if a color cannot be represented in the target space, then a suitable, reasonably close value will be chosen. The only methods that return an error result will be deserialization / decoding methods.

In addition, for the Linear and SRGB spaces, there will be a .transmute_xxx() method to deal with the rare case where you have the wrong type encoded - i.e. the image decoder only produces SRGB types, but the actual image file is linear.

There will be traits which add additional color manipulation operations. Not all traits will be defined for every color space:

Because the existing Color class is used so widely, it will be retained for the next several Bevy releases to ease migration. It may eventually be replaced with a new class which allows multiple bevy_color colors to be represented in an enum.

To ease the pain of transition, API methods that accept a Color can be updated to accept an impl IntoLinRgba trait which automatically converts the argument to LinRgba.

To address the need for named standard colors (like Color::Red), a new namespace bevy_color::names will contain definitions for all the standard colors.

Alternative solutions

There has been proposals to integrate the palette crate, which is a very full-featured library for color representation and manipulation. However, a cursory look at this crate's source code suggests that it is over-engineered for our purpose. Bevy would be better served with a more lightweight solution that only includes the capabilities we are likely to actually need.

See also #1402

hymm commented 9 months ago

palette pr here https://github.com/bevyengine/bevy/pull/9698

Palette does give pretty much all the features asked for above. I think we should consider it as long as we can show that it's not bloating compile times.

MarkusTheOrt commented 9 months ago

I am very much in favor of reworking colors!

I was thinking about possible implementations and whilst it might not produce all the nicest of code, I think the ergonomics of using PhantomData should not be overlooked.

instead of having SRGBColor, or HSLColor we'd then have Color<SRGB>, Color<HSL>. this would give us type hints at compile time whilst being a zero cost abstraction.

Color conversion can still be done using the From trait as the following is still valid code:

impl From<Color<SRGB>> for Color<HSL> {
    ...
}
mockersf commented 9 months ago

See also https://github.com/bevyengine/bevy/discussions/4648

viridia commented 9 months ago

@MarkusTheOrt - I'm not sure what the advantage is of doing this, it doesn't seem any more or less ergonomic. Honestly, I really don't think we need to do anything clever here - the most straightforward and obvious approach will work well enough.

@hymm - I think this is one of those questions where we will have to take the temperature of the Bevy community and/or leadership. If they decide they want to support palette, that's fine, I can work with that. However, if for whatever reason they decide not to support palette, then I think my proposal provides the best alternative.

viridia commented 9 months ago

There's a couple of changes I would make to the proposal:

1) Drop the non-alpha types from the proposal. 2) Feedback from discord suggests the name "Color" is redundant, so you'd just have "LinearRgba", "SRgba", etc.

One nice thing about this proposal is that the work is very distributable - anyone can take a trait or color space and work on just that.

Also, as Cart has pointed out, all the algorithms are widely known, standardized, there's not much room for variation, we can cut & paste from other places, once it's done it's not likely to change much.

Open question: would there be much benefit to a SIMD color space, for example DeviceColorSIMD? Seems like this would only be useful for operations that manipulated large numbers of individual colors.

JMS55 commented 9 months ago

Great link from @LPGhatguy https://developer.chrome.com/docs/css-ui/high-definition-css-color-guide#what_is_a_color_space

viridia commented 9 months ago

I have done a bit more research on this issue and have refined my ideas slightly. There are also some open questions.

Transmutes

Incorrect encoding: it sometimes happens, especially dealing with image files, where you have a color that would normally be sRGB but is in fact linear. For example, something like an illuminance map output from a renderer. So there is a need to be able to convert from one color space data type to another without actually transforming the color values.

Looking at the palette crate, I see this can be done by something like this:

// Transmute a linear color to SRGB
let rgba = SRgba.from_components(linear.to_components());

Conversely, you can force the conversion of the components using the same method + into:

// Transmute a linear color to SRGB
let rgba = SRgba.from_components(rgba.into::<LinearRgba>.to_components());

Polymorphism

For most use cases where you need to convert from one color space to another, you can do the conversions eagerly:

let srgba: SRgba = linear.into();

APIs that are meant to accept colors in multiple color spaces can take an Into parameter:

fn draw_rect(color: impl Into<LinearRgba>) {
    draw_rect_inner(color.into());
}

This makes draw_rect generic on the color space. It will convert the color to LinearRgba first and then call the inner function which is non-generic.

For things like bundle attributes (BackgroundColor) we can either convert the color to linear eagerly, or define some dyn trait like ToDeviceColor or possibly ToLinearRgba:

struct BackgroundColor {
    pub color: Box<dyn ToDeviceColor>,
};

This trait would have a single function, .to_device_color(), or possibly .to_linear_rgba() which converts the color from its current color space to linear RGBA. However, this approach has several downside, one of which is the need to box the color, the other is that there's no way to extract the color in its original color space if you need it for some reason.

For animating colors (examples such as CSS animations like hover states), ideally we would want the animation to be defined in a perceptual space during tweening - according to Raph Levien's post, Oklab works well for this purpose. This will require that we keep the control points of the animation (start and end) in Oklab, and convert the interpolated value to linear each frame. Bevy currently doesn't support such animations, but a UI framework is certainly going to want this.

This is assuming, of course, that the user will be satisfied with our choice of color space (there is precedent for this - in browsers, you don't get to choose what color space colors are lerped in). If the user really wants to have the choice to lerp in either RGB or Oklab space, then we need to store the start and endpoints in their original form, polymorphically. The starting and ending color have to be in the same color space, and it's hard to enforce this via static typing.

I'm hesitant to introduce a Color style enum because of its non-extensibility. If the user wants to work in XYZ color space, and Bevy does not provide it, this is not a problem for most things - the user can define their own XYZ struct type and implement the various traits we provide. But what they cannot do is add a new enum variant to Color without modifying Bevy itself.

The only other way to do runtime polymorphism is dyn traits, which does allow the user to extend the set of implementations - however, this requires that we know in advance all the possible methods that are likely to get called on the color value. Plus, you know, boxing.

Named colors

Currently there are about two dozen named colors (ALICE_BLUE and so on), which are all defined in sRGB. A question is whether users are going to want named colors in other color spaces - do we want named::linear::ALICE_BLUE or is it sufficient to have the user do the conversion themselves? This gets more interesting when we start thinking about the NONE / transparent color: if we're using pre-converted colors in rendering and we want to detect NONE, do we want a named::linear::NONE which is distinct from named::srgba::NONE?

JMS55 commented 9 months ago

Imo we should remove the named colors anyways.

viridia commented 9 months ago

OK, I figured out some more.

The original impetus behind all this was color interpolation, which works a lot better if both the start and end colors are known to be in the same color space. For something like a CSS color transition, you can define a struct which does this:

/// Represents a range of colors that can be linearly interpolated, defined by a start and
/// end point which must be in the same color space.
pub struct ColorRange<T: Mix> {
    start: T,
    end: T,
}

impl<T> ColorRange<T>
where
    T: Mix,
{
    /// Get the color value at the given interpolation factor, which should be between 0.0
    /// and 1.0.
    pub fn at(&self, factor: f32) -> T {
        self.start.mix(&self.end, factor)
    }
}

ColorRange only requires that the color type implements the mix() method, which does the actual lerp.

This works great if the types of your colors are known statically. What if you really do need dynamic typing? In that case, we can define a dynamic trait which wraps a ColorRange:

trait AnyColorRange {
    fn at_linear(&self, factor: f32) -> LinearRgba;
}

The trait performs the interpolation and also converts the result to linear RGBA.

I'm pretty happy with this approach; it's pretty efficient, and while the dyn trait does require a box, it's generally only one box per animating widget, which is not a high price to pay.

viridia commented 9 months ago

Prototype library here: https://github.com/viridia/quill/blob/main/crates/bevy_color/src/lib.rs

jnhyatt commented 9 months ago

Looks like palette was ruled out for being too complex, has any thought been given to kolor? It seems to support the color spaces mentioned here, it's based on glam, and general seems fairly lightweight while still having most of what we want.

And tbh this feels close enough to what we want that adding another Bevy crate seems like overkill to me. If nothing else, we could use kolor as a base and add functionality to it somewhere in Bevy as needed.

viridia commented 9 months ago

@jnhyatt Thanks for the link, I hadn't known about that.

There are really several controversial issues to be addressed here. One is whether to use a third-party crate for conversion between color spaces.

However, an issue that is equally contentious is that we're also proposing a significant change to the way Bevy processes colors during rendering. Currently, Bevy maintains colors in a multi-color-space representation (enum Color) and presumably only converts to linear RGB when communicating with the GPU (I'm not clear on the details). So for example when you assign a BackgroundColor to a UI Node, the color is an enum, and that enum gets converted to linear every frame during the render phase.

In the new model, colors would be converted to linear RGB higher up in the stack - that is, your UI framework, scene graph or CSS stylesheet would still support multiple color spaces, but these would get converted to device colors / linear eagerly. So now BackgroundColor might be a LinearRgba only, and you have to call .into() on the color when inserting or updating that component into the entity.

This is in fact how it works in frameworks like three.js: the user is responsible for converting colors into the right color space before calling the framework APIs. And by creating static types for each color space, we can remind the user that the conversion is required. Effectively we're following the principle of EIBTI - Explicit is Better than Implicit.

Note that this second issue doesn't go away by picking a third-party crate to do color conversions.

From an implementation standpoint, this second issue has a larger footprint than the issue of how we convert from one color space to another. I mean, Github Copilot can generate perfectly good code for converting from Linear RGBA to Oklab, that's not a problem (it can even write unit tests for it). Integrating the result into the Bevy rendering pipeline is the more "interesting" issue.

jnhyatt commented 9 months ago

@viridia Thanks for the explanation. I agree kolor is absolutely missing some key features, if you can't express type-safety when reasoning about color spaces, it's completely off the table. With that in mind, I'm happy to give a more thorough review and try to give more relevant feedback.

minecrawler commented 9 months ago

@viridia wouldn't early conversion to e.g. sRGB be problematic for stuff like tonemapping and HDR? I remember a discussion (involving ACES) about having colors in a bigger color space as long as possible to do blending and so on as accurately as possible and only convert it at the very end to a device color space...

So wouldn't it make sense to design bevy colors around the idea of making it easy to implement proper color management, just like Rust makes it easy to write proper code and harder to do something exclusively different (usually stupid)?

Then, in the higher stack, colors would be maintained in a rich format, which gets converted e.g. in a shader just before finishing the frame - depending on the screen e.g. to sRGB. Ideally, a developer would decide beforehand which format they want to maintain. A build pipeline would convert and optimize assets (e.g. scenes/prefabs) and typing would statically assert that only that one space is used. The result: a developer can get assets online (or get the artists on board), throw them into a configured project, run the asset pipeline and start developing without worrying about accidentally using the wrong space or loading anything with hidden problems (as along as the asset pipeline is transparent).

viridia commented 9 months ago

@minecrawler Thanks for that comment. I'll admit that this is an area which I don't know that much about, and I'm glad to see someone has an awareness of the issues.

My first question is, can that "larger" color space you refer to be a single, pre-chosen color space, or is there a need for multiple/polymorphic color spaces to be supported all the way down the pipeline?

I think there's no disagreement around the idea that near the top of the stack, we need flexibility to operate in multiple color spaces - not just for asset pipelines, but for interactive color pickers, stylesheets (or more generally, any human-writeable file format which supports a choice of color spaces). My question is, how far down does that flexibility need to go?

The original impetus for this proposal was the difficulty of implementing lerp() on the existing Color class: because interpolation yields different results in different color spaces, the choice of color space is important. Interpolating between two colors in the same space is well-defined, but happens when you try and lerp between an RGBA color and an HSLA color? Obviously, you need to convert one of them, but which one (the start or the end point)? And what happens if the start or end point is outside the gamut of the other end point? Having a polymorphic color representation yields a host of ambiguities which go away once you require the user to explicitly pick a color space.

The way I see it, polymorphic colors should be confined to those specific apps that need that capability, rather than making the whole system polymorphic which makes everyone pay the cost of it.

However, I could be wrong :)

bushrat011899 commented 9 months ago

Throwing in my 2¢ worth, if the work is going to be done to duplicate the work of crates like palette "but Bevy", the benefits (to Bevy and its users) needs to be apparent. Palette is over engineered, but that complexity brings a flexibility for users with more extreme colour requirements than the norm.

With that said, I would be happy to help on a new crate if those clear benefits can be outlined from the beginning.

I personally don't like the idea of implementing From in a lossy way. I think that has the potential to create more confusion than it's worth. Perhaps a better alternative would be TryFrom and an extension trait which could take that Result and unwrap it with the nearest suitable substitute.

let oklab_red = rgb_red.try_into().unwrap_or_nearest();

That would also allow for isomorphic spaces to instead implement From.

Another potential thorn I foresee is the n-squared nature of needing every colour space to implement From every other space (a problem which palette works around with some macro magic and a custom trait IIRC). Since this crate (probably?) won't allow custom colour spaces implemented by users, we could probably stick with the standard Try/From traits.

minecrawler commented 9 months ago

My first question is, can that "larger" color space you refer to be a single, pre-chosen color space, or is there a need for multiple/polymorphic color spaces to be supported all the way down the pipeline?

Implementation details, really. I vote for YES, let's make this a configuration and everything need to build to a static asset of that format and be of that format in code (so things can be fixed at compile time).

I think there's no disagreement around the idea that near the top of the stack, we need flexibility to operate in multiple color spaces - not just for asset pipelines, but for interactive color pickers, stylesheets (or more generally, any human-writeable file format which supports a choice of color spaces). My question is, how far down does that flexibility need to go?

I goes as far as tool boundaries go. Let's say, as an artist, you create textures in Gimp or Krita. Then what you should do is export your art in the agreed format, inspect it for correctness or change whatever knobs to make it look the intended way. When you are happy, it's Bevy's job to take the asset and displat it in the correct way, just as the editor did before (at least on the same surface).

The original impetus for this proposal was the difficulty of implementing lerp() on the existing Color class: because interpolation yields different results in different color spaces, the choice of color space is important. Interpolating between two colors in the same space is well-defined, but happens when you try and lerp between an RGBA color and an HSLA color? Obviously, you need to convert one of them, but which one (the start or the end point)? And what happens if the start or end point is outside the gamut of the other end point? Having a polymorphic color representation yields a host of ambiguities which go away once you require the user to explicitly pick a color space.

If you enforce one color space, this problem just goes away.

The way I see it, polymorphic colors should be confined to those specific apps that need that capability, rather than making the whole system polymorphic which makes everyone pay the cost of it.

Converting between color spaces should be possible at all times, but it should be explicit. A developer should know that conversion is "expensive" and lossy.

viridia commented 9 months ago

@minecrawler I'm going to have to disagree with some of your points.

If you enforce one color space, this problem just goes away.

I'm not sure what you mean by "enforce". Take for example, doing linear gradients: it's generally accepted that gradients calculated in polar coordinate spaces (HSL, LCH, etc.) look brighter and more colorful than gradients calculated in cartesian spaces (RGB) which tend to look muddy and dull. For a programmer who is implementing a gradient, we really need to give them the choice of what color space they want - we can't/shouldn't impose a single color space choice for them.

So if by "enforce" you mean that the user has to pick a color space explicitly to do their color mixing, then fine. If you mean that the framework limits the choices to only one color space, then no.

At least, not at that level. I know this is a bit confusing, because we're talking about different levels. At the level of GPU primitives I think there should only be one color space, which is whatever the hardware supports. At the middle level, where programmers do fun things with color algorithms, there should be multiple color spaces, but they should be explicit and strongly typed in code - we don't want to constantly have to match different spaces to do calculations. At the highest level, that of assets and files, there should also be multiple color spaces, but dynamic, where the input file gets to decide how a given color value is to be interpreted.

@bushrat011899 Even if we do adopt palette, we still have to address the other half of the issue. Palette gives us a bunch of individual color space types, but it doesn't give us something that corresponds to Bevy's Color type. We can't get rid of Color entirely, but part of what I am proposing is to greatly reduce the usage of Color in favor of statically typed color space types.

TehPers commented 8 months ago

For converting named colors, would it be sufficient to have the named colors be types with some predetermined values in each of the color spaces? For example, struct Red; would be a ZST with some hardcoded values for sRGB, HSV, etc in the From impls.

viridia commented 7 months ago

Prototype here: https://github.com/viridia/bevy_color