Skip to content

Refactor affects_lightmapped_meshes, introduce AmbientLightFlags#24509

Merged
alice-i-cecile merged 1 commit into
bevyengine:mainfrom
coreh:refactor-ambient-light-flags
Jun 9, 2026
Merged

Refactor affects_lightmapped_meshes, introduce AmbientLightFlags#24509
alice-i-cecile merged 1 commit into
bevyengine:mainfrom
coreh:refactor-ambient-light-flags

Conversation

@coreh

@coreh coreh commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Objective

  • Make ambient light more consistent with other light sources in the code
  • Pave the way for more ambient light boolean flags other than affects_lightmapped_meshes
  • Make it easier to support monochromatic ambient lights as a follow up once Minimum Viable Monochromatic Lights #24428 lands

Solution

  • Add AmbientLightFlags bitflags struct
  • Rename ambient_light_affects_lightmapped_meshes to ambient_light_flags in GpuLights struct and matching WGSL-side Lights struct
  • Added WGSL AMBIENT_LIGHT_FLAGS_AFFECTS_LIGHTMAPPED_MESHES_BIT constant
  • Set up plumbing to both pass and use the new bitfield consistently with other light flags

Testing

  • Did you test these changes? If so, how? Yes, by viewing the lighting and lightmaps examples
  • Are there any parts that need more testing? Not that I know of
  • How can other people (reviewers) test your changes? Is there anything specific they need to know? Manually verify ambient light still works
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test? macOS

@coreh coreh added A-Rendering Drawing game state to the screen C-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 1, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering Jun 1, 2026

@olukowski olukowski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code looks good, and ambient light still works properly in the lighting example on my machine.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, this makes sense as a cleanup to me.

@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 Jun 7, 2026
@coreh

coreh commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

One heads up is that @Elabajaba mentioned that ambient light will be replaced, so this might be short-lived:

https://discord.com/channels/691052431525675048/743663924229963868/1510811921782280282

@alice-i-cecile

Copy link
Copy Markdown
Member

Yep, that's my hope too. AmbientLight is rather a trap currently, and produced a very dated look. Useful for some very specific retro projects, but not something we should be shipping.

@alice-i-cecile

Copy link
Copy Markdown
Member

#18207 is the breadcrumb to the "remove ambient light" work.

@alice-i-cecile
alice-i-cecile added this pull request to the merge queue Jun 9, 2026
Merged via the queue into bevyengine:main with commit c367ca0 Jun 9, 2026
49 checks passed
@github-project-automation github-project-automation Bot moved this from Needs SME Triage to Done in Rendering Jun 9, 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-Code-Quality A section of code that is hard to understand or change D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes 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.

3 participants