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

bevy_color: migrate color conversion code from bevy_render #12068

Closed
viridia opened this issue Feb 23, 2024 · 2 comments · Fixed by #12105
Closed

bevy_color: migrate color conversion code from bevy_render #12068

viridia opened this issue Feb 23, 2024 · 2 comments · Fixed by #12105
Labels
A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change
Milestone
0.14

Comments

@viridia
Copy link
Contributor

viridia commented Feb 23, 2024

Currently, bevy_color uses color conversion functions contained in bevy_render. This means that bevy_color depends on bevy_render. This dependency should be the other way around - bevy_color should be a lower-level dependency of bevy_render.

To fix this, a number of modules will need to be moved:

  • The colorspace module in bevy_render should be moved to bevy_color.
  • The From impls that convert between the old legacy color type to the new color types should be moved to bevy_render.
@viridia viridia added C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Feb 23, 2024
@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change and removed C-Enhancement A new feature S-Needs-Triage This issue needs to be labelled labels Feb 24, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Feb 24, 2024
@bushrat011899 bushrat011899 mentioned this issue Feb 25, 2024
Merged
@CptPotato
Copy link
Contributor

Side question: With this change, is bevy_render's dependency on bevy_color even needed, or could it simply use wgpu::Color?

@viridia
Copy link
Contributor Author

viridia commented Feb 25, 2024

An example where bevy_render uses color is ClearColorConfig. This needs to be a type which can easily be converted from a color via .into().

Also, it looks like wgpu::Color uses f64, which seems like overkill for Bevy rendering pipelines.

