Improve Bloom 3D lighting by BD103 · Pull Request #11981 · bevyengine/bevy

BD103 · 2024-02-19T16:35:22Z

Objective

With the recent lighting changes, the default configuration in the bloom_3d example is less clear what bloom actually does
- See this screenshot for a comparison.
bloom_3d additionally uses a for-loop to spawn the spheres, which can be turned into commands::spawn_batch call.
The text is black, which is difficult to see on the gray background.

Solution

Increase emmisive values of materials.
Set text to white.

Showcase

Before:

After:

`Additive` is a lot more clear in showing what bloom actually does. See https://github.com/bevyengine/bevy-website/pull/1023/files/4fdb1455d5a3371d69db32b036e38731342b48de#r1494648414 for motivation behind this change.

mockersf · 2024-02-19T16:40:21Z

I would prefer the default settings to be useful rather than tailoring the example

BD103 · 2024-02-19T16:45:55Z

I would prefer the default settings to be useful rather than tailoring the example

BloomSettings's default is BloomSettings::NATURAL. None of the other associated constants (OLD_SCHOOL and SCREEN_BLUR) are really useful in highlighting the effects of bloom. Perhaps a new associated constant could be added with the desired effect?

bevy/crates/bevy_core_pipeline/src/bloom/settings.rs

Lines 148 to 152 in 5f1dd39

    
           impl Default for BloomSettings { 
        
               fn default() -> Self { 
        
                   Self::NATURAL 
        
               } 
        
           }

examples/3d/bloom_3d.rs

alice-i-cecile · 2024-02-22T03:10:11Z

I'm neutral on the spawn_batch change. It is harder to read, but it's also nice to demonstrate how to do nested iterators with spawn_batch. I'd probably revert it: it's not the point of the example.

alice-i-cecile · 2024-02-22T03:11:24Z

I would prefer the default settings to be useful rather than tailoring the example

BloomSettings's default is BloomSettings::NATURAL. None of the other associated constants (OLD_SCHOOL and SCREEN_BLUR) are really useful in highlighting the effects of bloom. Perhaps a new associated constant could be added with the desired effect?

Agreed: if we want to showcase the effect, I would add a more intense associated constant, and then a mechanism to cycle through the intensities (starting with the default).

This reverts commit 5a903b4.

crates/bevy_core_pipeline/src/bloom/settings.rs

Co-authored-by: Alice Cecile <[email protected]>

rparrett · 2024-02-25T13:44:59Z

There have been various reports of emissive materials needing much higher values to achieve the same brightness after 0.13 in general, unrelated to bloom, and the values in this example haven't been changed. I see output similar to the old output if I multiply the color values by 10.

Another option might be to use a different exposure value.

Pinging some bloom people for a look: @JMS55 @StarLederer .

JMS55 · 2024-02-25T21:36:48Z

Yes, emissive values need to be higher due to exposure changes.

I multiplied the values by 10, based off of bevyengine#11981 (comment).

BD103 · 2024-02-26T13:04:05Z

Updated screenshot with higher emission values:

I think additive is a bit too much here, and probably should be reverted to energy conserving. Thoughts?

rparrett · 2024-02-26T13:28:36Z

I think additive is a bit too much here, and probably should be reverted to energy conserving. Thoughts?

My thought was that probably only changing the color values was necessary.

BD103 · 2024-02-26T15:05:01Z

I reverted most changes except the emission bits, since that achieves the same effect.

Marking this as draft until I have the time to update the PR description.

examples/3d/bloom_3d.rs

alice-i-cecile · 2024-03-06T15:51:33Z

@BD103 once you've resolved merge conflicts I'll merge this in :)

BD103 · 2024-03-06T17:28:52Z

@BD103 once you've resolved merge conflicts I'll merge this in :)

Got it, I'll get this done by the end of the day!

BD103 · 2024-03-07T03:21:36Z

Conflicts resolved and tested :D

