Merge pull request #6751 from sebavan/master

 Fix Parallel Shader Compile with lights UBOs

src/Lights/light.ts

@@ -483,7 +483,7 @@ export abstract class Light extends Node {
 
         // Remove from meshes
         for (var mesh of this.getScene().meshes) {
-            mesh._removeLightSource(this);
+            mesh._removeLightSource(this, true);
         }
 
         this._uniformBuffer.dispose();

src/Materials/PBR/pbrBaseMaterial.ts

@@ -732,6 +732,8 @@ export abstract class PBRBaseMaterial extends PushMaterial {
      */
     public customShaderNameResolve: (shaderName: string, uniforms: string[], uniformBuffers: string[], samplers: string[], defines: PBRMaterialDefines) => string;
 
+    protected _rebuildInParallel = false;
+
     /**
      * Instantiates a new PBRMaterial instance.
      *
@@ -1007,15 +1009,24 @@ export abstract class PBRBaseMaterial extends PushMaterial {
             Logger.Warn("PBRMaterial: Normals have been created for the mesh: " + mesh.name);
         }
 
-        let previousEffect = subMesh.effect;
+        const previousEffect = subMesh.effect;
+        const lightDisposed = defines._areLightsDisposed;
         let effect = this._prepareEffect(mesh, defines, this.onCompiled, this.onError, useInstances);
 
         if (effect) {
             // Use previous effect while new one is compiling
             if (this.allowShaderHotSwapping && previousEffect && !effect.isReady()) {
                 effect = previousEffect;
+                this._rebuildInParallel = true;
                 defines.markAsUnprocessed();
+
+                if (lightDisposed) {
+                    // re register in case it takes more than one frame.
+                    defines._areLightsDisposed = true;
+                    return false;
+                }
             } else {
+                this._rebuildInParallel = false;
                 scene.resetCachedMaterial();
                 subMesh.setEffect(effect, defines);
                 this.buildUniformLayout();
@@ -1887,7 +1898,7 @@ export abstract class PBRBaseMaterial extends PushMaterial {
         if (mustRebind || !this.isFrozen) {
             // Lights
             if (scene.lightsEnabled && !this._disableLighting) {
-                MaterialHelper.BindLights(scene, mesh, this._activeEffect, defines, this._maxSimultaneousLights, this._lightFalloff !== PBRBaseMaterial.LIGHTFALLOFF_STANDARD);
+                MaterialHelper.BindLights(scene, mesh, this._activeEffect, defines, this._maxSimultaneousLights, this._lightFalloff !== PBRBaseMaterial.LIGHTFALLOFF_STANDARD, this._rebuildInParallel);
             }
 
             // View

src/Materials/materialDefines.ts

@@ -11,6 +11,8 @@ export class MaterialDefines {
     /** @hidden */
     public _areLightsDirty = true;
     /** @hidden */
+    public _areLightsDisposed = false;
+    /** @hidden */
     public _areAttributesDirty = true;
     /** @hidden */
     public _areTexturesDirty = true;
@@ -49,6 +51,7 @@ export class MaterialDefines {
         this._areTexturesDirty = false;
         this._areFresnelDirty = false;
         this._areLightsDirty = false;
+        this._areLightsDisposed = false;
         this._areMiscDirty = false;
         this._areImageProcessingDirty = false;
     }
@@ -83,9 +86,11 @@ export class MaterialDefines {
 
     /**
      * Marks the material to indicate the lights need to be re-calculated
+     * @param disposed Defines whether the light is dirty due to dispose or not
      */
-    public markAsLightDirty() {
+    public markAsLightDirty(disposed = false) {
         this._areLightsDirty = true;
+        this._areLightsDisposed = this._areLightsDisposed || disposed;
         this._isDirty = true;
     }
 

src/Materials/materialHelper.ts

@@ -679,12 +679,15 @@ export class MaterialHelper {
      * @param effect The effect we are binding the data to
      * @param useSpecular Defines if specular is supported
      * @param usePhysicalLightFalloff Specifies whether the light falloff is defined physically or not
+     * @param rebuildInParallel Specifies whether the shader is rebuilding in parallel
      */
-    public static BindLight(light: Light, lightIndex: number, scene: Scene, mesh: AbstractMesh, effect: Effect, useSpecular: boolean, usePhysicalLightFalloff = false): void {
+    public static BindLight(light: Light, lightIndex: number, scene: Scene, mesh: AbstractMesh, effect: Effect, useSpecular: boolean, usePhysicalLightFalloff = false, rebuildInParallel = false): void {
         let iAsString = lightIndex.toString();
 
         let scaledIntensity = light.getScaledIntensity();
-        light._uniformBuffer.bindToEffect(effect, "Light" + iAsString);
+        if (!rebuildInParallel) {
+            light._uniformBuffer.bindToEffect(effect, "Light" + iAsString);
+        }
 
         MaterialHelper.BindLightProperties(light, effect, lightIndex);
 
@@ -710,14 +713,15 @@ export class MaterialHelper {
      * @param defines The generated defines for the effect
      * @param maxSimultaneousLights The maximum number of light that can be bound to the effect
      * @param usePhysicalLightFalloff Specifies whether the light falloff is defined physically or not
+     * @param rebuildInParallel Specifies whether the shader is rebuilding in parallel
      */
-    public static BindLights(scene: Scene, mesh: AbstractMesh, effect: Effect, defines: any, maxSimultaneousLights = 4, usePhysicalLightFalloff = false): void {
+    public static BindLights(scene: Scene, mesh: AbstractMesh, effect: Effect, defines: any, maxSimultaneousLights = 4, usePhysicalLightFalloff = false, rebuildInParallel = false): void {
         let len = Math.min(mesh.lightSources.length, maxSimultaneousLights);
 
         for (var i = 0; i < len; i++) {
 
             let light = mesh.lightSources[i];
-            this.BindLight(light, i, scene, mesh, effect, typeof defines === "boolean" ? defines : defines["SPECULARTERM"], usePhysicalLightFalloff);
+            this.BindLight(light, i, scene, mesh, effect, typeof defines === "boolean" ? defines : defines["SPECULARTERM"], usePhysicalLightFalloff, rebuildInParallel);
         }
     }
 

src/Materials/standardMaterial.ts

@@ -666,6 +666,7 @@ export class StandardMaterial extends PushMaterial {
     protected _worldViewProjectionMatrix = Matrix.Zero();
     protected _globalAmbientColor = new Color3(0, 0, 0);
     protected _useLogarithmicDepth: boolean;
+    protected _rebuildInParallel = false;
 
     /**
      * Instantiates a new standard material.
@@ -1020,6 +1021,7 @@ export class StandardMaterial extends PushMaterial {
 
         // Get correct effect
         if (defines.isDirty) {
+            const lightDisposed = defines._areLightsDisposed;
             defines.markAsProcessed();
 
             // Fallbacks
@@ -1167,8 +1169,16 @@ export class StandardMaterial extends PushMaterial {
                 // Use previous effect while new one is compiling
                 if (this.allowShaderHotSwapping && previousEffect && !effect.isReady()) {
                     effect = previousEffect;
+                    this._rebuildInParallel = true;
                     defines.markAsUnprocessed();
+
+                    if (lightDisposed) {
+                        // re register in case it takes more than one frame.
+                        defines._areLightsDisposed = true;
+                        return false;
+                    }
                 } else {
+                    this._rebuildInParallel = false;
                     scene.resetCachedMaterial();
                     subMesh.setEffect(effect, defines);
                     this.buildUniformLayout();
@@ -1477,7 +1487,7 @@ export class StandardMaterial extends PushMaterial {
         if (mustRebind || !this.isFrozen) {
             // Lights
             if (scene.lightsEnabled && !this._disableLighting) {
-                MaterialHelper.BindLights(scene, mesh, effect, defines, this._maxSimultaneousLights);
+                MaterialHelper.BindLights(scene, mesh, effect, defines, this._maxSimultaneousLights, false, this._rebuildInParallel);
             }
 
             // View

src/Meshes/abstractMesh.ts

@@ -791,7 +791,7 @@ export class AbstractMesh extends TransformNode implements IDisposable, ICullabl
     }
 
     /** @hidden */
-    public _removeLightSource(light: Light): void {
+    public _removeLightSource(light: Light, dispose: boolean): void {
         var index = this._lightSources.indexOf(light);
 
         if (index === -1) {
@@ -799,7 +799,7 @@ export class AbstractMesh extends TransformNode implements IDisposable, ICullabl
         }
         this._lightSources.splice(index, 1);
 
-        this._markSubMeshesAsLightDirty();
+        this._markSubMeshesAsLightDirty(dispose);
     }
 
     private _markSubMeshesAsDirty(func: (defines: MaterialDefines) => void) {
@@ -815,8 +815,8 @@ export class AbstractMesh extends TransformNode implements IDisposable, ICullabl
     }
 
     /** @hidden */
-    public _markSubMeshesAsLightDirty() {
-        this._markSubMeshesAsDirty((defines) => defines.markAsLightDirty());
+    public _markSubMeshesAsLightDirty(dispose: boolean = false) {
+        this._markSubMeshesAsDirty((defines) => defines.markAsLightDirty(dispose));
     }
 
     /** @hidden */

src/Meshes/instancedMesh.ts

@@ -70,7 +70,7 @@ export class InstancedMesh extends AbstractMesh {
         // Do nothing as all the work will be done by source mesh
     }
 
-    public _removeLightSource(light: Light): void {
+    public _removeLightSource(light: Light, dispose: boolean): void {
         // Do nothing as all the work will be done by source mesh
     }
 

src/scene.ts

@@ -2106,7 +2106,7 @@ export class Scene extends AbstractScene implements IAnimatable {
         if (index !== -1) {
             // Remove from meshes
             for (var mesh of this.meshes) {
-                mesh._removeLightSource(toRemove);
+                mesh._removeLightSource(toRemove, false);
             }
 
             // Remove from the scene if mesh found