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

[Merged by Bors] - Bloom #6397

Closed
wants to merge 33 commits into from
Closed

[Merged by Bors] - Bloom #6397

wants to merge 33 commits into from

Conversation

JMS55
Copy link
Contributor

@JMS55 JMS55 commented Oct 28, 2022

Objective

image

Solution

  • A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
  • Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

Changelog

  • Added a core_pipeline::bloom::BloomSettings component.
  • Added BloomNode that runs between the main pass and tonemapping.
  • Added a BloomPlugin that is loaded as part of CorePipelinePlugin.
  • Added a bloom example project.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Oct 28, 2022
Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

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

First review pass looked good, but I didn't have time to go really deep.

crates/bevy_pbr/src/bloom/bloom.wgsl Outdated Show resolved Hide resolved
crates/bevy_pbr/src/bloom/bloom.wgsl Outdated Show resolved Hide resolved
crates/bevy_pbr/src/bloom/bloom.wgsl Outdated Show resolved Hide resolved
crates/bevy_pbr/src/bloom/mod.rs Outdated Show resolved Hide resolved
@JMS55
Copy link
Contributor Author

JMS55 commented Oct 28, 2022

Should we add the BloomPlugin by default as part of PbrPlugin btw? It's something I carried over from the previous PR, unsure how I feel about it.

@cart
Copy link
Member

cart commented Oct 28, 2022

I think the bloom plugin should be added by default, but disabled by default in the bloom settings (edit: or rather ... cameras should not have bloom enabled by default). The functionality should be compiled in, but it should have zero cost when disabled.

@JMS55
Copy link
Contributor Author

JMS55 commented Oct 28, 2022

Ok, that's kinda what we have now. Note that it won't be entirely 0 cost, as there's still the plugin registration and pipeline creation at startup, and probably the node running (and then immediately returning).

Something else I've realized: I'm not sure I'm handling viewport sizes correctly. I need to test this on one of the examples that have different viewport vs window/texture sizes.

@cart cart added this to the Bevy 0.9 milestone Nov 2, 2022
@cart cart marked this pull request as ready for review November 2, 2022 20:51
Copy link
Contributor

@jakobhellermann jakobhellermann left a comment

Choose a reason for hiding this comment

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

Haven't reviewed the render code but it's great to see this come together. Left a few comments, otherwise this looks good superficially.

crates/bevy_pbr/src/bloom/bloom.wgsl Outdated Show resolved Hide resolved
crates/bevy_pbr/src/bloom/mod.rs Outdated Show resolved Hide resolved
examples/3d/bloom.rs Outdated Show resolved Hide resolved
Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

Once the build is fixed I think this is good to go.

crates/bevy_core_pipeline/src/bloom/mod.rs Show resolved Hide resolved
@cart
Copy link
Member

cart commented Nov 3, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 3, 2022
# Objective

- Adds a bloom pass for HDR-enabled Camera3ds.
- Supersedes (and all credit due to!) #3430 and #2876

