Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance bevy_render::color with palette #9698

Closed

Conversation

bushrat011899
Copy link
Contributor

@bushrat011899 bushrat011899 commented Sep 5, 2023

Objective

Solution

This is a major rework of the bevy_render::color module. The goal of this PR is to provide more color manipulation options to Bevy and to end users by leveraging the palette crate. There are numerous alternative color libraries for Rust:

Out of the ones above, I determined palette to be the best suited for Bevy based on the following criteria:

  1. Feature-Complete: palette supports all colorspaces, conversions, and operations that Bevy currently implements itself.
  2. Popular: Excluding rgb, palette is the most popular color management library on crates.io, with well over a million downloads.
  3. Licence-Compatible: palette is available in both MIT and Apache-2.0 licences, matching Bevy's current licensing arrangement.

What this PR does is replace the entire color module with a single file containing:

  1. A re-export of palette
  2. A Color struct.

This new Color struct is a custom color type using the FromColorUnclamped derive macro. Internally, Color is a simple f32 sRGBA type equivalent to palette::Srgba:

#[derive(
    FromColorUnclamped, WithAlpha, Debug, Clone, Copy, PartialEq, Serialize, Deserialize, Reflect,
)]
#[reflect(PartialEq, Serialize, Deserialize)]
#[palette(skip_derives(Rgb), rgb_standard = "encoding::Srgb")]
pub struct Color {
    /// Red channel. [0.0, 1.0]
    r: f32,
    /// Green channel. [0.0, 1.0]
    g: f32,
    /// Blue channel. [0.0, 1.0]
    b: f32,
    /// Alpha channel. [0.0, 1.0]
    #[palette(alpha)]
    a: f32,
}

The reason for creating a custom color type is to allow for all the same custom implementations that Bevy already has for its existing Color type. Its compatibility with serde, wgpu, encase, and reflect is unchanged.

The reason I chose a single colorspace for Color as a struct rather than an enum of multiple spaces is to avoid the current confusion around space-specific methods and properties. Issues #8838 and #7746 are largely caused by the broad nature of the current Color type. By choosing sRGBA as the canonical colorspace for Color, the potential for confusion should be almost entirely eliminated. As an additional bonus, Color as a struct now uses 4 fewer bytes for the same information and fidelity. While likely negligible difference in performance, for such a fundamental type, I believe this is a notable benefit.

Color Spaces

To make working with this more restrictive Color type easier, I've included constructors and named converters for all the colorspaces currently used within the current enum:

impl Color {
    /* snip */

    /// New `Color` with HSL representation in sRGB colorspace.
    pub fn hsla(hue: f32, saturation: f32, lightness: f32, alpha: f32) -> Self {
        palette::Hsla::new(hue, saturation, lightness, alpha).into_color()
    }

    /// Converts this `Color` into HSLA
    pub fn as_hsla(self) -> palette::Hsla {
        self.in_space()
    }

    /* snip */
}

Using palette, the entire colorspaces.rs file is redundant and can be replaced. A particular idiosyncrasy I noticed within that file is how the LCH color space is converted into and out of sRGB (LCH -> Lab -> XYZ -> Linear sRGB -> sRGB). This is not a bad algorithm, but it does feel unfortunate that Bevy contained conversion methods for Lab and XYZ, but in a way that nobody could actually take advantage of it. Now, any colorspace supported by palette is available to Bevy:

Space Type Description
GammaLuma Gamma 2.2 encoded luminance.
GammaLumaa Gamma 2.2 encoded luminance with an alpha component.
GammaSrgb Gamma 2.2 encoded sRGB.
GammaSrgba Gamma 2.2 encoded sRGB with an alpha component.
Hsla Linear HSL with an alpha component. See the Hsla implementation in Alpha.
Hsluva HSLuv with an alpha component. See the Hsluva implementation in Alpha.
Hsva Linear HSV with an alpha component. See the Hsva implementation in Alpha.
Hwba Linear HWB with an alpha component. See the Hwba implementation in Alpha.
Laba CIE Lab* (CIELAB) with an alpha component. See the Laba implementation in Alpha.
Lcha CIE LCh° with an alpha component. See the Lcha implementation in Alpha.
Lchuva CIE LCuv h°uv with an alpha component. See the Lchuva implementation in Alpha.
LinLuma Linear luminance.
LinLumaa Linear luminance with an alpha component.
LinSrgb Linear sRGB.
LinSrgba Linear sRGB with an alpha component.
Luva CIE Luv* (CIELUV) with an alpha component. See the Luva implementation in Alpha.
Okhsla Okhsl with an alpha component.
Okhsva Okhsv with an alpha component. See the Okhsva implementation in Alpha.
Okhwba Okhwb with an alpha component. See the Okhwba implementation in Alpha.
Oklaba Oklab with an alpha component.
Oklcha Oklch with an alpha component. See the Oklcha implementation in Alpha.
Srgb Non-linear sRGB.
SrgbLuma sRGB encoded luminance.
SrgbLumaa sRGB encoded luminance with an alpha component.
Srgba Non-linear sRGB with an alpha component.
Xyza CIE 1931 XYZ with an alpha component. See the Xyza implementation in Alpha.
Yxya CIE 1931 Yxy (xyY) with an alpha component. See the Yxya implementation in Alpha.

