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

StandardMaterial::emissive field should use LinearRgba #13212

Closed
alice-i-cecile opened this issue May 3, 2024 · 4 comments · Fixed by #13352
Closed

StandardMaterial::emissive field should use LinearRgba #13212

alice-i-cecile opened this issue May 3, 2024 · 4 comments · Fixed by #13352
Labels
A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy X-Contentious There are nontrivial implications that should be thought through

Comments

@alice-i-cecile
Copy link
Member

What problem does this solve or what need does it fill?

All other low-level rendering fields store a LinearRgba: emissive should as well.

This is particularly true here, as it represents a physical representation of light.

Found in #13103 (comment).

What solution would you like?

Change the field type of ColorMaterial and update call-sites.

Additional context

See #12212 for broader context.

@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use X-Contentious There are nontrivial implications that should be thought through labels May 3, 2024
@andristarr
Copy link
Contributor

Based on the discussion linked in additional context: do we want to swap the color field on the struct to be of LinearRgba, do I understand this correctly?

@alice-i-cecile
Copy link
Member Author

Yep, that's exactly correct :) Nice simple change.

@alice-i-cecile alice-i-cecile changed the title ColorMaterial::emissive field should use LinearRgba StandardMaterial::emissive field should use LinearRgba May 13, 2024
@alice-i-cecile
Copy link
Member Author

Sorry @andristarr, I misspoke in the title of this issue. We actually want to update the emissive field on StandardMaterial.

@andristarr
Copy link
Contributor

No worries, I will update the PR!

github-merge-queue bot pushed a commit that referenced this issue May 24, 2024
StandardMaterial stores a LinearRgba instead of a Color for emissive

Fixes #13212
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-Code-Quality A section of code that is hard to understand or change C-Usability A targeted quality-of-life change that makes Bevy easier to use D-Trivial Nice and easy! A great choice to get started with Bevy X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
2 participants