Fix material readiness to gate on light texture readiness by bghgary · Pull Request #18255 · BabylonJS/Babylon.js

bghgary · 2026-04-08T18:32:45Z

Problem

SpotLight projection textures and IES profile textures were not gated by material readiness checks. When a projection texture was still loading, prepareLightSpecificDefines would set PROJECTEDLIGHTTEXTURE to false, the shader would compile without the projection effect, and the material would report isReady() = true.

This caused scene.executeWhenReady() to fire before the projection texture loaded, producing incorrect rendering (e.g. intermittent visual test failures in BabylonNative).

Root Cause

SpotLight.prepareLightSpecificDefines() treats unloaded projection textures as 'not present' rather than 'not ready'
No readiness gate existed for light texture resources in the material pipeline
scene.isReady() -> mesh.isReady() -> material.isReadyForSubMesh() all returned true despite the projection texture still loading

Fix

Light base class: Added areLightTexturesReady() virtual method (returns true by default)
SpotLight: Overrides to check _projectionTexture and _iesProfileTexture readiness
PrepareDefinesForLight: Tracks lightTexturesReady in the state object
PrepareDefinesForLights: Propagates to defines._areLightTexturesReady
All materials: Gate isReadyForSubMesh() on defines._areLightTexturesReady
- StandardMaterial, PBRBaseMaterial, BackgroundMaterial, OpenPBRMaterial, NodeMaterial
- Node Material light blocks (LightBlock, PBRMetallicRoughnessBlock)

When the projection texture finishes loading, the existing _markMeshesAsLightDirty() callback (already wired in the projectionTexture setter) triggers re-evaluation, the define flips to true, the shader recompiles with the projection effect, and the material becomes ready.

SpotLight projection textures and IES profile textures were not gated by material readiness checks. When a projection texture was still loading, prepareLightSpecificDefines would set PROJECTEDLIGHTTEXTURE to false, the shader would compile without the projection effect, and the material would report isReady()=true. This caused scene.executeWhenReady() to fire before the projection texture loaded, producing incorrect rendering. Changes: - Add areLightTexturesReady() virtual method to Light base class - Override in SpotLight to check projection and IES textures - Track light texture readiness in PrepareDefinesForLight state - Propagate via defines._areLightTexturesReady in PrepareDefinesForLights - Gate isReadyForSubMesh() on light texture readiness in all materials: StandardMaterial, PBRBaseMaterial, BackgroundMaterial, OpenPBRMaterial, NodeMaterial, and Node Material light blocks Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR fixes a readiness gap where SpotLight projection/IES textures could still be loading while materials (and therefore scene.executeWhenReady()) reported ready, causing rendering to proceed with shaders compiled without the projected/IES lighting path.

Changes:

Add Light.areLightTexturesReady() (default true) and override it in SpotLight to gate on projection/IES texture readiness.
Propagate a lightTexturesReady flag through PrepareDefinesForLight(s) into defines._areLightTexturesReady.
Update core materials (Standard, PBR, OpenPBR, Background, NodeMaterial) to return not-ready when defines._areLightTexturesReady === false.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
packages/dev/core/src/Materials/standardMaterial.ts	Gates `isReadyForSubMesh` on `defines._areLightTexturesReady`.
packages/dev/core/src/Materials/PBR/pbrBaseMaterial.ts	Gates PBR readiness on `defines._areLightTexturesReady`.
packages/dev/core/src/Materials/PBR/openpbrMaterial.ts	Gates OpenPBR readiness on `defines._areLightTexturesReady`.
packages/dev/core/src/Materials/Node/nodeMaterial.ts	Gates NodeMaterial readiness on `defines._areLightTexturesReady`.
packages/dev/core/src/Materials/Node/Blocks/PBR/pbrMetallicRoughnessBlock.ts	Tracks per-light `lightTexturesReady` via `PrepareDefinesForLight`.
packages/dev/core/src/Materials/Node/Blocks/Dual/lightBlock.ts	Tracks per-light `lightTexturesReady` via `PrepareDefinesForLight`.
packages/dev/core/src/Materials/materialHelper.ts	Extends the typed `PrepareDefinesForLight` state contract to include `lightTexturesReady`.
packages/dev/core/src/Materials/materialHelper.functions.ts	Implements readiness propagation (`state.lightTexturesReady` → `defines._areLightTexturesReady`) and checks `light.areLightTexturesReady()`.
packages/dev/core/src/Materials/Background/backgroundMaterial.ts	Gates BackgroundMaterial readiness on `defines._areLightTexturesReady`.
packages/dev/core/src/Lights/spotLight.ts	Implements `areLightTexturesReady()` by checking projection + IES textures.
packages/dev/core/src/Lights/light.ts	Adds base `areLightTexturesReady()` API for light subclasses.