github-merge-queue bot pushed a commit that referenced this issue Feb 25, 2024
@bushrat011899
Made bevy_color a dependency of bevy_render (#12105)
5e63f68 
# Objective

- Fixes #12068

## Solution

- Split `bevy_render::color::colorspace` across the various space
implementations in `bevy_color` as appropriate.
- Moved `From` implementations involving
`bevy_render::color::LegacyColor` into `bevy_render::color`

## Migration Guide

###
`bevy_render::color::colorspace::SrgbColorSpace::<f32>::linear_to_nonlinear_srgb`

Use `bevy_color::color::gamma_function_inverse`

###
`bevy_render::color::colorspace::SrgbColorSpace::<f32>::nonlinear_to_linear_srgb`

Use `bevy_color::color::gamma_function`

###
`bevy_render::color::colorspace::SrgbColorSpace::<u8>::linear_to_nonlinear_srgb`

Modify the `u8` value to instead be an `f32` (`|x| x as f32 / 255.`),
use `bevy_color::color::gamma_function_inverse`, and back again.

###
`bevy_render::color::colorspace::SrgbColorSpace::<u8>::nonlinear_to_linear_srgb`

Modify the `u8` value to instead be an `f32` (`|x| x as f32 / 255.`),
use `bevy_color::color::gamma_function`, and back again.

###
`bevy_render::color::colorspace::HslRepresentation::hsl_to_nonlinear_srgb`

Use `Hsla`'s implementation of `Into<Srgba>`

###
`bevy_render::color::colorspace::HslRepresentation::nonlinear_srgb_to_hsl`

Use `Srgba`'s implementation of `Into<Hsla>`

###
`bevy_render::color::colorspace::LchRepresentation::lch_to_nonlinear_srgb`

Use `Lcha`'s implementation of `Into<Srgba>`

###
`bevy_render::color::colorspace::LchRepresentation::nonlinear_srgb_to_lch`

Use `Srgba`'s implementation of `Into<Lcha>`
msvbg pushed a commit to msvbg/bevy that referenced this issue Feb 26, 2024
@bushrat011899 @msvbg
Made bevy_color a dependency of bevy_render (bevyengine#12105)
b234083 
# Objective

- Fixes bevyengine#12068

## Solution

- Split `bevy_render::color::colorspace` across the various space
implementations in `bevy_color` as appropriate.
- Moved `From` implementations involving
`bevy_render::color::LegacyColor` into `bevy_render::color`

## Migration Guide

###
`bevy_render::color::colorspace::SrgbColorSpace::<f32>::linear_to_nonlinear_srgb`

Use `bevy_color::color::gamma_function_inverse`

###
`bevy_render::color::colorspace::SrgbColorSpace::<f32>::nonlinear_to_linear_srgb`

Use `bevy_color::color::gamma_function`

###
`bevy_render::color::colorspace::SrgbColorSpace::<u8>::linear_to_nonlinear_srgb`

Modify the `u8` value to instead be an `f32` (`|x| x as f32 / 255.`),
use `bevy_color::color::gamma_function_inverse`, and back again.

###
`bevy_render::color::colorspace::SrgbColorSpace::<u8>::nonlinear_to_linear_srgb`

Modify the `u8` value to instead be an `f32` (`|x| x as f32 / 255.`),
use `bevy_color::color::gamma_function`, and back again.

###
`bevy_render::color::colorspace::HslRepresentation::hsl_to_nonlinear_srgb`

Use `Hsla`'s implementation of `Into<Srgba>`

###
`bevy_render::color::colorspace::HslRepresentation::nonlinear_srgb_to_hsl`

Use `Srgba`'s implementation of `Into<Hsla>`

###
`bevy_render::color::colorspace::LchRepresentation::lch_to_nonlinear_srgb`

Use `Lcha`'s implementation of `Into<Srgba>`

###
`bevy_render::color::colorspace::LchRepresentation::nonlinear_srgb_to_lch`

Use `Srgba`'s implementation of `Into<Lcha>`
msvbg pushed a commit to msvbg/bevy that referenced this issue Feb 26, 2024
@bushrat011899 @msvbg
Made bevy_color a dependency of bevy_render (bevyengine#12105)
62bf747 
# Objective

- Fixes bevyengine#12068

## Solution

- Split `bevy_render::color::colorspace` across the various space
implementations in `bevy_color` as appropriate.
- Moved `From` implementations involving
`bevy_render::color::LegacyColor` into `bevy_render::color`

## Migration Guide

###
`bevy_render::color::colorspace::SrgbColorSpace::<f32>::linear_to_nonlinear_srgb`

Use `bevy_color::color::gamma_function_inverse`

###
`bevy_render::color::colorspace::SrgbColorSpace::<f32>::nonlinear_to_linear_srgb`

Use `bevy_color::color::gamma_function`

###
`bevy_render::color::colorspace::SrgbColorSpace::<u8>::linear_to_nonlinear_srgb`

Modify the `u8` value to instead be an `f32` (`|x| x as f32 / 255.`),
use `bevy_color::color::gamma_function_inverse`, and back again.

###
`bevy_render::color::colorspace::SrgbColorSpace::<u8>::nonlinear_to_linear_srgb`

Modify the `u8` value to instead be an `f32` (`|x| x as f32 / 255.`),
use `bevy_color::color::gamma_function`, and back again.

###
`bevy_render::color::colorspace::HslRepresentation::hsl_to_nonlinear_srgb`

Use `Hsla`'s implementation of `Into<Srgba>`

###
`bevy_render::color::colorspace::HslRepresentation::nonlinear_srgb_to_hsl`

Use `Srgba`'s implementation of `Into<Hsla>`

###
`bevy_render::color::colorspace::LchRepresentation::lch_to_nonlinear_srgb`

Use `Lcha`'s implementation of `Into<Srgba>`

###
`bevy_render::color::colorspace::LchRepresentation::nonlinear_srgb_to_lch`

Use `Srgba`'s implementation of `Into<Lcha>`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Milestone
0.14
Development

Successfully merging a pull request may close this issue.

Made bevy_color a dependency of bevy_render bushrat011899/bevy
3 participants
@viridia @alice-i-cecile @CptPotato