mix_in

Mixes two Color values at a particular ratio and in any arbitrary space. This is a wrapper for the palette::Mix trait:

use bevy_render::color::{Color, palette::Oklcha}

let red = Color::RED;
let blue = Color::BLUE;
let purple = blue.mix_in::<Oklcha>(red, 0.5);

assert_eq!(purple, Color::rgba(0.7299066, 0., 0.7600829, 1.));

This resolves the long-standing #1402, without enforcing a specific colorspace to lerp within. Again, note that the only use that is required (aside from Color obviously) is the space you intend on working in.

hue_in / with_hue_in / shift_hue_in

These methods allow for access, replacement, and rotation of a Color's hue in any compatible space, using the palette:GetHue, palette::WithHue, and palette::ShiftHue triats:

use bevy_render::color::{Color, palette::Hsva}

let color = Color::PURPLE.shift_hue_in::<Hsva>(60.); // Mid-Red
let hue = color.hue_in::<Hsva>(); // 0 Degrees
let color = color.with_hue_in::<Hsva>(180.); // Mid-Cyan

saturate_in / desaturate_in

These methods allow relative adjustment to saturation via palette::Saturate and palette::Desaturate. It's important to note these are relative adjustments to saturation, not absolute:

use bevy_render::color::{Color, palette::Hsla}

let color = Color::RED; // rgb(255, 0, 0)
let color_pale = color.saturate_in::<Hsla>(0.5); // rgb(255, 128, 128)
let color_paler = color_pale.saturate_in::<Hsla>(0.5); // rgb(255, 191, 191)

lighten_in / darken_in

These allow relative adjustment to brightness/lightness/etc. via palette::Lighten and palette::Darken.

use bevy_render::color::{Color, palette::Hsla}

let color = Color::PURPLE; // rgb(127, 0, 127)
let color_pale = color.lighten_in::<Hsla>(0.5); // rgb(191, 0, 191)
let color_paler = color_pale.lighten_in::<Hsla>(0.5); // rgb(255, 32, 255)

in_space / from_space

These are convenience methods that wrap palette's FromColorUnclamped and IntoColor, which themselves are analogous to Rust's native From and Into traits. The reason for these wrapper methods is to allow the use of these traits without importing them explicitly, and to avoid the (fairly clunky in my opinion) <X as Y>::method(...) syntax.

use bevy_render::color::{Color, palette::Hsla}

let red = Color::RED;
let red_hsla = red.in_space::<Hsla>();
let hue = red_hsla.hue; // 0 Degrees

assert_eq!(Color::RED, Color::from_space(red_hsla));

These transforms are the backbone for all the previous Color operations. But, I would recommend for complex color operations, an end user should convert into their desired space, operate, and then convert back at the end. The following methods perform From and Into operations on either side, introducing wasted compute for chained operations.

use bevy_render::color::{Color, palette::{Oklcha, Mix, Lighten, ShiftHue}}

// sRGBA Color Space
let red = Color::RED;
let blue = Color::BLUE;

// Oklcha Color Space
let red = red.in_space::<Oklcha>();
let blue = blue.in_space::<Oklcha>();

let player_health = red.lighten(0.5);
let enemy_health = red.mix(blue, 0.5).lighten(0.25);
let ally_health = blue.lighten(0.5);

// sRGBA Color Space
let player_health = Color::from_space(player_health);
let enemy_health = Color::from_space(enemy_health);
let ally_health = Color::from_space(ally_health);

The above is a small example, but I think the point is fairly clear.

difference_in

This method allows for CIEDE2000 color difference calculation within a specified colorspace.

let blue = Color::BLUE;
let red = Color::RED;

let purple = red.mix_in::<palette::Srgba>(blue, 0.5);

assert!(purple.difference_in::<palette::Lch>(Color::PURPLE).abs() < 1e-3);

The utility of this method is to allow for the approximate comparison of arbitrary colors in a way that should be more robust to floating point errors. palette offers other difference metrics which can be cheaper to compute, but are incongruent with typical human perception.

perceptually_eq

This method is an opinionated convenience wrapper for difference_in which operates in Lch and considers a CIEDE2000 difference of 1.0 to be the threshold for a just noticeable difference.

let a = Color::hsl(0., 1.0, 0.5);
let b = Color::hsl(1., 1.0, 0.5);

// A 1 degree shift in hue within HSL isn't visually distinctive.
assert!(a.perceptually_eq(b));

let a = Color::hsl(0., 1.0, 0.5);
let b = Color::hsl(10., 1.0, 0.5);

// A 10 degree shift is.
assert!(!a.perceptually_eq(b));

Remarks

This is a major change in how Bevy works with color, but I've preserved enough of the public facing API of Color that I believe the pain of transition is well worth the greater flexibility. I'm submitting this initially as a draft PR due to the certainly controversial nature of this PR, and I would greatly appreciate feedback on how best to tackle making a change like this (e.g., possible RFC)

Migration Guide

Color Is Not an enum

match statements over the color variants no longer work, since there is only a single variant! As such, any code that looks like this:

match color {
    Color::Rgba { red, green, blue, alpha } => {
        // Some Work
    },
    // ...
}

Can be replaced with:

let Srgba { red, green, blue, alpha } = Color.in_space::<Srgba>();

// Some Work

as_some space No Longer Returns Color

Any code which needed to transform a Color into a particular space must now work with the concrete type that represents that space. This is a fairly major workflow change, but I think a positive one. I think there are two cases where these methods were being used:

  1. To immediately work on a Color. In which case, the concrete type saves on a match statement with an unreachable!() wildcard arm.
  2. To prepare a Color for work future work. In which case, that future work still needs to match over all possible variants, with reasonable conversion anyway.

One downside is that a Color as a part of a component may receive repeated conversions into and out of sRBGA if a system is operating on it. However, I believe this wont be a significant issue with the following workarounds:

  1. Store the concrete colorspace in the component instead, better communicating intent.
  2. Modify Color to lazily convert to sRGBA, storing the provided colorspace representation internally. Since Color is no longer a public enum, this option is now viable, along with any other metadata the rendering team may want to hide within Color for whatever reason.

with_X / set_X Are Removed

Methods which provided access to non-sRGBA values (lightness, chroma, hue, saturation, etc.) have all been removed. My reasoning for this is palette supports so many alternate colorspaces, I fear adding a single h method would only add confusion, even if properly documented.

This is pretty easy to get around regardless, simply use color.in_space::<S>() to get into the space you want to work in, access the color property you need, and then use Color::from_space(color) to get back.

let hue = color.h(); // Hue in what space? This method is now removed.

let hue = color.in_space::<Hsv>().hue; // Oh HSV!

Another benefit to this change is no property accessor performs implicit conversion, avoiding possible wasted compute without the user's intent.

Arithmetic Operations Are Only in sRGB(A)

This might be controversial, and I'm open to being told otherwise, but I believe this is a trap for new users:

fn a_mistake_in_waiting(color: Color) -> Color {
    0.5 * color
}

The reason is that scalar multiplication of Color was dependent on whatever space the enum was already in. If color was sRGBA, it would scale the color channels, which I think is what a naive user would expect. But if they happened to be passed a Color::Hsla variant instead, now the function does something entirely different. Where I see this being particularly hairy is with the provided constant colors (Color::RED, etc.), as whatever space they're defined in will decide how the above hypothetical function will behave.

Regardless, I've included all the same operator definitions that the original Color enum, just exclusively for the sRGB(A) color space. In my opinion, this still provides all the same critical functionality, without the potential foot-guns.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen X-Controversial There is active debate or serious implications around merging this PR C-Bug An unexpected or incorrect behavior C-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Sep 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Sep 5, 2023

Note that this was previously discussed in #1146 and #4648, and was quite contentious at the time.

I personally think we should probably do this (see the remaining open bugs and usability problems), but it's a call for @cart and the rendering SMEs (@superdump, @robtfm).

An RFC might be useful, but your PR description is really solid, and may suffice.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Your PR increases Bevy Minimum Supported Rust Version. Please update the rust-version field in the root Cargo.toml file.

