Fix clustered child light resync after parent enabled changes#18412
Fix clustered child light resync after parent enabled changes#18412deltakosh merged 2 commits intoBabylonJS:masterfrom
Conversation
Prevent lights owned by ClusteredLightContainer from resyncing themselves back into mesh light source lists when their parent enabled state changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
There was a problem hiding this comment.
Pull request overview
Fixes a clustered-lighting edge case where child point/spot lights could be reintroduced into per-mesh lightSources (and thus bound as regular lights) after a parent enabled-state change, even while still owned by a ClusteredLightContainer.
Changes:
- Adds an internal marker on
Lightto indicate when it is owned by aClusteredLightContainer. - Skips mesh light-source resync for clustered child lights to prevent them from being re-added via parent enabled-state propagation.
- Adds a unit regression test covering parent enabled-state toggling with clustered child lights.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/dev/core/test/unit/Lights/Clustered/clusteredLightContainer.test.ts | Adds regression coverage for parent enabled-state changes not reintroducing clustered child lights to mesh.lightSources. |
| packages/dev/core/src/Lights/light.ts | Introduces _isClusteredLight and uses it to short-circuit _resyncMeshes() for clustered child lights. |
| packages/dev/core/src/Lights/Clustered/clusteredLightContainer.ts | Sets/clears the clustered-ownership marker when adding/removing child lights. |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18412/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18412/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18412/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
Use an owning clustered container reference instead of a boolean, skip excluded mesh resyncs while a light is clustered, and cover duplicate ownership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
🟢 Memory Leak Test Results13 passed, 0 leaked out of 13 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (13)
|
Summary
Fixes a clustered lighting bug reported on the forum: https://forum.babylonjs.com/t/issue-when-using-lights-with-a-mesh-as-a-parent-in-clustered-lighting/63308
When a point or spot light is owned by a
ClusteredLightContainer, it must not also be present in per-meshlightSources, otherwise materials can bind it as a regular light in addition to the clustered container.The previous cleanup handled
addLight(), but a parent enabled-state change triggeredLight._syncParentEnabledState()and_resyncMeshes(), which could re-add the raw child light to meshes even though it was still owned by the clustered container.Fix
ClusteredLightContainer.Validation
npx prettier --check packages/dev/core/src/Lights/Clustered/clusteredLightContainer.ts packages/dev/core/src/Lights/light.ts packages/dev/core/test/unit/Lights/Clustered/clusteredLightContainer.test.tsnpm test -- --run packages/dev/core/test/unit/Lights/Clustered/clusteredLightContainer.test.ts