Skip to content

Add PBR Neutral tone mapping#23761

Merged
alice-i-cecile merged 6 commits intobevyengine:mainfrom
brianchirls:tonemap_pbr_neutral
Apr 13, 2026
Merged

Add PBR Neutral tone mapping#23761
alice-i-cecile merged 6 commits intobevyengine:mainfrom
brianchirls:tonemap_pbr_neutral

Conversation

@brianchirls
Copy link
Copy Markdown
Contributor

Objective

Add a tonemapping option for for e-commerce, architecture and CAD applications.

Solution

Implemented PBR Neutral tone mapping

  • Added to 2d and 3d materials, deferred and forward rendering
  • Added to bloom_2d example
  • Added to tonemapping example

Testing

Run bloom_2d and tonemapping examples

@github-actions
Copy link
Copy Markdown
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide, as well as our policy regarding AI usage, and we look forward to reviewing your pull request shortly ✨

@kfc35 kfc35 added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 11, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Apr 11, 2026
@kfc35 kfc35 added the D-Straightforward Simple bug fixes and API improvements, docs, test and examples label Apr 11, 2026
Comment thread crates/bevy_core_pipeline/src/tonemapping/mod.rs Outdated
@alice-i-cecile
Copy link
Copy Markdown
Member

I think that something like this should exist, and this looks like a solid, standard option.

Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl
Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl
Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl Outdated
Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl Outdated
Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl Outdated
Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl
Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I'm moderately confident that the math is correctly translated here, but we can clean it up to more closely match the actual equations we're referencing.

@brianchirls
Copy link
Copy Markdown
Contributor Author

@alice-i-cecile I'm happy to make these suggested changes. Thanks for the review.

Just to be clear, generally your preference is to match the symbols/names from the equation in the linked readme, not the glsl source nor descriptive names?

@alice-i-cecile
Copy link
Copy Markdown
Member

@alice-i-cecile I'm happy to make these suggested changes. Thanks for the review.

Just to be clear, generally your preference is to match the symbols/names from the equation in the linked readme, not the glsl source nor descriptive names?

My preference is to use descriptive names, ideally with comments mapping them onto the glsl source and equation names.

Be sure to link the GLSL source directly if you're translating it for attribution reasons.

Copy link
Copy Markdown
Contributor

@atlv24 atlv24 left a comment

Choose a reason for hiding this comment

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

1<<8 overflows the current reserved bit count for tonemapping variants, it needs to be increased if you want this to work.

Comment thread crates/bevy_sprite_render/src/render/mod.rs Outdated
Comment thread crates/bevy_sprite_render/src/mesh2d/mesh.rs Outdated
Comment thread crates/bevy_pbr/src/render/mesh.rs Outdated
@brianchirls
Copy link
Copy Markdown
Contributor Author

@alice-i-cecile I'm happy to make these suggested changes. Thanks for the review.
Just to be clear, generally your preference is to match the symbols/names from the equation in the linked readme, not the glsl source nor descriptive names?

My preference is to use descriptive names, ideally with comments mapping them onto the glsl source and equation names.

Be sure to link the GLSL source directly if you're translating it for attribution reasons.

The URL to the GLSL source is already there in the comment, right above the WGSL function.

Comment thread crates/bevy_core_pipeline/src/tonemapping/mod.rs Outdated
brianchirls pushed a commit to brianchirls/bevy that referenced this pull request Apr 13, 2026
- Edit inline documentation, with linked source URL
- Rename and comment shader function variables
- Fix bit mask constants
brianchirls pushed a commit to brianchirls/bevy that referenced this pull request Apr 13, 2026
Brian Chirls added 3 commits April 13, 2026 10:42
Designed to faithfully reproduce base color under neutral lighting, for
e-commerce, architecture and CAD applications.
https://github.com/KhronosGroup/ToneMapping/tree/main/PBR_Neutral

- Added to 2d and 3d materials, deferred and forward rendering
- Added to bloom_2d example
- Added to tonemapping example
- Edit inline documentation, with linked source URL
- Rename and comment shader function variables
- Fix bit mask constants
@brianchirls brianchirls force-pushed the tonemap_pbr_neutral branch from 3c096f8 to d34ed3d Compare April 13, 2026 14:42
@brianchirls
Copy link
Copy Markdown
Contributor Author

I've done my best to pick names and write comments for the various values/variables in the shader code, based on my understanding of this article by the original author. Some of them are artifacts of the arithmetic optimization so aren't in the definitive equations. What I've written is more descriptive than either those equations or the provided GLSL code.

Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl
Copy link
Copy Markdown
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Good, much clearer :) Thanks!

Comment thread crates/bevy_core_pipeline/src/tonemapping/tonemapping_shared.wgsl Outdated
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 13, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 13, 2026
Merged via the queue into bevyengine:main with commit f8bdd2b Apr 13, 2026
38 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Rendering Apr 13, 2026
@brianchirls
Copy link
Copy Markdown
Contributor Author

Woo hoo! Thanks for the quick review!

mate-h pushed a commit to mate-h/bevy that referenced this pull request Apr 14, 2026
# Objective

Add a tonemapping option for for e-commerce, architecture and CAD
applications.

## Solution

Implemented [PBR
Neutral](https://github.com/KhronosGroup/ToneMapping/tree/main/PBR_Neutral)
tone mapping

- Added to 2d and 3d materials, deferred and forward rendering
- Added to bloom_2d example
- Added to tonemapping example

## Testing

Run bloom_2d and tonemapping examples

---------

Co-authored-by: Brian Chirls <brian.chirls@ambr.net>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: IceSentry <IceSentry@users.noreply.github.com>
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 D-Straightforward Simple bug fixes and API improvements, docs, test and 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

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants