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

move wgsl color operations from bevy_pbr to bevy_render #13209

Merged

Conversation

arcashka
Copy link
Contributor

@arcashka arcashka commented May 3, 2024

Objective

bevy_pbr/utils.wgsl shader file contains mathematical constants and color conversion functions. Both of those should be accessible without enabling bevy_pbr feature. For example, tonemapping can be done in non pbr scenario, and it uses color conversion functions.

Fixes #13207

Solution

  • Move mathematical constants (such as PI, E) from bevy_pbr/src/render/utils.wgsl into bevy_render/src/maths.wgsl
  • Move color conversion functions from bevy_pbr/src/render/utils.wgsl into new file bevy_render/src/color_operations.wgsl

Testing

Ran multiple examples, checked they are working:

  • tonemapping
  • color_grading
  • 3d_scene
  • animated_material
  • deferred_rendering
  • 3d_shapes
  • fog
  • irradiance_volumes
  • meshlet
  • parallax_mapping
  • pbr
  • reflection_probes
  • shadow_biases
  • 2d_gizmos
  • light_gizmos

Changelog

  • Moved mathematical constants (such as PI, E) from bevy_pbr/src/render/utils.wgsl into bevy_render/src/maths.wgsl
  • Moved color conversion functions from bevy_pbr/src/render/utils.wgsl into new file bevy_render/src/color_operations.wgsl

Migration Guide

In user's shader code replace usage of mathematical constants from bevy_pbr::utils to the usage of the same constants from bevy_render::maths.

Copy link
Contributor

github-actions bot commented May 3, 2024

Welcome, new contributor!

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

@arcashka arcashka marked this pull request as ready for review May 3, 2024 17:51
@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-Color Color spaces and color math C-Usability A simple quality-of-life change that makes Bevy easier to use labels May 3, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I agree with this reorganization: these aren't PBR specific.

Copy link
Contributor

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

Improved organization. LGTM.

Copy link
Contributor

@NthTensor NthTensor left a comment

Choose a reason for hiding this comment

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

This may help a bit when the rendering crates get reorganized.

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide labels May 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 4, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 4, 2024
Merged via the queue into bevyengine:main with commit 6027890 May 4, 2024
33 checks passed
@arcashka arcashka deleted the move_color_operations_from_bevy_pbr branch May 4, 2024 10:46
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 C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Code-Quality A section of code that is hard to understand or change C-Usability A simple 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.

Invalid shader import after #13121
4 participants