@bushrat011899 bushrat011899 marked this pull request as ready for review September 8, 2023 12:41
@bushrat011899
Copy link
Contributor Author

I believe this PR is ready for review. I've addressed the warnings generated by the github-actions bot, updated documentation, and fleshed out the PR description. Feel free to reach out if anyone has any questions! 😊

Copy link
Contributor

@ickshonpe ickshonpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really appreciate the detail you went into with the PR description. The single representation srgba Color struct feels like a good change to me, more opinionated, less confusing, and I wasn't able to find any obvious mistakes in the code or comments.

I don't have enough knowledge about colour spaces and rendering to have a useful opinion on approval though unfortunately.

Copy link
Contributor

@hymm hymm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally in favor of the direction of this PR. Making everything more explicit feels like the right choice as color spaces are pretty complex and losing precision from implicit conversions feels wrong.

It'd be nice to have an example of animating in different color spaces.

examples/ui/grid.rs Show resolved Hide resolved
deny.toml Outdated Show resolved Hide resolved
alpha: 1.0,
}
#[must_use]
pub fn hsla(hue: f32, saturation: f32, lightness: f32, alpha: f32) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want all these constructors for Color from alternative color spaces. I feel like having the user do my_color = Hsla::new(h, s, l, a).into_color() will make is explict that some conversion (loss of precision) is going on and they'll be more likely to store a Hsla when it makes sense to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfectly reasonable. I kept them to reduce the effort required to migrate user code, but they could be removed (or expanded to other types). Does anyone else have strong feelings either way regarding these space-specific constructors?

crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Show resolved Hide resolved
@bushrat011899
Copy link
Contributor Author

I'm generally in favor of the direction of this PR. Making everything more explicit feels like the right choice as color spaces are pretty complex and losing precision from implicit conversions feels wrong.

It'd be nice to have an example of animating in different color spaces.

This is a little verbose, but here's an example controlling the window clear color in Oklch with minimal conversions:

//! Shows how to dynamically change a color in an arbitrary color space.
//!
//! Use the arrow keys, and left Control and Shift keys to control hue, lightness,
//! and chroma.
//!
//! Conversion into sRGBA only occurs when the color has changed.

use bevy::{prelude::*, render::color::palette};
use palette::{IntoColor, DarkenAssign, LightenAssign, ShiftHueAssign};

#[derive(Resource)]
struct Background {
    current: palette::Oklcha,
}

fn main() {
    App::new()
        .init_resource::<ClearColor>()
        .insert_resource(FixedTime::new_from_secs(1. / 30.))
        .insert_resource(Background {
            current: Color::rgb(0.5, 0.5, 0.9).into_color(),
        })
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .add_systems(FixedUpdate, change_background_color)
        .add_systems(
            Update,
            update_clear_color_from_background.after(change_background_color),
        )
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
}

fn update_clear_color_from_background(
    background: Res<Background>,
    mut clear_color: ResMut<ClearColor>,
) {
    if background.is_changed() {
        clear_color.0 = Color::from_space(background.current);
    }
}

fn change_background_color(input: Res<Input<KeyCode>>, mut background: ResMut<Background>) {
    if input.pressed(KeyCode::Right) {
        background.current.shift_hue_assign(1.);
    }

    if input.pressed(KeyCode::Left) {
        background.current.shift_hue_assign(-1.);
    }

    if input.pressed(KeyCode::Up) {
        background.current.lighten_assign(0.02);
    }

    if input.pressed(KeyCode::Down) {
        background.current.darken_assign(0.02);
    }

    if input.pressed(KeyCode::ShiftLeft) {
        background.current.chroma *= 1.02;
    }

    if input.pressed(KeyCode::ControlLeft) {
        background.current.chroma *= 0.98;
    }
}

Because these colors could be changing constantly, I chose to store the Oklch representation in a resource. But if the background wasn't changing often, you could instead use the Color type itself, and the associated X_in methods.

@bushrat011899
Copy link
Contributor Author

bushrat011899 commented Sep 18, 2023

As another example, here's one to demonstrate color mixing in 5 different color spaces, using a basic quadratic spline (lerp of lerp):

//! Demonstrates the differences in blending colors depending on which space the operation
//! is performed in.
//! 
//! This demo cycles through a quadratic spline over `Color::RED`, `Color::GREEN`, and
//! `Color::BLUE` using a `sin()` driver.

use bevy::{prelude::*, render::color::palette};

#[derive(Component, Default)]
struct Slider<T>(std::marker::PhantomData<T>);

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, |mut commands: Commands| {
            commands.spawn(Camera2dBundle::default());

            commands
                .spawn(NodeBundle {
                    style: Style {
                        display: Display::Grid,
                        width: Val::Percent(100.0),
                        height: Val::Percent(100.0),
                        grid_template_columns: std::iter::repeat(GridTrack::auto())
                            .take(5)
                            .collect(),
                        grid_template_rows: vec![GridTrack::auto()],
                        ..default()
                    },
                    background_color: BackgroundColor(Color::WHITE),
                    ..default()
                })
                .with_children(|builder| {
                    builder
                        .spawn((NodeBundle::default(), Slider::<palette::Srgb>::default()))
                        .with_children(|builder| {
                            builder.spawn(TextBundle::from_section("sRGB", default()));
                        });
                    builder
                        .spawn((NodeBundle::default(), Slider::<palette::LinSrgb>::default()))
                        .with_children(|builder| {
                            builder.spawn(TextBundle::from_section("Linear sRGB", default()));
                        });
                    builder
                        .spawn((NodeBundle::default(), Slider::<palette::Hsl>::default()))
                        .with_children(|builder| {
                            builder.spawn(TextBundle::from_section("HSL", default()));
                        });
                    builder
                        .spawn((NodeBundle::default(), Slider::<palette::Lch>::default()))
                        .with_children(|builder| {
                            builder.spawn(TextBundle::from_section("Lch", default()));
                        });
                    builder
                        .spawn((NodeBundle::default(), Slider::<palette::Oklch>::default()))
                        .with_children(|builder| {
                            builder.spawn(TextBundle::from_section("Oklch", default()));
                        });
                });
        })
        .add_systems(
            Update,
            (
                slider_updater::<palette::Srgb>,
                slider_updater::<palette::LinSrgb>,
                slider_updater::<palette::Hsl>,
                slider_updater::<palette::Lch>,
                slider_updater::<palette::Oklch>,
            ),
        )
        .run();
}

fn slider_updater<T>(mut query: Query<&mut BackgroundColor, With<Slider<T>>>, time: Res<Time>)
where
    T: Send
        + Sync
        + 'static
        + palette::Mix<Scalar = f32>
        + palette::convert::FromColorUnclamped<Color>
        + palette::Clamp,
    Color: palette::convert::FromColorUnclamped<T>,
{
    let t = 0.5 + 0.5 * time.elapsed_seconds_wrapped().sin();

    const P1: Color = Color::RED;
    const P2: Color = Color::GREEN;
    const P3: Color = Color::BLUE;

    let p12 = P1.mix_in::<T>(P2, t);
    let p23 = P2.mix_in::<T>(P3, t);
    
    let new_color = p12.mix_in::<T>(p23, t);

    for mut old_color in query.iter_mut() {
        // Example of using perceptual equality to avoid color mutation
        if !new_color.perceptually_eq(old_color.0) {
            old_color.0 = new_color;
        }
    }
}

The above shows how to create a function which is generic over any color space supporting certain palette operations (Mix, Clamp, etc.). I don't expect end users to write systems like this, as I expect they will choose a specific color space and stick with it, likely the Bevy Color. I'm using it hear to allow me to use a simple marker component (Slider<T>) to abstract over the color animation.

Notice that in this screenshot, the same point in the gradient can produce completely different results:
image

Below is an animated GIF version of this test. Please take this with a grain of salt due to how a GIF compresses color information.
ColorTest

@hymm
Copy link
Contributor

hymm commented Sep 25, 2023

I actually meant add a example to the repo. I like the second one since it shows users that interpolating in different color spaces leads to different results.

Fixed `color_mixing` Formatting
The errors raised during `cargo deny check bans` appear to be unrelated to `palette`
@bushrat011899
Copy link
Contributor Author

I actually meant add a example to the repo. I like the second one since it shows users that interpolating in different color spaces leads to different results.

Can do! I've added it to the ui group of examples since it uses UI elements to demonstrate color mixing.

@hymm hymm added this to the 0.12 milestone Sep 28, 2023
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 30, 2023
@hymm hymm mentioned this pull request Dec 15, 2023
@alice-i-cecile alice-i-cecile removed this from the 0.13 milestone Jan 24, 2024
@bushrat011899
Copy link
Contributor Author

Closing in favour of #12013.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior C-Feature A new feature, making something new possible M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Controversial There is active debate or serious implications around merging this PR
Projects
None yet
6 participants