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

bevy_color: Add Laba to support Lab Color Model #12115

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

bushrat011899
Copy link
Contributor

@bushrat011899 bushrat011899 commented Feb 26, 2024

Objective

  • Improve compatibility with CSS Module 4
  • Simplify Lcha conversion functions

Solution

  • Added Laba which implements the Lab color model.
  • Updated Color and LegacyColor accordingly.

Migration Guide

  • Convert Laba to either Xyza or Lcha using the provided From implementations and then handle accordingly.

Notes

The Lab color space is a required stepping stone when converting between XYZ and Lch, therefore we already use the Lab color model, just in an nameless fashion prone to errors.

This PR also includes a slightly broader refactor of the From implementations between the various colour spaces to better reflect the graph of definitions. My goal was to keep domain specific knowledge of each colour space contained to their respective files (e.g., the From<Oklaba> for LinearRgba definition was in linear_rgba.rs when it probably belongs in oklaba.rs, since Linear sRGB is a fundamental space and Oklab is defined in its relation to it)

@bushrat011899 bushrat011899 added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide labels Feb 26, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Feb 26, 2024
@alice-i-cecile
Copy link
Member

The prerequisite PR has been merged :) Please rebase when you get a chance and we can add this in.

Also added a couple of `macro_rules` definitions to automate highly repetitive definitions.
Copy link
Contributor

@viridia viridia left a comment

Choose a reason for hiding this comment

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

Do we want to explain about derived conversions in the crate-level docs?

@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 Feb 26, 2024
@alice-i-cecile
Copy link
Member

Yeah, I'd like to develop a bit of a map for the conversions in the crate level docs. That can be split out into another PR easily though.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Feb 26, 2024
@viridia
Copy link
Contributor

viridia commented Feb 26, 2024

Also, I know we've been trying to explain to new users what all these various color spaces are for but at some point we will have to give up, because the distinctions are (a) hair-splitting and (b) historical, which makes it hard to write a clear and distinct explanation.

@bushrat011899
Copy link
Contributor Author

I was thinking it might be nice to actually include a diagram showing how the various colour spaces relate to each other, but I'm not sure what the best way to include that in Rust Docs would be.

Example Diagram

@alice-i-cecile
Copy link
Member

If we go that route, I'd recommend aquamarine, which pulls in mermaid support for rustdoc.

@bushrat011899
Copy link
Contributor Author

That's definitely the route I'd want to take. I'll make a little PR for this now

Merged via the queue into bevyengine:main with commit 4d325eb Feb 26, 2024
26 checks passed
msvbg pushed a commit to msvbg/bevy that referenced this pull request Feb 27, 2024
# Objective

- Improve compatibility with CSS Module 4
- Simplify `Lcha` conversion functions

## Solution

- Added `Laba` which implements the Lab color model.
- Updated `Color` and `LegacyColor` accordingly.

## Migration Guide

- Convert `Laba` to either `Xyza` or `Lcha` using the provided `From`
implementations and then handle accordingly.

## Notes

The Lab color space is a required stepping stone when converting between
XYZ and Lch, therefore we already use the Lab color model, just in an
nameless fashion prone to errors.

This PR also includes a slightly broader refactor of the `From`
implementations between the various colour spaces to better reflect the
graph of definitions. My goal was to keep domain specific knowledge of
each colour space contained to their respective files (e.g., the
`From<Oklaba> for LinearRgba` definition was in `linear_rgba.rs` when it
probably belongs in `oklaba.rs`, since Linear sRGB is a fundamental
space and Oklab is defined in its relation to it)
@BD103 BD103 added the A-Color Color spaces and color math label May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Color Color spaces and color math A-Rendering Drawing game state to the screen 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants