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

Fix dim emissive values in lighting example #12343

Merged
merged 1 commit into from Mar 7, 2024
Merged

Fix dim emissive values in lighting example #12343

merged 1 commit into from Mar 7, 2024

Conversation

andristarr
Copy link
Contributor

Objective

Solution

  • Increasing the emissive of the objects representing the lights.

@andristarr andristarr changed the title Increaing lighting ex emissive Increasing lighting ex emissive Mar 6, 2024
@TrialDragon TrialDragon added C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples labels Mar 6, 2024
@rparrett
Copy link
Contributor

rparrett commented Mar 7, 2024 •
edited

@BD103 do the values in this PR look good? They are a bit dimmer than what I came up with (~700) based the output of 0.12.1 but I don't know what we should prefer, artistically.

BD103
BD103 suggested changes Mar 7, 2024
View reviewed changes
Copy link
Member

@BD103 BD103 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 @rparrett, I think the emission needs to be increased a little bit more.

Could you also remove the unused bevy::pbr::light_consts import?

bevy/examples/3d/lighting.rs

Line 8 in b26f20a

pbr::{light_consts, CascadeShadowConfigBuilder},

@andristarr andristarr force-pushed the lighting-example-emissive-incr branch from b26f20a to 6c26980 Compare March 7, 2024 18:41
@andristarr
Copy link
Contributor Author

Done! Perfectly increased by a magnitude of 100x.

rparrett
rparrett approved these changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@rparrett rparrett left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks!

Could you (or a maintainer) adjust the PR title to something like

Fix dim emissive values in `lighting` example

?

The PR title is what ends up in the commit log / changelog, so it's good if it looks nice and pretty.

@andristarr andristarr changed the title Increasing lighting ex emissive Fix dim emissive values in lighting example Mar 7, 2024
@andristarr
Copy link
Contributor Author

Done!

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 7, 2024
@mockersf mockersf added this pull request to the merge queue Mar 7, 2024
Merged via the queue into bevyengine:main with commit 432a4f1 Mar 7, 2024
26 checks passed
spectria-limina pushed a commit to spectria-limina/bevy that referenced this pull request Mar 9, 2024
@andristarr @spectria-limina
Fix dim emissive values in lighting example (bevyengine#12343)
89816ae 
# Objective

- Fixes bevyengine#12330 

## Solution

- Increasing the emissive of the objects representing the lights.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BD103 BD103 BD103 requested changes

@rparrett rparrett rparrett approved these changes

@mockersf mockersf mockersf approved these changes

Assignees
No one assigned
Labels
C-Bug An unexpected or incorrect behavior C-Examples An addition or correction to our examples 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lighting example emissive values need to be adjusted
5 participants
@andristarr @rparrett @mockersf @BD103 @TrialDragon