Skip to content

fix(pcss example): separate light entities and adjust intensity instead of remove components#23802

Open
kfc35 wants to merge 1 commit intobevyengine:mainfrom
kfc35:23769_pcss_restructure_example
Open

fix(pcss example): separate light entities and adjust intensity instead of remove components#23802
kfc35 wants to merge 1 commit intobevyengine:mainfrom
kfc35:23769_pcss_restructure_example

Conversation

@kfc35
Copy link
Copy Markdown
Contributor

@kfc35 kfc35 commented Apr 14, 2026

Objective

  • Fixes Pcss example is broken #23769
  • Fixes just the example and does nothing to address the deeper issue of how SyncComponents is configured for lights (I can make an issue for this if desired -- basically despawning and then respawning a light on the same entity probably has some broken aspect to it as evidenced by this example being broken)

The other PR (#23790) I have open will not fully fix the example once #23713 is merged anyway, since that PR will cause a separate issue — shadow maps will only generated once for spot / point lights, and I’ve run into issues removing and re-adding lights and their associated render components given that constraint.

Solution

  • Separate the single combined light entity that toggles between light components into three separate light entities.
  • Adjust illuminance/intensity of these three light entities instead of removing their respective component, avoiding any mess that results from SyncComponents happening.

Testing

  • You can toggle lights freely now via cargo run --example pcss --features=“experimental_pbr_pcss"

@kfc35 kfc35 added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 14, 2026
@github-project-automation github-project-automation bot moved this to Needs SME Triage in Rendering Apr 14, 2026
@kfc35 kfc35 added the C-Bug An unexpected or incorrect behavior label Apr 14, 2026
@AntoninD-Galadrim
Copy link
Copy Markdown

In my opinion, we should avoid band-aid solutions when they hide a deeper problem that users will face.

@kfc35
Copy link
Copy Markdown
Contributor Author

kfc35 commented Apr 14, 2026

In my opinion, we should avoid band-aid solutions when they hide a deeper problem that users will face.

I agree with this, and I think ideally, the other PR (or something inspired by it) is merged too (#23769). In fact I think I’ll take it out of draft to be seriously considered.

However, I still think this should be considered. #23713 makes me question whether an entity should switch between light sources because of how directional and spot light shadows maps are co-located (IIUC from reading the code in light.rs in that PR). Do we expect to support that use case for users — an entity switching between different light components? I honestly do not know! But, since that mentioned PR is going to be merged, switching an entity between directional and spot lights components breaks the shadows for spotlights after spotlight shadows have already been calculated once. The additional required attribute that is shared between spot and point lights in that PR also seems to complicate things more. I spent quite a few hours yesterday trying to figure it out but couldn’t! So hopefully someone else more rendering savvy can look into the SyncComponents issue, particularly after that PR is merged, if that is truly the way we should fix it rather than also modifying the example.

@kfc35 kfc35 force-pushed the 23769_pcss_restructure_example branch from 4f54bdd to ddf6adc Compare April 14, 2026 17:50
@AntoninD-Galadrim
Copy link
Copy Markdown

AntoninD-Galadrim commented Apr 15, 2026

I think changing light types on the same entities makes sense in an editor context, where you might have a light entity where the user can easily swap between the different light types.

More broadly, from what i've seen, bevy tries to support every user land setup it can, so it is nice to keep examples broken as a kind of test (that's part of the role of examples from my understanding)

@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label Apr 15, 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 C-Examples An addition or correction to our examples D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

Pcss example is broken

3 participants