bevyengine / bevy

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

Add a Color lerp() method #1402

Closed Josh015 closed 6 months ago

Josh015 commented 3 years ago

You already have lerp() for all the Vec* types, and it would be handy to have the ability to interpolate between two colors in linear space.

tigregalis commented 3 years ago

I agree. I often find myself using things like Color::from(Vec4::from(color).lerp(Vec4::from(other_color), 0.5))

Something to consider: Interpolation in linear vs sRGB colorspaces will yield (very) different results. The above yields, between White #FFFFFF to Black #000000, a 50% grey ~#808080, but interpolating it in the linear colorspace yields a "73.5%" grey ~#bbbbbb.

#[test]
fn test_lerp() {
    const ERROR: f32 = 0.000001;

    let color = Color::WHITE;
    let other_color = Color::BLACK;

    // interpolation in sRGB colorspace

    let target_color = Color::rgba(0.5, 0.5, 0.5, 1.0);
    let new_color = Color::from(Vec4::from(color).lerp(Vec4::from(other_color), 0.5));
    assert!((new_color.r() - target_color.r()).abs() < ERROR);
    assert!((new_color.g() - target_color.g()).abs() < ERROR);
    assert!((new_color.b() - target_color.b()).abs() < ERROR);
    assert!((new_color.a() - target_color.a()).abs() < ERROR);

    // interpolation in linear RGB colorspace

    let target_color = Color::rgba_linear(0.5, 0.5, 0.5, 1.0);
    let new_color_vec = Vec4::new(
        color.r_linear(),
        color.g_linear(),
        color.b_linear(),
        color.a(),
    )
    .lerp(
        Vec4::new(
            other_color.r_linear(),
            other_color.g_linear(),
            other_color.b_linear(),
            other_color.a(),
        ),
        0.5,
    );
    let new_color = Color::rgba_linear(
        new_color_vec.x,
        new_color_vec.y,
        new_color_vec.z,
        new_color_vec.w,
    );
    assert!((new_color.r() - target_color.r()).abs() < ERROR);
    assert!((new_color.g() - target_color.g()).abs() < ERROR);
    assert!((new_color.b() - target_color.b()).abs() < ERROR);
    assert!((new_color.a() - target_color.a()).abs() < ERROR);
}

I suggest perhaps a lerp (for sRGB) and a lerp_linear (for Linear).

Josh015 commented 3 years ago

I think I might just take a stab at this myself if the maintainers are cool with it.

tigregalis commented 3 years ago

I think I might just take a stab at this myself if the maintainers are cool with it.

I think it should be straightforward. You'll find the struct definition in bevy\crates\bevy_render\src\color.rs

Some potentially relevant details at the bottom of this discussion: https://github.com/bevyengine/bevy/issues/1193

Josh015 commented 3 years ago

My concern is that I don't think the default lerp method should be working in sRGB space, since all the other math functions on Color are working in linear space. I think lerp should be linear and we should skip sRGB interpolation entirely since it's not a mathematically correct operation.

tigregalis commented 3 years ago

My concern is that I don't think the default lerp method should be working in sRGB space, since all the other math functions on Color are working in linear space. I think lerp should be linear and we should skip sRGB interpolation entirely since it's not a mathematically correct operation.

Mathematical operations are currently in sRGB space: https://github.com/bevyengine/bevy/pull/688#issuecomment-711414011

Ultimately I think separate types are the right way to go, but that will require slightly smarter gpu data conversions, so I'd rather handle that later.

For now, I do think it makes sense to fix the math ops. I think it makes sense to adjust them to use nonlinear srgb everywhere:

// crates/bevy_render/src/color.rs
impl Mul<f32> for Color {
    type Output = Color;

    fn mul(self, rhs: f32) -> Self::Output {
        Color::rgba(self.r() * rhs.r(), ...);
    }
}

Ah, not true actually. Add is in linear space, so that's a bug (or it's correct, and the other methods are bugs).

Josh015 commented 3 years ago

Eww! You're right! Color mixes color spaces across its operators, so it has inconsistent behavior. Seems like that module is badly in need of an overhaul.

tigregalis commented 3 years ago

On the topic of what operations are correct, I did find this article which agrees with what you're saying: https://observablehq.com/@sebastien/srgb-rgb-gamma

As it turns out, colors on the Web are expressed in the sRGB colour space, which is gamma-corrected (also called gamma-compressed), which is a non linear space. As a result, any computation on Web colours is likely going to be off, as it assumes to operate in a linear space while it is not. We're going to unpack this in more detail here.

And so all operations should it seems be in linear space.

