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

Add methods for getting and setting individual color channels for all color spaces. #7759

Closed
wants to merge 8 commits into from
Closed

Add methods for getting and setting individual color channels for all color spaces. #7759

wants to merge 8 commits into from

Conversation

LiamGallagher737
Copy link
Member

@LiamGallagher737 LiamGallagher737 commented Feb 20, 2023

Objective

Fixes #7746 except for making them const which is blocked on const float arithmetic.

Solution

Added a bunch more methods for getting and setting individual color channels for every color space.

Changelog

Added

  • X_Y Get the Y value in linear X colorspace.
  • set_X_Y Set the Y value in X colorspace.
  • with_X_Y Returns this color with Y set to a new value in X colorspace.

For all the remaining channels and color spaces.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use labels Feb 20, 2023
Copy link
Contributor

@coreh coreh left a comment

Choose a reason for hiding this comment

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

Looks mostly good! Left some very minor comments re: naming, method consolidation and behavior of setters, but after that, 👍 from me

crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Show resolved Hide resolved
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
@LiamGallagher737 LiamGallagher737 requested a review from coreh March 7, 2023 03:16
crates/bevy_render/src/color/mod.rs Outdated Show resolved Hide resolved
@mockersf
Copy link
Member

For the docs on all the set_ functions, it doesn't just return in the given kind, it also modified self.

I so much want to remove that file 🙁

@LiamGallagher737
Copy link
Member Author

I so much want to remove that file 🙁

I personally think using a crate like palette is the way to go

@Selene-Amanita Selene-Amanita 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 Aug 15, 2023
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Aug 21, 2023

Fixed in #920. Choice was made fairly arbitrarily; feel free to submit follow-up PRs for nits.

@LiamGallagher737 LiamGallagher737 deleted the more-color-channel-getters branch March 2, 2024 22:20
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-Usability A targeted quality-of-life change that makes Bevy easier to use 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.

Add const fn Color::hue, Color::saturation and Color::lightness etc
6 participants