Copilot · 2026-04-08T18:37:28Z

packages/dev/core/src/Materials/Node/Blocks/Dual/lightBlock.ts

                defines.rebuild();
            }
+
+            defines._areLightTexturesReady = state.lightTexturesReady;


In the single-light path, this assignment overwrites defines._areLightTexturesReady each time prepareDefines runs. If multiple LightBlocks are used (each bound to a different light), a later ready light can flip the flag back to true, masking an earlier not-ready light texture. Accumulate the readiness across blocks (eg keep it false once any light reports not ready).

Suggested change

defines._areLightTexturesReady = state.lightTexturesReady;

defines._areLightTexturesReady = defines._areLightTexturesReady !== false && state.lightTexturesReady;

Copilot · 2026-04-08T18:37:29Z

packages/dev/core/src/Materials/Node/Blocks/PBR/pbrMetallicRoughnessBlock.ts

                defines.rebuild();
            }
+
+            defines._areLightTexturesReady = state.lightTexturesReady;


Same overwrite issue as LightBlock: defines._areLightTexturesReady is set from a per-light state, so with multiple instances/blocks the last evaluated light wins. This can incorrectly report the material as ready while some light textures are still loading. Consider AND-ing with the existing flag (defaulting to true) so readiness is aggregated across all light blocks.

Suggested change

defines._areLightTexturesReady = state.lightTexturesReady;

defines._areLightTexturesReady = (defines._areLightTexturesReady ?? true) && state.lightTexturesReady;

Copilot · 2026-04-08T18:37:29Z

packages/dev/core/src/Lights/spotLight.ts

