Repository: playcanvas/engine
Pull request #6250 (2024-04-12 11:40:01)

Basic shader generation refactoring
This PR uses recently added support for `#include` in the Preprocessor (https://github.com/playcanvas/engine/pull/6242) to refactor the basic shader generation. Instead of a JS code that glues strings and chunks together, it creates a list of defines based on the generation options, and the vertex and fragment code is simply processed with those. This makes it a lot easier to understand the shader code, which is now specified as a single string.

This is a prototype of where we want to go with the standard shader in the future.

**New public API:**
- Shader.constructor not accepts list of includes and defines for both vertex and fragment shaders, which are used to drive the shader code processing.


Patch:
diff --git a/src/core/preprocessor.js b/src/core/preprocessor.js
index 269a687c5..43490ffc7 100644
--- a/src/core/preprocessor.js
+++ b/src/core/preprocessor.js
@@ -44,12 +44,17 @@ class Preprocessor {
      * Run c-like preprocessor on the source code, and resolves the code based on the defines and ifdefs
      *
      * @param {string} source - The source code to work on.
-     * @param {Map<string, string>} [includes] - An object containing key-value pairs of include names and their
-     * content.
+     * @param {Map<string, string>} [defines] - A map containing key-value pairs of define names
+     * and their values. These are used for resolving #ifdef style of directives in the source.
+     * @param {Map<string, string>} [includes] - A map containing key-value pairs of include names
+     * and their content. These are used for resolving #include directives in the source.
      * @param {boolean} [stripUnusedColorAttachments] - If true, strips unused color attachments.
      * @returns {string|null} Returns preprocessed source code, or null in case of error.
      */
-    static run(source, includes = new Map(), stripUnusedColorAttachments = false) {
+    static run(source, defines, includes = new Map(), stripUnusedColorAttachments = false) {
+
+        // shallow clone as we will modify the map
+        defines = defines ? new Map(defines) : new Map();
 
         // strips comments, handles // and many cases of /*
         source = source.replace(/\/\*[\s\S]*?\*\/|([^\\:]|^)\/\/.*$/gm, '$1');
@@ -60,7 +65,6 @@ class Preprocessor {
             .join('\n');
 
         // generate defines to remove unused color attachments
-        const defines = new Map();
         if (stripUnusedColorAttachments) {
 
             // find out how many times pcFragColorX is used (see gles3.js)
diff --git a/src/framework/handlers/script.js b/src/framework/handlers/script.js
index 9206eed12..d77f0f71f 100644
--- a/src/framework/handlers/script.js
+++ b/src/framework/handlers/script.js
@@ -8,6 +8,8 @@ import { ResourceLoader } from './loader.js';
 import { ResourceHandler } from './handler.js';
 import { ScriptAttributes } from '../script/script-attributes.js';
 
+const toLowerCamelCase = str => str[0].toLowerCase() + str.substring(1);
+
 /**
  * Resource handler for loading JavaScript files dynamically.  Two types of JavaScript files can be
  * loaded, PlayCanvas scripts which contain calls to {@link createScript}, or regular JavaScript
@@ -164,7 +166,7 @@ class ScriptHandler extends ResourceHandler {
                         }
                         scriptClass.attributes = attributes;
                     }
-                    registerScript(scriptClass, scriptClass.name.toLowerCase());
+                    registerScript(scriptClass, toLowerCamelCase(scriptClass.name));
                 }
             }
 
diff --git a/src/platform/graphics/shader-utils.js b/src/platform/graphics/shader-utils.js
index 0bdb08702..c35bb59ae 100644
--- a/src/platform/graphics/shader-utils.js
+++ b/src/platform/graphics/shader-utils.js
@@ -50,7 +50,20 @@ class ShaderUtils {
      * @param {string} [options.fragmentDefines] - The fragment shader defines.
      * @param {string} [options.fragmentExtensions] - The fragment shader extensions code.
      * @param {string} [options.fragmentPreamble] - The preamble string for the fragment shader.
-     * @param {boolean} [options.useTransformFeedback] - Whether to use transform feedback. Defaults to false.
+     * @param {boolean} [options.useTransformFeedback] - Whether to use transform feedback. Defaults
+     * to false.
+     * @param {Map<string, string>} [options.vertexIncludesMap] - A map containing key-value pairs of
+     * include names and their content. These are used for resolving #include directives in the
+     * vertex shader source.
+     * @param {Map<string, string>} [options.vertexDefinesMap] - A map containing key-value pairs of
+     * define names and their values. These are used for resolving #ifdef style of directives in the
+     * vertex code.
+     * @param {Map<string, string>} [options.fragmentIncludesMap] - A map containing key-value pairs
+     * of include names and their content. These are used for resolving #include directives in the
+     * fragment shader source.
+     * @param {Map<string, string>} [options.fragmentDefinesMap] - A map containing key-value pairs of
+     * define names and their values. These are used for resolving #ifdef style of directives in the
+     * fragment code.
      * @param {string | string[]} [options.fragmentOutputTypes] - Fragment shader output types,
      * which default to vec4. Passing a string will set the output type for all color attachments.
      * Passing an array will set the output type for each color attachment.
@@ -112,6 +125,10 @@ class ShaderUtils {
             name: name,
             attributes: attribs,
             vshader: vertCode,
+            vdefines: options.vertexDefinesMap,
+            vincludes: options.vertexIncludesMap,
+            fdefines: options.fragmentDefinesMap,
+            fincludes: options.fragmentIncludesMap,
             fshader: fragCode,
             useTransformFeedback: options.useTransformFeedback
         };
diff --git a/src/platform/graphics/shader.js b/src/platform/graphics/shader.js
index f191e6032..e7266a8ba 100644
--- a/src/platform/graphics/shader.js
+++ b/src/platform/graphics/shader.js
@@ -53,6 +53,18 @@ class Shader {
      * useTransformFeedback or compute shader is specified.
      * @param {string} [definition.cshader] - Compute shader source (WGSL code). Only supported on
      * WebGPU platform.
+     * @param {Map<string, string>} [definition.vincludes] - A map containing key-value pairs of
+     * include names and their content. These are used for resolving #include directives in the
+     * vertex shader source.
+     * @param {Map<string, string>} [definition.vdefines] - A map containing key-value pairs of
+     * define names and their values. These are used for resolving #ifdef style of directives in the
+     * vertex code.
+     * @param {Map<string, string>} [definition.fincludes] - A map containing key-value pairs
+     * of include names and their content. These are used for resolving #include directives in the
+     * fragment shader source.
+     * @param {Map<string, string>} [definition.fdefines] - A map containing key-value pairs of
+     * define names and their values. These are used for resolving #ifdef style of directives in the
+     * fragment code.
      * @param {boolean} [definition.useTransformFeedback] - Specifies that this shader outputs
      * post-VS data to a buffer.
      * @param {string | string[]} [definition.fragmentOutputTypes] - Fragment shader output types,
@@ -106,13 +118,13 @@ class Shader {
             Debug.assert(definition.fshader, 'No fragment shader has been specified when creating a shader.');
 
             // pre-process shader sources
-            definition.vshader = Preprocessor.run(definition.vshader);
+            definition.vshader = Preprocessor.run(definition.vshader, definition.vdefines, definition.vincludes);
 
             // Strip unused color attachments from fragment shader.
             // Note: this is only needed for iOS 15 on WebGL2 where there seems to be a bug where color attachments that are not
             // written to generate metal linking errors. This is fixed on iOS 16, and iOS 14 does not support WebGL2.
             const stripUnusedColorAttachments = graphicsDevice.isWebGL2 && (platform.name === 'osx' || platform.name === 'ios');
-            definition.fshader = Preprocessor.run(definition.fshader, undefined, stripUnusedColorAttachments);
+            definition.fshader = Preprocessor.run(definition.fshader, definition.fdefines, definition.fincludes, stripUnusedColorAttachments);
         }
 
         this.impl = graphicsDevice.createShaderImpl(this);
diff --git a/src/scene/shader-lib/program-library.js b/src/scene/shader-lib/program-library.js
index cb1a69b21..2ab260e49 100644
--- a/src/scene/shader-lib/program-library.js
+++ b/src/scene/shader-lib/program-library.js
@@ -172,6 +172,10 @@ class ProgramLibrary {
                 name: `${generatedShaderDef.name}${passName}-proc`,
                 attributes: generatedShaderDef.attributes,
                 vshader: generatedShaderDef.vshader,
+                vdefines: generatedShaderDef.vdefines,
+                vincludes: generatedShaderDef.vincludes,
+                fdefines: generatedShaderDef.fdefines,
+                fincludes: generatedShaderDef.fincludes,
                 fshader: generatedShaderDef.fshader,
                 processingOptions: processingOptions,
                 shaderLanguage: generatedShaderDef.shaderLanguage
diff --git a/src/scene/shader-lib/programs/basic.js b/src/scene/shader-lib/programs/basic.js
index 4566f5805..b00868015 100644
--- a/src/scene/shader-lib/programs/basic.js
+++ b/src/scene/shader-lib/programs/basic.js
@@ -3,14 +3,118 @@ import {
 } from '../../../platform/graphics/constants.js';
 import { ShaderUtils } from '../../../platform/graphics/shader-utils.js';
 import { shaderChunks } from '../chunks/chunks.js';
-
-import {
-    SHADER_DEPTH, SHADER_PICK
-} from '../../constants.js';
 import { ShaderPass } from '../../shader-pass.js';
-
 import { ShaderGenerator } from './shader-generator.js';
 
+const vShader = `
+
+    #include "shaderPassDefines"
+    #include "transformDeclVS"
+
+    #ifdef SKIN
+        #include "skinCode"
+    #endif
+
+    #include "transformVS"
+
+    #ifdef VERTEX_COLORS
+        attribute vec4 vertex_color;
+        varying vec4 vColor;
+    #endif
+
+    #ifdef DIFFUSE_MAP
+        attribute vec2 vertex_texCoord0;
+        varying vec2 vUv0;
+    #endif
+
+    #ifdef DEPTH_PASS
+        varying float vDepth;
+        
+        #ifndef VIEWMATRIX
+        #define VIEWMATRIX
+            uniform mat4 matrix_view;
+        #endif
+
+        #ifndef CAMERAPLANES
+        #define CAMERAPLANES
+            uniform vec4 camera_params;
+        #endif
+    #endif
+
+    void main(void) {
+        gl_Position = getPosition();
+
+        #ifdef DEPTH_PASS
+            vDepth = -(matrix_view * vec4(getWorldPosition(),1.0)).z * camera_params.x;
+        #endif        
+
+        #ifdef VERTEX_COLORS
+            vColor = vertex_color;
+        #endif
+
+        #ifdef DIFFUSE_MAP
+            vUv0 = vertex_texCoord0;
+        #endif
+    }
+`;
+
+const fShader = `
+
+    #include "shaderPassDefines"
+
+    #ifdef VERTEX_COLORS
+        varying vec4 vColor;
+    #else
+        uniform vec4 uColor;
+    #endif
+
+    #ifdef DIFFUSE_MAP
+        varying vec2 vUv0;
+        uniform sampler2D texture_diffuseMap;
+    #endif
+
+    #ifdef FOG
+        #include "fogCode"
+    #endif
+
+    #ifdef ALPHA_TEST
+        #include "alphaTestPS"
+    #endif
+
+    #ifdef DEPTH_PASS
+        varying float vDepth;
+        #include "packDepthPS"
+    #endif
+
+    void main(void) {
+
+        #ifdef VERTEX_COLORS
+            gl_FragColor = vColor;
+        #else
+            gl_FragColor = uColor;
+        #endif
+
+        #ifdef DIFFUSE_MAP
+            gl_FragColor *= texture2D(texture_diffuseMap, vUv0);
+        #endif
+
+        #ifdef ALPHA_TEST
+            alphaTest(gl_FragColor.a);
+        #endif
+
+        #ifndef PICK_PASS
+
+            #ifdef DEPTH_PASS
+                gl_FragColor = packFloat(vDepth);
+            #else
+                #ifdef FOG
+                    glFragColor.rgb = addFog(gl_FragColor.rgb);
+                #endif
+            #endif
+        #endif
+    }
+`;
+
 class ShaderGeneratorBasic extends ShaderGenerator {
     generateKey(options) {
         let key = 'basic';
@@ -30,7 +134,8 @@ class ShaderGeneratorBasic extends ShaderGenerator {
         return key;
     }
 
-    createShaderDefinition(device, options) {
+    createAttributesDefinition(definitionOptions, options) {
+
         // GENERATE ATTRIBUTES
         const attributes = {
             vertex_position: SEMANTIC_POSITION
@@ -46,124 +151,60 @@ class ShaderGeneratorBasic extends ShaderGenerator {
             attributes.vertex_texCoord0 = SEMANTIC_TEXCOORD0;
         }
 
-        const shaderPassInfo = ShaderPass.get(device).getByIndex(options.pass);
-        const shaderPassDefines = shaderPassInfo.shaderDefines;
-
-        // GENERATE VERTEX SHADER
-        let vshader = shaderPassDefines;
-
-        // VERTEX SHADER DECLARATIONS
-        vshader += shaderChunks.transformDeclVS;
-
-        if (options.skin) {
-            vshader += ShaderGenerator.skinCode(device);
-            vshader += shaderChunks.transformSkinnedVS;
-        } else {
-            vshader += shaderChunks.transformVS;
-        }
-
-        if (options.vertexColors) {
-            vshader += 'attribute vec4 vertex_color;\n';
-            vshader += 'varying vec4 vColor;\n';
-        }
-        if (options.diffuseMap) {
-            vshader += 'attribute vec2 vertex_texCoord0;\n';
-            vshader += 'varying vec2 vUv0;\n';
-        }
-
-        if (options.pass === SHADER_DEPTH) {
-            vshader += 'varying float vDepth;\n';
-            vshader += '#ifndef VIEWMATRIX\n';
-            vshader += '#define VIEWMATRIX\n';
-            vshader += 'uniform mat4 matrix_view;\n';
-            vshader += '#endif\n';
-            vshader += '#ifndef CAMERAPLANES\n';
-            vshader += '#define CAMERAPLANES\n';
-            vshader += 'uniform vec4 camera_params;\n\n';
-            vshader += '#endif\n';
-        }
+        definitionOptions.attributes = attributes;
+    }
 
-        // VERTEX SHADER BODY
-        vshader += ShaderGenerator.begin();
+    createVertexDefinition(device, definitionOptions, options, shaderPassInfo) {
 
-        vshader += "   gl_Position = getPosition();\n";
+        const includes = new Map();
+        const defines = new Map();
 
-        if (options.pass === SHADER_DEPTH) {
-            vshader += "    vDepth = -(matrix_view * vec4(getWorldPosition(),1.0)).z * camera_params.x;\n";
-        }
+        includes.set('shaderPassDefines', shaderPassInfo.shaderDefines);
+        includes.set('transformDeclVS', shaderChunks.transformDeclVS);
+        includes.set('transformVS', shaderChunks.transformVS);
+        includes.set('skinCode', ShaderGenerator.skinCode(device));
 
-        if (options.vertexColors) {
-            vshader += '    vColor = vertex_color;\n';
-        }
-        if (options.diffuseMap) {
-            vshader += '    vUv0 = vertex_texCoord0;\n';
-        }
+        if (options.skin) defines.set('SKIN', true);
+        if (options.vertexColors) defines.set('VERTEX_COLORS', true);
+        if (options.diffuseMap) defines.set('DIFFUSE_MAP', true);
 
-        vshader += ShaderGenerator.end();
+        definitionOptions.vertexCode = vShader;
+        definitionOptions.vertexIncludesMap = includes;
+        definitionOptions.vertexDefinesMap = defines;
+    }
 
-        // GENERATE FRAGMENT SHADER
-        let fshader = shaderPassDefines;
+    createFragmentDefinition(definitionOptions, options, shaderPassInfo) {
 
-        // FRAGMENT SHADER DECLARATIONS
-        if (options.vertexColors) {
-            fshader += 'varying vec4 vColor;\n';
-        } else {
-            fshader += 'uniform vec4 uColor;\n';
-        }
-        if (options.diffuseMap) {
-            fshader += 'varying vec2 vUv0;\n';
-            fshader += 'uniform sampler2D texture_diffuseMap;\n';
-        }
-        if (options.fog) {
-            fshader += ShaderGenerator.fogCode(options.fog);
-        }
-        if (options.alphaTest) {
-            fshader += shaderChunks.alphaTestPS;
-        }
+        const includes = new Map();
+        const defines = new Map();
 
-        if (options.pass === SHADER_DEPTH) {
-            // ##### SCREEN DEPTH PASS #####
-            fshader += 'varying float vDepth;\n';
-            fshader += shaderChunks.packDepthPS;
-        }
+        includes.set('shaderPassDefines', shaderPassInfo.shaderDefines);
+        includes.set('fogCode', ShaderGenerator.fogCode(options.fog));
+        includes.set('alphaTestPS', shaderChunks.alphaTestPS);
+        includes.set('packDepthPS', shaderChunks.packDepthPS);
 
-        // FRAGMENT SHADER BODY
-        fshader += ShaderGenerator.begin();
+        if (options.vertexColors) defines.set('VERTEX_COLORS', true);
+        if (options.diffuseMap) defines.set('DIFFUSE_MAP', true);
+        if (options.fog) defines.set('FOG', true);
+        if (options.alphaTest) defines.set('ALPHA_TEST', true);
 
-        // Read the map texels that the shader needs
-        if (options.vertexColors) {
-            fshader += '    gl_FragColor = vColor;\n';
-        } else {
-            fshader += '    gl_FragColor = uColor;\n';
-        }
-        if (options.diffuseMap) {
-            fshader += '    gl_FragColor *= texture2D(texture_diffuseMap, vUv0);\n';
-        }
+        definitionOptions.fragmentCode = fShader;
+        definitionOptions.fragmentIncludesMap = includes;
+        definitionOptions.fragmentDefinesMap = defines;
+    }
 
-        if (options.alphaTest) {
-            fshader += "   alphaTest(gl_FragColor.a);\n";
-        }
+    createShaderDefinition(device, options) {
 
-        if (options.pass !== SHADER_PICK) {
-            if (options.pass === SHADER_DEPTH) {
-                // ##### SCREEN DEPTH PASS #####
-                fshader += "    gl_FragColor = packFloat(vDepth);\n";
-            } else {
-                // ##### FORWARD PASS #####
-                if (options.fog) {
-                    fshader += "   glFragColor.rgb = addFog(gl_FragColor.rgb);\n";
-                }
-            }
-        }
+        const definitionOptions = {
+            name: 'BasicShader'
+        };
 
-        fshader += ShaderGenerator.end();
+        const shaderPassInfo = ShaderPass.get(device).getByIndex(options.pass);
+        this.createAttributesDefinition(definitionOptions, options);
+        this.createVertexDefinition(device, definitionOptions, options, shaderPassInfo);
+        this.createFragmentDefinition(definitionOptions, options, shaderPassInfo);
 
-        return ShaderUtils.createDefinition(device, {
-            name: 'BasicShader',
-            attributes: attributes,
-            vertexCode: vshader,
-            fragmentCode: fshader
-        });
+        return ShaderUtils.createDefinition(device, definitionOptions);
     }
 }
 


Test patch:
diff --git a/test/core/preprocessor.test.mjs b/test/core/preprocessor.test.mjs
index cf26a6724..2995abe6b 100644
--- a/test/core/preprocessor.test.mjs
+++ b/test/core/preprocessor.test.mjs
@@ -13,6 +13,9 @@ describe('Preprocessor', function () {
         `],
         ['inc2', 'block2']
     ]);
+
+    const defines = new Map();
+
     const srcData = `
         
         #define FEATURE1
@@ -75,82 +78,82 @@ describe('Preprocessor', function () {
     `;
 
     it('returns false for MORPH_A', function () {
-        expect(Preprocessor.run(srcData, includes).includes('MORPH_A')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('MORPH_A')).to.equal(false);
     });
 
     it('returns false for MORPH_B', function () {
-        expect(Preprocessor.run(srcData, includes).includes('MORPH_B')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('MORPH_B')).to.equal(false);
     });
 
     it('returns true for $', function () {
-        expect(Preprocessor.run(srcData, includes).includes('$')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('$')).to.equal(true);
     });
 
     it('returns true for TEST1', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST1')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST1')).to.equal(true);
     });
 
     it('returns true for TEST2', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST2')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST2')).to.equal(true);
     });
 
     it('returns true for TEST3', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST3')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST3')).to.equal(true);
     });
 
     it('returns true for TEST4', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST4')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST4')).to.equal(true);
     });
 
     it('returns false for TEST5', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST5')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST5')).to.equal(false);
     });
 
     it('returns true for TEST6', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST6')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST6')).to.equal(true);
     });
 
     it('returns false for TEST7', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST7')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST7')).to.equal(false);
     });
 
     it('returns false for TEST8', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST8')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST8')).to.equal(false);
     });
 
     it('returns false for TEST9', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST9')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST9')).to.equal(false);
     });
 
     it('returns true for TEST10', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST10')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST10')).to.equal(true);
     });
 
     it('returns false for TEST11', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST11')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST11')).to.equal(false);
     });
 
     it('returns false for TEST12', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST12')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST12')).to.equal(false);
     });
 
     it('returns true for TEST13', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST13')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST13')).to.equal(true);
     });
 
     it('returns false for TEST14', function () {
-        expect(Preprocessor.run(srcData, includes).includes('TEST14')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('TEST14')).to.equal(false);
     });
 
     it('returns true for INC1', function () {
-        expect(Preprocessor.run(srcData, includes).includes('block1')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('block1')).to.equal(true);
     });
 
     it('returns false for INC2', function () {
-        expect(Preprocessor.run(srcData, includes).includes('block2')).to.equal(false);
+        expect(Preprocessor.run(srcData, defines, includes).includes('block2')).to.equal(false);
     });
 
     it('returns true for nested', function () {
-        expect(Preprocessor.run(srcData, includes).includes('nested')).to.equal(true);
+        expect(Preprocessor.run(srcData, defines, includes).includes('nested')).to.equal(true);
     });
 });