bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Shader color conversion

Open lynn-lumen opened this issue 1 year ago • 3 comments

Objective

  • Fixes #12201

Solution

  • Adds color_conversion.wgsl containing methods for converting between all bevy_color color types in shader code.
    • Please note that this shader is contained in the bevy_render crate (not bevy_color), as this would introduce cyclic dependency.
    • All conversion methods are equivelant to their Rust versions in bevy_color.
  • Currently all colors can be converted from and to LinearRgb(a) and to all internediate steps according to the diagram in bevy_color
  • The conversion methods can be imported in any WGSL shader using e.g. #import bevy_render::color_conversion::linear_rgba_to_xyza

Additional information

I am not sure how discoverable WGSL imports are. The examples demonstrate some use cases, but if available WGSL imports are documented somewhere, I have missed that.

lynn-lumen avatar Apr 12 '24 12:04 lynn-lumen

I'm not familiar enough with WGSL to comment on how good the exact implementation is, but is there a way you could add tests for these methods that compare to the Rust implementations in bevy_color? Since bevy_color is already independently implemented and tested against palette, if the conversions here agree there, I'd say it's good to go.

I believe it would be the first instance of such a testing setup, and would involve a decent bit of setup for the boilerplate of a test, so I'm not sure if you'd want to add that to this PR. I think there would be value in such a testing framework for other utility shaders as well, but again I'm not certain.

I like this idea, but I think, this is way beyond the scope of this PR. If it's fine with you, I would look into this and add such tests in a follow up PR

lynn-lumen avatar May 10 '24 12:05 lynn-lumen

I think that's perfectly fine, I agree it's quite a bit of work!

bushrat011899 avatar May 10 '24 12:05 bushrat011899

Can you fix the conflicts?

Also, what's the use case for this? Shouldn't color conversion be done before it ends up on the gpu?

IceSentry avatar Aug 21 '24 20:08 IceSentry