- With the recent lighting changes, the default configuration in the `bloom_3d` example is less clear what bloom actually does - See [this screenshot](https://github.com/bevyengine/bevy-website/pull/1023/files/4fdb1455d5a3371d69db32b036e38731342b48de#r1494648414) for a comparison. - `bloom_3d` additionally uses a for-loop to spawn the spheres, which can be turned into `commands::spawn_batch` call. - The text is black, which is difficult to see on the gray background. - Increase emmisive values of materials. - Set text to white. Before: <img width="1392" alt="before" src="https://github.com/bevyengine/bevy/assets/59022059/757057ad-ed9f-4eed-b135-8e2032fcdeb5"> After: <img width="1392" alt="image" src="https://github.com/bevyengine/bevy/assets/59022059/3f9dc7a8-94b2-44b9-8ac3-deef1905221b"> --------- Co-authored-by: Alice Cecile <[email protected]>

# Objective - With the recent lighting changes, the default configuration in the `bloom_3d` example is less clear what bloom actually does - See [this screenshot](https://github.com/bevyengine/bevy-website/pull/1023/files/4fdb1455d5a3371d69db32b036e38731342b48de#r1494648414) for a comparison. - `bloom_3d` additionally uses a for-loop to spawn the spheres, which can be turned into `commands::spawn_batch` call. - The text is black, which is difficult to see on the gray background. ## Solution - Increase emmisive values of materials. - Set text to white. ## Showcase Before: <img width="1392" alt="before" src="https://github.com/bevyengine/bevy/assets/59022059/757057ad-ed9f-4eed-b135-8e2032fcdeb5"> After: <img width="1392" alt="image" src="https://github.com/bevyengine/bevy/assets/59022059/3f9dc7a8-94b2-44b9-8ac3-deef1905221b"> --------- Co-authored-by: Alice Cecile <[email protected]>

BD103 added 3 commits February 19, 2024 11:05

refactor: use spawn_batch in bloom_3d

5a903b4

feat: change bloom_3d text from black to white

79a8a30

BD103 added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels Feb 19, 2024

BD103 mentioned this pull request Feb 19, 2024

Update Examples Screenshots bevyengine/bevy-website#1023

Merged

mockersf added this to the 0.13.1 milestone Feb 19, 2024

BD103 mentioned this pull request Feb 21, 2024

Improve lighting in more examples #12021

Merged

alice-i-cecile reviewed Feb 22, 2024

View reviewed changes

examples/3d/bloom_3d.rs Outdated Show resolved Hide resolved

Revert "refactor: use spawn_batch in bloom_3d"

89081bd

This reverts commit 5a903b4.

BD103 changed the title ~~Improve Bloom 3D screenshot and use spawn_batch~~ Improve Bloom 3D lighting Feb 22, 2024

BD103 added 2 commits February 22, 2024 07:59

chore: comment on bloom_3d random number generation

67d6203

feat: natural additive bloom setting

78e0b57

BD103 commented Feb 22, 2024

View reviewed changes

crates/bevy_core_pipeline/src/bloom/settings.rs Outdated Show resolved Hide resolved

BD103 added 2 commits February 22, 2024 08:10

fix: use natural additive setting when toggling bloom back on

768274f

refactor: rustfmt

00f9039

alice-i-cecile reviewed Feb 22, 2024

View reviewed changes

crates/bevy_core_pipeline/src/bloom/settings.rs Outdated Show resolved Hide resolved

fix: small comment changes

6ff15fa

Co-authored-by: Alice Cecile <[email protected]>

alice-i-cecile approved these changes Feb 22, 2024

View reviewed changes

BD103 mentioned this pull request Feb 22, 2024

meta: Add Showcase section to PR template #11750

Merged

feat: increase emissive values in bloom_3d

7839fb6

I multiplied the values by 10, based off of bevyengine#11981 (comment).

Merge branch 'main' into bloom-3d

8ea88b7

BD103 added 2 commits February 26, 2024 10:02

feat!: revert most changes except emission values

1917840

refactor: rustfmt

aaa850c

BD103 marked this pull request as draft February 26, 2024 15:04

BD103 marked this pull request as ready for review February 26, 2024 17:15

rparrett reviewed Feb 27, 2024

View reviewed changes

examples/3d/bloom_3d.rs Show resolved Hide resolved

rparrett approved these changes Mar 6, 2024

View reviewed changes

rparrett 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 6, 2024

Merge branch 'main' into bloom-3d

438489a

alice-i-cecile added this pull request to the merge queue Mar 7, 2024

Merged via the queue into bevyengine:main with commit 713d91b Mar 7, 2024

BD103 deleted the bloom-3d branch March 7, 2024 15:46

Uh oh!

Conversation

BD103 commented Feb 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Solution

Showcase

Uh oh!

mockersf commented Feb 19, 2024

Uh oh!

BD103 commented Feb 19, 2024

Uh oh!

Uh oh!

alice-i-cecile commented Feb 22, 2024

Uh oh!

alice-i-cecile commented Feb 22, 2024

Uh oh!

Uh oh!

Uh oh!

rparrett commented Feb 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JMS55 commented Feb 25, 2024

Uh oh!

BD103 commented Feb 26, 2024

Uh oh!

rparrett commented Feb 26, 2024

Uh oh!

BD103 commented Feb 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

alice-i-cecile commented Mar 6, 2024

Uh oh!

BD103 commented Mar 6, 2024

Uh oh!

BD103 commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

BD103 commented Feb 19, 2024 •

edited

Loading

rparrett commented Feb 25, 2024 •

edited

Loading

BD103 commented Feb 26, 2024 •

edited

Loading

BD103 commented Mar 7, 2024 •

edited

Loading