From cac7efcc3704ed45c4597264a3a594365efccb81 Mon Sep 17 00:00:00 2001
From: Martin Valigursky <59932779+mvaligursky@users.noreply.github.com>
Date: Wed, 21 Feb 2024 16:31:10 +0000
Subject: [PATCH] Link WebGL shaders immediately after compilation (#6076)

Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
---
 .../graphics/webgl/webgl-graphics-device.js   |  9 -----
 src/platform/graphics/webgl/webgl-shader.js   | 35 +++----------------
 src/scene/renderer/forward-renderer.js        |  3 --
 src/scene/renderer/shadow-renderer.js         |  3 --
 4 files changed, 5 insertions(+), 45 deletions(-)

diff --git a/src/platform/graphics/webgl/webgl-graphics-device.js b/src/platform/graphics/webgl/webgl-graphics-device.js
index 1d3583f247f..317d03699d2 100644
--- a/src/platform/graphics/webgl/webgl-graphics-device.js
+++ b/src/platform/graphics/webgl/webgl-graphics-device.js
@@ -1352,15 +1352,6 @@ class WebglGraphicsDevice extends GraphicsDevice {
         this.gpuProfiler?.restoreContext();
     }
 
-    /**
-     * Called after a batch of shaders was created, to guide in their optimal preparation for rendering.
-     *
-     * @ignore
-     */
-    endShaderBatch() {
-        WebglShader.endShaderBatch(this);
-    }
-
     /**
      * Set the active rectangle for rendering on the specified device.
      *
diff --git a/src/platform/graphics/webgl/webgl-shader.js b/src/platform/graphics/webgl/webgl-shader.js
index 36bdd21d86f..2f4d601b775 100644
--- a/src/platform/graphics/webgl/webgl-shader.js
+++ b/src/platform/graphics/webgl/webgl-shader.js
@@ -34,18 +34,8 @@ class CompiledShaderCache {
     }
 }
 
-// class used to hold a list of recently created shaders forming a batch, to allow their more optimized compilation
-class ShaderBatchCache {
-    shaders = [];
-
-    loseContext(device) {
-        this.shaders = [];
-    }
-}
-
 const _vertexShaderCache = new DeviceCache();
 const _fragmentShaderCache = new DeviceCache();
-const _shaderBatchCache = new DeviceCache();
 
 /**
  * A WebGL implementation of the Shader.
@@ -58,12 +48,11 @@ class WebglShader {
     constructor(shader) {
         this.init();
 
-        // kick off vertex and fragment shader compilation, but not linking here, as that would
-        // make it blocking.
+        // kick off vertex and fragment shader compilation
         this.compile(shader.device, shader);
 
-        // add the shader to recently created list
-        WebglShader.getBatchShaders(shader.device).push(shader);
+        // kick off linking, as this is non-blocking too
+        this.link(shader.device, shader);
 
         // add it to a device list of all shaders
         shader.device.shaders.push(shader);
@@ -91,22 +80,6 @@ class WebglShader {
         this.glFragmentShader = null;
     }
 
-    static getBatchShaders(device) {
-        const batchCache = _shaderBatchCache.get(device, () => {
-            return new ShaderBatchCache();
-        });
-        return batchCache.shaders;
-    }
-
-    static endShaderBatch(device) {
-
-        // Trigger link step for all recently created shaders. This allows linking to be done in parallel, before
-        // the blocking wait on the linking result is triggered in finalize function
-        const shaders = WebglShader.getBatchShaders(device);
-        shaders.forEach(shader => shader.impl.link(device, shader));
-        shaders.length = 0;
-    }
-
     /**
      * Dispose the shader when the context has been lost.
      */
@@ -305,6 +278,8 @@ class WebglShader {
             linkStartTime = now();
         });
 
+        // check the link status of a shader - this is a blocking operation waiting for the shader
+        // to finish compiling and linking
         const linkStatus = gl.getProgramParameter(glProgram, gl.LINK_STATUS);
         if (!linkStatus) {
 
diff --git a/src/scene/renderer/forward-renderer.js b/src/scene/renderer/forward-renderer.js
index 75aa598ede3..0d136fa8c11 100644
--- a/src/scene/renderer/forward-renderer.js
+++ b/src/scene/renderer/forward-renderer.js
@@ -530,9 +530,6 @@ class ForwardRenderer extends Renderer {
             prevLightMask = lightMask;
         }
 
-        // process the batch of shaders created here
-        device.endShaderBatch?.();
-
         return _drawCallList;
     }
 
diff --git a/src/scene/renderer/shadow-renderer.js b/src/scene/renderer/shadow-renderer.js
index c6e2455dfd6..64c6ef7ad26 100644
--- a/src/scene/renderer/shadow-renderer.js
+++ b/src/scene/renderer/shadow-renderer.js
@@ -300,9 +300,6 @@ class ShadowRenderer {
         const passFlags = 1 << SHADER_SHADOW;
         const shadowPass = this.getShadowPass(light);
 
-        // TODO: Similarly to forward renderer, a shader creation part of this loop should be split into a separate loop,
-        // and endShaderBatch should be called at its end
-
         // Render
         const count = visibleCasters.length;
         for (let i = 0; i < count; i++) {