Skip to content

Fix AmbientLight::affects_lightmapped_meshes not working #20083

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
merged 3 commits into from
Jul 14, 2025

Conversation

marlyx
Copy link
Contributor

@marlyx marlyx commented Jul 11, 2025

Objective

Fix lightmapped materials not respecting the AmbientLight::affects_lightmapped_meshes setting.

NOTE: This only makes the setting work on the forward renderer. Making it work on the deferred renderer would probably require encoding more information in the g-buffer or similar. Please advise if I missed some obvious way to get it working on deferred.

Solution

  • Make ambient light conditionally applied depending on the affects_lightmapped_meshes setting when material mesh i lightmapped
  • Remove what looks to be leftover Lights (wgsl) members: environment_map_smallest_specular_mip_level, environment_map_intensity. (These where not present in the rust equivalent GpuLights and was not used in the wgsl code)

Open Questions

  • Ambient light is also blended into the transmitted light if DIFFUSE_TRANSMISSION is enabled on the material. Should that also be guarded by the same conditional as the indirect contribution?

Testing

Ran a modified version of the lightmaps example, where there is a bright red ambient light added and the small cube do not have a lightmap added (To be able to see ambient light applied to meshes without lightmaps)


Showcase

Main: Lightmap example with bright red ambient, small box have no lightmap

Main

(All meshes get ambient light even when affects_lightmapped_meshes = false)

This PR: Lightmap example with bright red ambient, small box have no lightmap

With fix

(Only the small box get ambient light when affects_lightmapped_meshes = false)

Copy link
Member

@janhohenheim janhohenheim left a comment

Choose a reason for hiding this comment

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

No comment about the dead code removed, but the disabled ambient light looks sound to me :)

@janhohenheim janhohenheim added A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jul 11, 2025
@janhohenheim janhohenheim added this to the 0.17 milestone Jul 11, 2025
@janhohenheim janhohenheim 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 Jul 11, 2025
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jul 14, 2025
Merged via the queue into bevyengine:main with commit b91a233 Jul 14, 2025
41 checks passed
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 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