![image](https://user-images.githubusercontent.com/47158642/198698783-228edc00-20b5-4218-a613-331ccd474f38.png)

## Solution

- A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
- Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

---

## Changelog

- Added a `core_pipeline::bloom::BloomSettings` component.
- Added `BloomNode` that runs between the main pass and tonemapping.
- Added a `BloomPlugin` that is loaded as part of CorePipelinePlugin.
- Added a bloom example project.

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: DGriffin91 <github@dgdigital.net>
Cargo.toml Show resolved Hide resolved
@bors
Copy link
Contributor

bors bot commented Nov 3, 2022

Canceled.

@cart
Copy link
Member

cart commented Nov 3, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 3, 2022
# Objective

- Adds a bloom pass for HDR-enabled Camera3ds.
- Supersedes (and all credit due to!) #3430 and #2876

![image](https://user-images.githubusercontent.com/47158642/198698783-228edc00-20b5-4218-a613-331ccd474f38.png)

## Solution

- A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
- Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

---

## Changelog

- Added a `core_pipeline::bloom::BloomSettings` component.
- Added `BloomNode` that runs between the main pass and tonemapping.
- Added a `BloomPlugin` that is loaded as part of CorePipelinePlugin.
- Added a bloom example project.

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: DGriffin91 <github@dgdigital.net>
@cart
Copy link
Member

cart commented Nov 3, 2022

bors r-

@bors
Copy link
Contributor

bors bot commented Nov 3, 2022

Canceled.

@cart
Copy link
Member

cart commented Nov 3, 2022

Just realized we can't enable the bloom example on WASM because it multisamples by default (and hdr multisampling isn't supported on wasm)

Cargo.toml Outdated Show resolved Hide resolved
@cart
Copy link
Member

cart commented Nov 3, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 3, 2022
# Objective

- Adds a bloom pass for HDR-enabled Camera3ds.
- Supersedes (and all credit due to!) #3430 and #2876

![image](https://user-images.githubusercontent.com/47158642/198698783-228edc00-20b5-4218-a613-331ccd474f38.png)

## Solution

- A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
- Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

---

## Changelog

- Added a `core_pipeline::bloom::BloomSettings` component.
- Added `BloomNode` that runs between the main pass and tonemapping.
- Added a `BloomPlugin` that is loaded as part of CorePipelinePlugin.
- Added a bloom example project.

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: DGriffin91 <github@dgdigital.net>
@bors
Copy link
Contributor

bors bot commented Nov 3, 2022

Build failed:

Cargo.toml Show resolved Hide resolved
Co-authored-by: François <mockersf@gmail.com>
@mockersf
Copy link
Member

mockersf commented Nov 4, 2022

bors retry

bors bot pushed a commit that referenced this pull request Nov 4, 2022
# Objective

- Adds a bloom pass for HDR-enabled Camera3ds.
- Supersedes (and all credit due to!) #3430 and #2876

![image](https://user-images.githubusercontent.com/47158642/198698783-228edc00-20b5-4218-a613-331ccd474f38.png)

## Solution

- A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
- Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

---

## Changelog

- Added a `core_pipeline::bloom::BloomSettings` component.
- Added `BloomNode` that runs between the main pass and tonemapping.
- Added a `BloomPlugin` that is loaded as part of CorePipelinePlugin.
- Added a bloom example project.

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: DGriffin91 <github@dgdigital.net>
@bors
Copy link
Contributor

bors bot commented Nov 4, 2022

Canceled.

@mockersf
Copy link
Member

mockersf commented Nov 4, 2022

bors r+

bors bot pushed a commit that referenced this pull request Nov 4, 2022
# Objective

- Adds a bloom pass for HDR-enabled Camera3ds.
- Supersedes (and all credit due to!) #3430 and #2876

![image](https://user-images.githubusercontent.com/47158642/198698783-228edc00-20b5-4218-a613-331ccd474f38.png)

## Solution

- A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
- Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

---

## Changelog

- Added a `core_pipeline::bloom::BloomSettings` component.
- Added `BloomNode` that runs between the main pass and tonemapping.
- Added a `BloomPlugin` that is loaded as part of CorePipelinePlugin.
- Added a bloom example project.

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: DGriffin91 <github@dgdigital.net>
@bors bors bot changed the title Bloom [Merged by Bors] - Bloom Nov 4, 2022
@bors bors bot closed this Nov 4, 2022
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Adds a bloom pass for HDR-enabled Camera3ds.
- Supersedes (and all credit due to!) bevyengine#3430 and bevyengine#2876

![image](https://user-images.githubusercontent.com/47158642/198698783-228edc00-20b5-4218-a613-331ccd474f38.png)

## Solution

- A threshold is applied to isolate emissive samples, and then a series of downscale and upscaling passes are applied and composited together.
- Bloom is applied to 2d or 3d Cameras with hdr: true and a BloomSettings component.

---

## Changelog

- Added a `core_pipeline::bloom::BloomSettings` component.
- Added `BloomNode` that runs between the main pass and tonemapping.
- Added a `BloomPlugin` that is loaded as part of CorePipelinePlugin.
- Added a bloom example project.

Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Co-authored-by: DGriffin91 <github@dgdigital.net>
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-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants