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.

Sign up for GitHub

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

Jump to bottom

Adds back in way to convert color to u8 array, implemented for the two RGB color types, also renames Color::linear to Color::to_linear as discussed on discord. #13739

Closed
wants to merge 5 commits into from
Closed

Adds back in way to convert color to u8 array, implemented for the two RGB color types, also renames Color::linear to Color::to_linear as discussed on discord. #13739

wants to merge 5 commits into from

Conversation

gagnus
Copy link
Contributor

@gagnus gagnus commented Jun 7, 2024 •
edited
Loading

Objective

  • One thing missing from the new Color implementation in 0.14 is the ability to easily convert to a u8 representation of the rgb color.

Solution

  • I have added to_u8_array and to_u8_array_no_alpha to a new trait called ColorToPacked to mirror the f32 conversions in ColorToComponents and implemented the new trait for Srgba and LinearRgba.
  • To go with those I also added matching from_u8... functions and converted a couple of cases that used ad-hoc implementations of that conversion to use these.
  • After discussion on Discord of the experience of using the API I renamed Color::linear to Color::to_linear, as without that it looks like a constructor (like Color::rgb).
  • I also added to_srgba which is the other commonly converted to type of color (for UI and 2D) to match to_linear.
  • Removed a redundant extra implementation of to_f32_array for LinearColor as it is also supplied in ColorToComponents (I'm surprised that's allowed?)

Testing

  • Ran all tests and manually tested.
  • Added to_and_from_u8 to linear_rgba::tests

Changelog

  • visible change is Color::linear becomes Color::to_linear.

@gagnus
Rename Color::linear to Color::to_linear, added Color::to_srgba as th…
99be344 
…e other major option. Added to_u8_array, from_u8_array and _no_alpha versions of them to ColorToComponent, implemented for SRgba and LinearRgba and left unimplemented! for other color types which don't make sense. Removed redundant extra version of to_f32_array in LinearRgba and added use ColorToComponent where that was used in other code, some extra tidy up of usage to use these new functions.
@gagnus
round float before converting to u8 to to_u8_array
38d62ac
@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Jun 7, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jun 7, 2024

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@gagnus gagnus changed the title Adds some limited u8 array conversion to the two RGB color types, also renames Color::linear to Color::to_linear as discussed on discord. Adds back in way to convert color to u8 array, implemented for the two RGB color types, also renames Color::linear to Color::to_linear as discussed on discord. Jun 7, 2024
@gagnus
Copy link
Contributor Author

gagnus commented Jun 7, 2024

Updated with a cargo fmt --all

@gagnus
New trait ColorToPacked for the u8 conversion functions, means all th…
5207210 
…e other color types dont need unimplemented versions.
@alice-i-cecile alice-i-cecile added C-Usability A simple quality-of-life change that makes Bevy easier to use X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples A-Color Color spaces and color math S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 7, 2024
@gagnus gagnus deleted the branch bevyengine:main June 8, 2024 21:08
@gagnus gagnus closed this Jun 8, 2024
@gagnus gagnus deleted the main branch June 8, 2024 21:08
@gagnus gagnus mentioned this pull request Jun 8, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 10, 2024
@gagnus
Adds back in way to convert color to u8 array, implemented for the tw…
298b01f 
…o RGB color types, also renames Color::linear to Color::to_linear. (#13759)

# Objective

One thing missing from the new Color implementation in 0.14 is the
ability to easily convert to a u8 representation of the rgb color.

(note this is a redo of PR #13739
as I needed to move the source branch

## Solution

I have added to_u8_array and to_u8_array_no_alpha to a new trait called
ColorToPacked to mirror the f32 conversions in ColorToComponents and
implemented the new trait for Srgba and LinearRgba.
To go with those I also added matching from_u8... functions and
converted a couple of cases that used ad-hoc implementations of that
conversion to use these.
After discussion on Discord of the experience of using the API I renamed
Color::linear to Color::to_linear, as without that it looks like a
constructor (like Color::rgb).
I also added to_srgba which is the other commonly converted to type of
color (for UI and 2D) to match to_linear.
Removed a redundant extra implementation of to_f32_array for LinearColor
as it is also supplied in ColorToComponents (I'm surprised that's
allowed?)

## Testing

Ran all tests and manually tested.
Added to_and_from_u8 to linear_rgba::tests

## Changelog

visible change is Color::linear becomes Color::to_linear.

---------

Co-authored-by: John Payne <20407779+johngpayne@users.noreply.github.com>
mockersf pushed a commit that referenced this pull request Jun 10, 2024
@gagnus @mockersf
Adds back in way to convert color to u8 array, implemented for the tw…
cbebcb0 
…o RGB color types, also renames Color::linear to Color::to_linear. (#13759)

# Objective

One thing missing from the new Color implementation in 0.14 is the
ability to easily convert to a u8 representation of the rgb color.

(note this is a redo of PR #13739
as I needed to move the source branch

## Solution

I have added to_u8_array and to_u8_array_no_alpha to a new trait called
ColorToPacked to mirror the f32 conversions in ColorToComponents and
implemented the new trait for Srgba and LinearRgba.
To go with those I also added matching from_u8... functions and
converted a couple of cases that used ad-hoc implementations of that
conversion to use these.
After discussion on Discord of the experience of using the API I renamed
Color::linear to Color::to_linear, as without that it looks like a
constructor (like Color::rgb).
I also added to_srgba which is the other commonly converted to type of
color (for UI and 2D) to match to_linear.
Removed a redundant extra implementation of to_f32_array for LinearColor
as it is also supplied in ColorToComponents (I'm surprised that's
allowed?)

## Testing

Ran all tests and manually tested.
Added to_and_from_u8 to linear_rgba::tests

## Changelog

visible change is Color::linear becomes Color::to_linear.

---------

Co-authored-by: John Payne <20407779+johngpayne@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viridia viridia viridia approved these changes

Assignees
No one assigned
Labels
A-Color Color spaces and color math C-Usability A simple quality-of-life change that makes Bevy easier to use D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon
Projects
None yet
Milestone
0.14
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gagnus @viridia @alice-i-cecile