However, what is the intent of a user when they interpolate between color (#ffffff) and other_color (#000000) by 0.5 - is it #808080 (50% non-linear rgb) or #bbbbbb (50% linear rgb)?

tigregalis commented 3 years ago

Now, I'm not sure: image Reference: https://www.colorhexa.com/000000-to-ffffff Both groups of squares represent values from black outside to white inside (with a 0.25 step change). Left is linear RGB. Right is non-linear sRGB. The right-hand looks most correct in terms of interpolation, which I think you can see with the 25% white square: for linear there is a sharp contrast between 0% and 25% white, for sRGB it is much closer.

show code ```rust use bevy::prelude::*; fn main() { App::build() .add_plugins(DefaultPlugins) .insert_resource(ClearColor(Color::BLUE)) .add_startup_system(setup.system()) .run(); } #[derive(Bundle, Default)] struct MinimalBundle { transform: Transform, global_transform: GlobalTransform, } fn setup(mut materials: ResMut>, commands: &mut Commands) { let color0 = Color::BLACK; let color1 = Color::WHITE; commands .spawn(OrthographicCameraBundle::new_2d()) .spawn(MinimalBundle { transform: Transform::from_translation(Vec3::new(-200., 0., 0.)), ..Default::default() }) .with_children(|parent| { parent .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(250.), ..Default::default() }, material: materials.add(lerp_linear(color0, color1, 0.).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(200.), ..Default::default() }, material: materials.add(lerp_linear(color0, color1, 0.25).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(150.), ..Default::default() }, material: materials.add(lerp_linear(color0, color1, 0.5).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(100.), ..Default::default() }, material: materials.add(lerp_linear(color0, color1, 0.75).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(50.), ..Default::default() }, material: materials.add(lerp_linear(color0, color1, 1.).into()), ..Default::default() }); }) .spawn(MinimalBundle { transform: Transform::from_translation(Vec3::new(200., 0., 0.)), ..Default::default() }) .with_children(|parent| { parent .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(250.), ..Default::default() }, material: materials.add(lerp(color0, color1, 0.).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(200.), ..Default::default() }, material: materials.add(lerp(color0, color1, 0.25).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(150.), ..Default::default() }, material: materials.add(lerp(color0, color1, 0.5).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(100.), ..Default::default() }, material: materials.add(lerp(color0, color1, 0.75).into()), ..Default::default() }) .spawn(SpriteBundle { sprite: Sprite { size: Vec2::splat(50.), ..Default::default() }, material: materials.add(lerp(color0, color1, 1.).into()), ..Default::default() }); }); } fn lerp(color0: Color, color1: Color, t: f32) -> Color { let r0 = color0.r(); let g0 = color0.g(); let b0 = color0.b(); let r1 = color1.r(); let g1 = color1.g(); let b1 = color1.b(); Color::rgb(r0 + t * (r1 - r0), g0 + t * (g1 - g0), b0 + t * (b1 - b0)) } fn lerp_linear(color0: Color, color1: Color, t: f32) -> Color { let r0 = color0.r_linear(); let g0 = color0.g_linear(); let b0 = color0.b_linear(); let r1 = color1.r_linear(); let g1 = color1.g_linear(); let b1 = color1.b_linear(); Color::rgb_linear(r0 + t * (r1 - r0), g0 + t * (g1 - g0), b0 + t * (b1 - b0)) } ```

This changes when you add colours.

    let color0 = Color::RED;
    let color1 = Color::GREEN;

image Reference: https://www.colorhexa.com/ff0000-to-00ff00 In this case, the left side (linear) looks more pleasing / expected.

But when the colours look like this:

    let color0 = Color::YELLOW;
    let color1 = Color::BLUE;

image Reference: https://www.colorhexa.com/ffff00-to-0000ff Neither seem appropriate.

In conclusion, colour is weird. Perhaps it makes sense to use something like palette.

bjorn3 commented 3 years ago

Both groups of squares represent values from black outside to white inside (with a 0.25 step change). Left is linear RGB. Right is non-linear sRGB. The right-hand looks most correct in terms of interpolation, which I think you can see with the 25% white square: for linear there is a sharp contrast between 0% and 25% white, for sRGB it is much closer.

Our perception of brightness is a non-linear function of the actual amount of amount of energy the photons hitting our eyes have. In the range black to white this function is somewhat close to the gamma function of sRGB, but not exactly. If you go from non-saturated to saturated, you will perceive the saturated colors brighter than non-saturated colors that use the same amount of energy for the photons. In fact our perception of colors depends on the exact lights used in the room around us.

Reference: https://www.colorhexa.com/ff0000-to-00ff00 In this case, the left side (linear) looks more pleasing / expected.

This is a quirk of the way sRGB works.

If you want a perceptually uniform lerp, you need to use a perceptually uniform colorspace like CIELAB. If you want a physically accurate lerp, you need to use linearRGB. It is not possible to have a single lerp suitable for all purposes, so you will need to use a different lerp for different purposes. Maybe we could call them linear_lerp and perceptual_lerp with appropriate documentation on the Color type about linearRGB and perceptually uniform colorspaces?

Edit:

Perhaps it makes sense to use something like palette.

Definitively!

RustyStriker commented 3 years ago

Looking at the links @tigregalis sent, it seems the most eye pleasing approach is to lerp on the hsv values, may result in a bit more work but it looks the most pleasing to the eye, most correct(most of the times, tho maybe an option to do a "reverse" lerp on the Hue) as it keeps saturation and value consistent across the steps.

Reference: https://www.colorhexa.com/ffff00-to-0000ff Neither seem appropriate.

In this case it seems HSV Gradient is the most correct transformation(as RGB gradient stops at grey in the middle, losing color all together)

Reference: https://www.colorhexa.com/ff0000-to-00ff00 In this case, the left side (linear) looks more pleasing / expected.

Here you can clearly see the difference in both value and saturation, resulting in uglier colors.

RustyStriker commented 3 years ago

This is what it would look like going one way only in the Hue channel.

Screenshot from 2021-02-07 13-45-00

Code for the interested: https://gist.github.com/RustyStriker/b9b48a7436f7e31d9d9e0495df26719b

inodentry commented 3 years ago

We should define all operations on colors in linear space, because that is the correct way to do it. There is more than enough ugliness in the world due to software not handling gamma properly (or users not knowing how to use it in a way that handles gamma properly). Let's not contribute to that. Let's make APIs that make it easy for users to do the right thing by default.

If we support operations on both Linear space and sRGB space, we should name them lerp and lerp_srgb, not lerp_linear and lerp. Add the extra suffix on the sRGB versions, to indicate that they are special. Make the easier and simpler option be the one that is most often the correct one to use.

We should do the same for all mathematical operations on colors. Work in linear space by default. Maybe add special methods for sRGB.

RustyStriker commented 3 years ago

We can also try a more "rust" approach, by following the way basic integers are implemented, simply create 3 types of color structs, each with functions relative to that specific color space, and have the ability to switch between the 3. That approach will remove any confusion on what color space you work with and will allow for all methods(and color specific methods) to work in a more "predictable" way

tigregalis commented 3 years ago

We can also try a more "rust" approach, by following the way basic integers are implemented, simply create 3 types of color structs, each with functions relative to that specific color space, and have the ability to switch between the 3. That approach will remove any confusion on what color space you work with and will allow for all methods(and color specific methods) to work in a more "predictable" way

That was I believe @cart 's eventual intent. But there was some mention of smarter GPU data conversions: converting into the appropriate colour space on the GPU itself?

However, Rust already has a crate designed exactly for this purpose (colour manipulation in multiple colour spaces) which is palette so perhaps we would be best served attempting to integrate with that. @cart what do you think of this?

cart commented 3 years ago

Yeah I think splitting the types out is the right call, but we'll need to sort out the gpu conversion first.

I'm open to using palette, but I'm also hesitant to adopt another dependency for just a couple of well documented conversion algorithms. I'd prefer to convert what we have to the types we want first, then decide how good of a fit palette is.

tigregalis commented 3 years ago

Yeah I think splitting the types out is the right call, but we'll need to sort out the gpu conversion first.

What does this involve?

I'm open to using palette, but I'm also hesitant to adopt another dependency for just a couple of well documented conversion algorithms. I'd prefer to convert what we have to the types we want first, then decide how good of a fit palette is. That's fair.

If we were to limit this to just conversions, then palette would certainly be overkill. I think the benefit is in doing colour manipulation, that it can be maintained outside of Bevy, and it is feature-rich already: it basically has all the colour spaces and colour manipulation functions anyone could want.

An alternative approach might be to have some From implementations (behind a feature flag) for converting between Bevy types and palette types, but I don't know whether you support this sort of pattern.

cart commented 3 years ago

What does this involve?

Either a manual RenderResources trait impl that handles the Srgb (high level color struct) -> Linear (format expected by gpu) conversion internally, or adding a way to properly express that conversion in the RenderResources derive (and/or associated RenderResource graph nodes)

An alternative approach might be to have some From implementations (behind a feature flag) for converting between Bevy types and palette types, but I don't know whether you support this sort of pattern.

Yeah I'm open to this, but in general I want to work toward "one code path" whenever possible. Supporting "bevy color ops" and "pallete color ops" makes it harder to document and fractures things like tutorials and the "ecosystem". I think we should design our ideal "Bevy Color API" and if that happens to line up perfectly with palette types with no major compromises (and no major cruft), then we can consider taking on palette as a dep.

alice-i-cecile commented 3 years ago

colorgrad-rs looks like another interesting crate in this space.

RustyStriker commented 2 years ago

How about concealing the actual value behind as_linear_rgb() as_srgb() as_hsv() and other color spaces/representations, thus forcing the user to choose a color space each time(internal representation can be either the last used, to try and avoid converting all the time, or the representation the gpu uses eventually).

This method will have to be supplied with from_color_space() functions instead of new(), and a way to write back to it(which if we just keep the last color space used, we can simply pass a reference to the internal value post conversion then let the user manipulate it directly with having to invoke another function)

2 big benefits are:

mockersf commented 2 years ago

an article on colour spaces from the point of view of gradient, relevant to lerping: https://raphlinus.github.io/color/2021/01/18/oklab-critique.html

viridia commented 9 months ago

Been thinking about this, since I also need color lerping for my project.

The problem is that Color, as currently designed, is too flexible: while I understand the convenience of being able to read colors in various input formats, once you actually have the color and want to operate on it (e.g. lerping, mixing, lighten, darken and so on), then having multiple formats creates complexity and confusion. It's relatively easy to interpolate from linear RGB to linear RGB, HSL to HSL, and so on - but not knowing in advance what color space is going to be used makes the problem intractable.

My work tends to focus on UI, and from that standpoint, the design of the Color class is suboptimal. I would be happier with a single, simple representation - linear RGB would work fine - and operate on it in that format. In cases where I want to do interpolation in HSL, I'd be happy to manually convert the start and end points of the interpolation to an explicit HSL format and lerp those.

However, I recognize that my requirements may not match for people who are, say, working with textures or GLSL files. That being said, I think that once we get down to the GPU level it's always going to be linear RGB, right?

What about this: define two different Color classes - let's call one ColorRepresentation, which supports multiple color spaces, and which is used when importing colors from outside sources, and the other just RGBA, which is used for all internal calculations. There would of course be conversion methods between the two types.

viridia commented 9 months ago

In the interest of getting this issue unblocked: I think what we need is a formal requirements/design doc.

I spent a few minutes looking at the sources for palette. It is indeed a big library, although most of the functions within it are very small. It has a modest number of dependencies. Some of the size / complexity arises due to supporting exotic color spaces.

Just in the Rgb color space alone palette supports the following types: GammaSrgb, GammaSrgba, LinSrgb, LinSrgba, PackedAbgr, PackedArgb, PackedBgra, PackedRgba, Rgba, Srgb, Srgba.

Palette also has traits for all the various operations - so for example, "Saturate" is a trait which has implementations for each of the different color spaces.

The question of whether to adopt something like palette really depends on what we want, which currently isn't written down in any kind of detail.

It's also worth considering what features should be optional. A user who only wants to load and display a GLTF model doesn't need hue rotation or mixing.

Let's say we decide not to adopt palette and write our own. Here's a strawman proposal:

I'm sure that there will be objections to what I've written here - that's useful feedback.

viridia commented 9 months ago

colorgrad-rs looks like another interesting crate in this space.

@alice-i-cecile Interesting, but not really helpful for my use case. I'm interested in lerping colors for CSS-style animations, at some point I'll also need static gradients but that's further down the road.

rlidwka commented 9 months ago

@alice-i-cecile Interesting, but not really helpful for my use case. I'm interested in lerping colors for CSS-style animations

Why is it not helpful? Seems to work fine for me:

use colorgrad::{BlendMode, Color, CustomGradient};

fn lerp(c1: Color, c2: Color, t: f64) -> Color {
    CustomGradient::new()
        .colors(&[c1, c2])
        .mode(BlendMode::Oklab)
        .domain(&[0.0, 1.0])
        .build()
        .unwrap()
        .at(t)
}

dbg!(lerp(Color::new(0., 0., 0., 1.), Color::new(1., 1., 1., 1.), 0.5));
viridia commented 9 months ago

@alice-i-cecile I think we should open up a separate ticket for refactoring the color types, since this is really about more than just lerping.

alice-i-cecile commented 9 months ago

Agreed: can you put together one? I think we have a reasonably clear design at this point but you know the details better than I.

viridia commented 9 months ago

@alice-i-cecile My only concern is that I'm not a rendering pipeline guy, I tend to specialize for things like on UI, game logic and scripting languages. And obviously, changing the representation of Color is likely going to have a big impact on the rendering pipeline as well as other parts of the engine I'm not familiar with. That being said, if there's a SME or someone who can critique my design docs, I'd feel more comfortable doing it.