+        if (this._projectionTexture && !this._projectionTexture.isReady()) {
+            return false;
+        }
+        if (this._iesProfileTexture && !this._iesProfileTexture.isReady()) {


This readiness check uses isReady(), which ignores BaseTexture.isBlocking semantics used elsewhere in material readiness (via isReadyOrNotBlocking()). To avoid unexpectedly blocking scene.executeWhenReady() for explicitly non-blocking projection/IES textures, consider using isReadyOrNotBlocking() here (or otherwise documenting/justifying why light textures should always be blocking).

Suggested change

if (this._projectionTexture && !this._projectionTexture.isReady()) {

return false;

}

if (this._iesProfileTexture && !this._iesProfileTexture.isReady()) {

if (this._projectionTexture && !this._projectionTexture.isReadyOrNotBlocking()) {

return false;

}

if (this._iesProfileTexture && !this._iesProfileTexture.isReadyOrNotBlocking()) {

Copilot · 2026-04-08T18:37:29Z

packages/dev/core/src/Materials/PBR/pbrBaseMaterial.ts

        const effect = this._prepareEffect(mesh, subMesh.getRenderingMesh(), defines, this.onCompiled, this.onError, useInstances, null);

+        if (defines._areLightTexturesReady === false) {
+            return false;
+        }
+


The _areLightTexturesReady gate happens after _prepareEffect(), which can compile/cache an effect with projected/IES defines disabled while textures are still loading. Consider checking _areLightTexturesReady before calling _prepareEffect() to avoid unnecessary compilation and caching of an effect that will never be used once the textures become ready.

Suggested change

const effect = this._prepareEffect(mesh, subMesh.getRenderingMesh(), defines, this.onCompiled, this.onError, useInstances, null);

if (defines._areLightTexturesReady === false) {

return false;

}

if (defines._areLightTexturesReady === false) {

return false;

}

const effect = this._prepareEffect(mesh, subMesh.getRenderingMesh(), defines, this.onCompiled, this.onError, useInstances, null);

Copilot · 2026-04-08T18:37:30Z

packages/dev/core/src/Materials/PBR/openpbrMaterial.ts

        const previousEffect = subMesh.effect;
        const lightDisposed = defines._areLightsDisposed;
        const effect = this._prepareEffect(mesh, subMesh.getRenderingMesh(), defines, this.onCompiled, this.onError, useInstances, null);

+        if (defines._areLightTexturesReady === false) {
+            return false;
+        }


Same as PBRBaseMaterial: gating on _areLightTexturesReady after _prepareEffect() can trigger an avoidable effect compilation/cache entry with projection/IES disabled. Moving the readiness gate earlier (before _prepareEffect) avoids extra work while the light textures are still loading.

Copilot · 2026-04-08T18:37:30Z

packages/dev/core/src/Materials/materialHelper.functions.ts


    light.prepareLightSpecificDefines(defines, lightIndex);

+    if (!light.areLightTexturesReady()) {
+        state.lightTexturesReady = false;
+    }


This change introduces new readiness behavior (materials are not ready until light textures are ready), but there are no automated tests guarding it. Adding a unit test (NullEngine) that sets a SpotLight projection/IES texture to a not-yet-ready texture and asserts material.isReadyForSubMesh(...)/scene.isReady() remains false until the texture becomes ready would help prevent regressions.

bjsplat · 2026-04-08T19:15:57Z

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

bjsplat · 2026-04-08T19:31:28Z

Snapshot stored with reference name:
refs/pull/18255/merge

Test environment:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18255/merge/index.html

To test a playground add it to the URL, for example:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18255/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/18255/merge
https://sandbox.babylonjs.com/?snapshot=refs/pull/18255/merge
https://gui.babylonjs.com/?snapshot=refs/pull/18255/merge
https://nme.babylonjs.com/?snapshot=refs/pull/18255/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/18255/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.

sebavan

Why using the defines for it ? I do not think we ever used them for it ?

bjsplat · 2026-04-08T20:09:39Z

Interaction tests

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18255/merge/testResults/interactionplaywright/index.html

bjsplat · 2026-04-08T20:30:46Z

Visualization tests for WebGPU

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18255/merge/testResults/webgpuplaywright/index.html

bjsplat · 2026-04-08T20:31:11Z

WebGL2 visualization test reporter:

https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18255/merge/testResults/webgl2playwright/index.html

Copilot AI review requested due to automatic review settings April 8, 2026 18:32

Copilot started reviewing on behalf of bghgary April 8, 2026 18:33 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

sebavan reviewed Apr 8, 2026

View reviewed changes

bghgary marked this pull request as draft April 9, 2026 15:31

	defines._areLightTexturesReady = state.lightTexturesReady;
	defines._areLightTexturesReady = defines._areLightTexturesReady !== false && state.lightTexturesReady;

	defines._areLightTexturesReady = state.lightTexturesReady;
	defines._areLightTexturesReady = (defines._areLightTexturesReady ?? true) && state.lightTexturesReady;

Conversation

bghgary commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root Cause

Fix

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

bjsplat commented Apr 8, 2026

Uh oh!

bjsplat commented Apr 8, 2026

Uh oh!

sebavan left a comment

Choose a reason for hiding this comment

Uh oh!

bjsplat commented Apr 8, 2026

Uh oh!

bjsplat commented Apr 8, 2026

Uh oh!

bjsplat commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bghgary commented Apr 8, 2026 •

edited

Loading