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

Ignore emissive_light when multiplying with exposure #13337

Closed
wants to merge 1 commit into from

Conversation

geckoxx
Copy link
Contributor

@geckoxx geckoxx commented May 12, 2024

Objective

Solution

  • This solution just ignores emissive_light when multiplying with exposure

Testing

emissive_fix

@JMS55 JMS55 added this to the 0.14 milestone May 12, 2024
@JMS55 JMS55 added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels May 12, 2024
@alice-i-cecile alice-i-cecile added D-Trivial Nice and easy! A great choice to get started with Bevy S-Needs-Testing Testing must be done before this is safe to merge S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 12, 2024
@DGriffin91
Copy link
Contributor

I'm not sure yet what the appropriate implementation is, but this one would mean that as the camera exposure is adjusted, the adjustment is not reflected in emissive materials, which I believe would be undesirable.

@geckoxx
Copy link
Contributor Author

geckoxx commented May 13, 2024

The Filament document states: "Because the EV scale is almost perceptually linear, the exposure value is also often used as a light unit. This means we could let artists specify the intensity of lights or emissive surfaces using exposure compensation as a unit. The intensity of emitted light would therefore be relative to the exposure settings. Using exposure compensation as a light unit should be avoided whenever possible but can be useful to force (or cancel) a bloom effect around emissive surfaces independently of the camera settings (for instance, a lightsaber in a game should always bloom)."

The exposure gets almost completely cancelled out with the emissive exposure compensation: https://google.github.io/filament/Filament.md.html#listing_fragmentemissive

So in my opinion this fix is better than the current calculation.
A better solution would be to calculate it like filament by adding the exposure compensation to the camera or to the material.

@geckoxx
Copy link
Contributor Author

geckoxx commented May 13, 2024

Closed in favour of #13350

@geckoxx geckoxx closed this May 13, 2024
@geckoxx geckoxx deleted the fix_emissive_light branch May 14, 2024 12:18
github-merge-queue bot pushed a commit that referenced this pull request May 17, 2024
# Objective

- The emissive color gets multiplied by the camera exposure value. But
this cancels out almost any emissive effect.
- Fixes #13133
- Closes PR #13337 

## Solution
- Add emissive_exposure_weight to the StandardMaterial
- In the shader this value is stored in the alpha channel of the
emissive color.
- This value defines how much the exposure influences the emissive
color.
- It's equal to Google's Filament:
https://google.github.io/filament/Materials.html#emissive

https://github.com/google/filament/blob/4f021583f1c721486baaa9291be5943216c244ec/shaders/src/shading_lit.fs#L287

## Testing

- The result of
[EmissiveStrengthTest](https://github.com/KhronosGroup/glTF-Sample-Models/tree/main/2.0/EmissiveStrengthTest)
with the default value of 0.0:

without bloom:

![emissive_fix](https://github.com/bevyengine/bevy/assets/688816/8f8c131a-464a-4d7b-a9e4-4e28d679ee5d)

with bloom:

![emissive_fix_bloom](https://github.com/bevyengine/bevy/assets/688816/89f200ee-3bd5-4daa-bf64-8999b56df3fa)
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-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Needs-Testing Testing must be done before this is safe to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emissive materials not displayed properly when imported from gltf files
4 participants