Skip to content

Compute light attenuation using Beer's law#24516

Open
issam3105 wants to merge 3 commits into
bevyengine:mainfrom
issam3105:Fix_volume_attenuation
Open

Compute light attenuation using Beer's law#24516
issam3105 wants to merge 3 commits into
bevyengine:mainfrom
issam3105:Fix_volume_attenuation

Conversation

@issam3105
Copy link
Copy Markdown
Contributor

@issam3105 issam3105 commented Jun 2, 2026

Objective

Fix volume attenuation for glTF materials using KHR_materials_volume.

Bevy was converting attenuationColor with pow(1.0 - color, E) to reuse the existing fog math atmospheric_fog() , which under-attenuates transmitted light. This makes dark volume materials render too bright.

I first tried to keep the existing structure and only fix the coefficient passed to atmospheric_fog() (first commit). Since that function attenuates with:

transmittance = exp(-be * distance)

and KHR_materials_volume defines attenuationColor as the transmittance after attenuationDistance, solving:

attenuationColor = exp(-be * attenuationDistance)

gives:

be = -log(attenuationColor) / attenuationDistance

After discussing this with @coreh, I switched to applying the volume attenuation directly instead of routing it through the fog abstraction.

Solution

transmittance = attenuationColor ^ (transmissionDistance / attenuationDistance)
radiance *= transmittance
  • thicknessTexture is now loaded as a linear data texture, since its G channel stores thickness rather than color.

Testing

Reviewers can test visually by loading the Khronos AttenuationTest and DragonAttenuation assets

Showcase

Before PR

image

After
image

Ref Khronos

image

Before

image

After

image

Ref Khronos

image

@coreh
Copy link
Copy Markdown
Contributor

coreh commented Jun 2, 2026

Hmm... I don't remember it exactly, but I suspect I might have wanted to reuse the existing fog math instead of implementing it separately, as the original transmission PR was already quite large

This seems more correct and if it matches the reference renderings better, I'm all for it

Edit: Actually I misread your explanation, looking at the code it's still reusing the fog math.

I'm not sure why I went with this one instead. Might have been a mistake or misunderstanding of some paper on my part?

@issam3105
Copy link
Copy Markdown
Contributor Author

issam3105 commented Jun 2, 2026

Actually I misread your explanation, looking at the code it's still reusing the fog math.

I'm not sure why I went with this one instead. Might have been a mistake or misunderstanding of some paper on my part?

Thank you for checking.

I first tried to keep the existing approach by only adapting the coefficient used with atmospheric_fog(), but I agree that tying glTF volume attenuation to the fog abstraction makes the intent less clear.

I pushed an update that applies the Khronos Sample Renderer formula directly in the transmission path. This avoids relying on fog internals and makes the code closer to the KHR_materials_volume reference implementation.

@issam3105 issam3105 force-pushed the Fix_volume_attenuation branch from e606dd6 to b2ad6f1 Compare June 3, 2026 05:22
@issam3105 issam3105 force-pushed the Fix_volume_attenuation branch from b2ad6f1 to 40b7d44 Compare June 3, 2026 06:14
@issam3105 issam3105 changed the title Fix volume attenuation Compute light attenuation using Beer's law Jun 3, 2026
@issam3105 issam3105 marked this pull request as ready for review June 3, 2026 08:23
@alice-i-cecile alice-i-cecile requested review from coreh and ecoskey June 3, 2026 16:03
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 3, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Jun 3, 2026
@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels Jun 3, 2026
Copy link
Copy Markdown

@mardzie mardzie left a comment

Choose a reason for hiding this comment

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

The Rust code looks fine. I am not familiar with WGSL and the math, someone with that expertise should also review.

Copy link
Copy Markdown
Contributor

@ecoskey ecoskey left a comment

Choose a reason for hiding this comment

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

Wgsl/math looks good! Always good to catch little errors like these in our volumetrics math.

Just for context, can you explain the addition to get_linear_textures? Were we not keeping track of the thickness texture at all, or just not as a linear texture?

@ecoskey ecoskey requested a review from mate-h June 4, 2026 19:23
@issam3105
Copy link
Copy Markdown
Contributor Author

Just for context, can you explain the addition to get_linear_textures? Were we not keeping track of the thickness texture at all, or just not as a linear texture?

We were keeping track of it, but not as a linear texture. Before this fix, I noticed that the Thickness Texture row in Khronos' AttenuationTest sample was lighter than the other rows:
image

Copy link
Copy Markdown
Contributor

@coreh coreh left a comment

Choose a reason for hiding this comment

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

LGTM

@ecoskey ecoskey 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 Jun 5, 2026
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-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 X-Uncontroversial This work is generally agreed upon

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

5 participants