[Fix] Avoid additional shaders creation caused by out of order lights (#5550)

* [Fix] Avoid additional shaders creation caused by out of order lights

* don’t include vertex format to be part of processing key on WebGL

* updates

---------

Co-authored-by: Martin Valigursky <[email protected]>

Author: mvaligursky

src/platform/graphics/shader-processor-options.js

@@ -75,16 +75,22 @@ class ShaderProcessorOptions {
     }
 
     /**
-     * Generate unique key represending the processing options.
+     * Generate unique key representing the processing options.
      *
+     * @param {import('./graphics-device.js').GraphicsDevice} device - The device.
      * @returns {string} - Returns the key.
      */
-    generateKey() {
+    generateKey(device) {
         // TODO: Optimize. Uniform and BindGroup formats should have their keys evaluated in their
         // constructors, and here we should simply concatenate those.
-        return JSON.stringify(this.uniformFormats) +
-        JSON.stringify(this.bindGroupFormats) +
-        this.vertexFormat?.renderingHashString;
+        let key = JSON.stringify(this.uniformFormats) + JSON.stringify(this.bindGroupFormats);
+
+        // WebGPU shaders are processed per vertex format
+        if (device.isWebGPU) {
+            key += this.vertexFormat?.renderingHashString;
+        }
+
+        return key;
     }
 }
 

src/scene/composition/layer-composition.js

@@ -157,17 +157,25 @@ class LayerComposition extends EventHandler {
 
     // function which splits list of lights on a a target object into separate lists of lights based on light type
     _splitLightsArray(target) {
-        const lights = target._lights;
-        target._splitLights[LIGHTTYPE_DIRECTIONAL].length = 0;
-        target._splitLights[LIGHTTYPE_OMNI].length = 0;
-        target._splitLights[LIGHTTYPE_SPOT].length = 0;
 
+        const splitLights = target._splitLights;
+        splitLights[LIGHTTYPE_DIRECTIONAL].length = 0;
+        splitLights[LIGHTTYPE_OMNI].length = 0;
+        splitLights[LIGHTTYPE_SPOT].length = 0;
+
+        const lights = target._lights;
         for (let i = 0; i < lights.length; i++) {
             const light = lights[i];
             if (light.enabled) {
-                target._splitLights[light._type].push(light);
+                splitLights[light._type].push(light);
             }
         }
+
+        // sort the lights by their key, as the order of lights is used to generate shader generation key,
+        // and this avoids new shaders being generated when lights are reordered
+        splitLights[LIGHTTYPE_DIRECTIONAL].sort((a, b) => a.key - b.key);
+        splitLights[LIGHTTYPE_OMNI].sort((a, b) => a.key - b.key);
+        splitLights[LIGHTTYPE_SPOT].sort((a, b) => a.key - b.key);
     }
 
     _update(device, clusteredLightingEnabled = false) {

src/scene/shader-lib/program-library.js

@@ -1,4 +1,5 @@
 import { Debug } from '../../core/debug.js';
+import { hashCode } from '../../core/hash.js';
 import { version, revision } from '../../core/core.js';
 
 import { Shader } from '../../platform/graphics/shader.js';
@@ -119,8 +120,12 @@ class ProgramLibrary {
 
         // we have a key for shader source code generation, a key for its further processing to work with
         // uniform buffers, and a final key to get the processed shader from the cache
-        const generationKey = generator.generateKey(options);
-        const processingKey = processingOptions.generateKey();
+        const generationKeyString = generator.generateKey(options);
+        const generationKey = hashCode(generationKeyString);
+
+        const processingKeyString = processingOptions.generateKey(this._device);
+        const processingKey = hashCode(processingKeyString);
+
         const totalKey = `${generationKey}#${processingKey}`;
 
         // do we have final processed shader
@@ -158,6 +163,13 @@ class ProgramLibrary {
 
             // add new shader to the processed cache
             processedShader = new Shader(this._device, shaderDefinition);
+
+            // keep the keys in the debug mode
+            Debug.call(() => {
+                processedShader._generationKey = generationKeyString;
+                processedShader._processingKey = processingKeyString;
+            });
+
             this.setCachedShader(totalKey, processedShader);
         }
 

src/scene/shader-lib/programs/lit-options-utils.js

@@ -14,17 +14,17 @@ const LitOptionsUtils = {
                 }
                 return key + options[key];
             })
-            .join("");
+            .join("\n");
     },
 
     generateLightsKey(options) {
-        return options.lights.map((light) => {
-            return (!options.clusteredLightingEnabled || light._type === LIGHTTYPE_DIRECTIONAL) ? light.key : "";
+        return 'lights:' + options.lights.map((light) => {
+            return (!options.clusteredLightingEnabled || light._type === LIGHTTYPE_DIRECTIONAL) ? `${light.key},` : '';
         }).join("");
     },
 
     generateChunksKey(options) {
-        return Object.keys(options.chunks ?? {})
+        return 'chunks:\n' + Object.keys(options.chunks ?? {})
             .sort()
             .map(key => key + options.chunks[key])
             .join("");

src/scene/shader-lib/programs/lit.js

@@ -1,4 +1,3 @@
-import { hashCode } from '../../../core/hash.js';
 import { ChunkBuilder } from '../chunk-builder.js';
 import { LitShader } from './lit-shader.js';
 import { LitOptionsUtils } from './lit-options-utils.js';
@@ -19,7 +18,7 @@ const lit = {
             options.shaderChunk +
             LitOptionsUtils.generateKey(options.litOptions);
 
-        return hashCode(key);
+        return key;
     },
 
     /**

src/scene/shader-lib/programs/standard.js

@@ -1,4 +1,3 @@
-import { hashCode } from '../../../core/hash.js';
 import { Debug } from '../../../core/debug.js';
 
 import {
@@ -40,11 +39,11 @@ const standard = {
             props = buildPropertiesList(options);
         }
 
-        const key = "standard" +
-            props.map(prop => prop + options[prop]).join("") +
+        const key = "standard:\n" +
+            props.map(prop => prop + options[prop]).join('\n') +
             LitOptionsUtils.generateKey(options.litOptions);
 
-        return hashCode(key);
+        return key;
     },
 
     // get the value to replace $UV with in Map Shader functions