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

StandardMaterial::emissive field should use LinearRgba #13212

Open
alice-i-cecile opened this issue May 3, 2024 · 4 comments · May be fixed by #13352
Open

StandardMaterial::emissive field should use LinearRgba #13212

alice-i-cecile opened this issue May 3, 2024 · 4 comments · May be 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 simple 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 simple 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.

@andristarr andristarr mentioned this issue May 12, 2024
Closed
@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!

@andristarr andristarr linked a pull request May 13, 2024 that will close this issue
Open
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 C-Usability A simple 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
Alice's Work Planning
Status: Active: bevy_color
Milestone
No milestone
2 participants
@alice-i-cecile @andristarr