Skip to content

Commit

Permalink
Updated handling of defines specified by a shader definition (#6253)
Browse files Browse the repository at this point in the history 
Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
  • Loading branch information
@mvaligursky
mvaligursky and Martin Valigursky authored Apr 15, 2024
1 parent dd65f24 commit 2a136a0
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 54 deletions.
8 changes: 2 additions & 6 deletions src/core/preprocessor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,12 @@ 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>} [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, defines, includes = new Map(), stripUnusedColorAttachments = false) {

// shallow clone as we will modify the map
defines = defines ? new Map(defines) : new Map();
static run(source, includes = new Map(), stripUnusedColorAttachments = false) {

// strips comments, handles // and many cases of /*
source = source.replace(/\/\*[\s\S]*?\*\/|([^\\:]|^)\/\/.*$/gm, '$1');
Expand All @@ -65,6 +60,7 @@ 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)
Expand Down
41 changes: 27 additions & 14 deletions src/platform/graphics/shader-utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,22 @@ class ShaderUtils {
* @param {object} [options.attributes] - Attributes. Will be extracted from the vertexCode if
* not provided.
* @param {string} options.vertexCode - The vertex shader code.
* @param {string} [options.vertexDefines] - The vertex shader defines.
* @param {string} [options.vertexExtensions] - The vertex shader extensions code.
* @param {string} [options.fragmentCode] - The fragment shader code.
* @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 {Map<string, string>} [options.vertexIncludesMap] - A map containing key-value pairs of
* @param {Map<string, string>} [options.vertexIncludes] - 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
* @param {Map<string, string>} [options.vertexDefines] - 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
* @param {Map<string, string>} [options.fragmentIncludes] - 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
* @param {Map<string, string>} [options.fragmentDefines] - 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,
Expand All @@ -71,6 +69,10 @@ class ShaderUtils {
*/
static createDefinition(device, options) {
Debug.assert(options);
Debug.assert(!options.vertexDefines || options.vertexDefines instanceof Map);
Debug.assert(!options.vertexIncludes || options.vertexIncludes instanceof Map);
Debug.assert(!options.fragmentDefines || options.fragmentDefines instanceof Map);
Debug.assert(!options.fragmentIncludes || options.fragmentIncludes instanceof Map);

const getDefines = (gpu, gl2, gl1, isVertex, options) => {

Expand Down Expand Up @@ -101,18 +103,18 @@ class ShaderUtils {
const name = options.name ?? 'Untitled';

// vertex code
const vertDefines = options.vertexDefines || getDefines(webgpuVS, gles3VS, '', true, options);
const vertCode = ShaderUtils.versionCode(device) +
vertDefines +
getDefines(webgpuVS, gles3VS, '', true, options) +
ShaderUtils.getDefinesCode(options.vertexDefines) +
sharedFS +
ShaderUtils.getShaderNameCode(name) +
options.vertexCode;

// fragment code
const fragDefines = options.fragmentDefines || getDefines(webgpuFS, gles3FS, gles2FS, false, options);
const fragCode = (options.fragmentPreamble || '') +
ShaderUtils.versionCode(device) +
fragDefines +
getDefines(webgpuFS, gles3FS, gles2FS, false, options) +
ShaderUtils.getDefinesCode(options.fragmentDefines) +
ShaderUtils.precisionCode(device) + '\n' +
sharedFS +
ShaderUtils.getShaderNameCode(name) +
Expand All @@ -125,15 +127,26 @@ class ShaderUtils {
name: name,
attributes: attribs,
vshader: vertCode,
vdefines: options.vertexDefinesMap,
vincludes: options.vertexIncludesMap,
fdefines: options.fragmentDefinesMap,
fincludes: options.fragmentIncludesMap,
vincludes: options.vertexIncludes,
fincludes: options.fragmentIncludes,
fshader: fragCode,
useTransformFeedback: options.useTransformFeedback
};
}

/**
* @param {Map<string, string>} [defines] - A map containing key-value pairs.
* @returns {string} The shader code for the defines.
* @private
*/
static getDefinesCode(defines) {
let code = '';
defines?.forEach((value, key) => {
code += `#define ${key} ${value}\n`;
});
return code;
}

// SpectorJS integration
static getShaderNameCode(name) {
return `#define SHADER_NAME ${name}\n`;
Expand Down
10 changes: 2 additions & 8 deletions src/platform/graphics/shader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,9 @@ class Shader {
* @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,
Expand Down Expand Up @@ -118,13 +112,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.vdefines, definition.vincludes);
definition.vshader = Preprocessor.run(definition.vshader, 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, definition.fdefines, definition.fincludes, stripUnusedColorAttachments);
definition.fshader = Preprocessor.run(definition.fshader, definition.fincludes, stripUnusedColorAttachments);
}

this.impl = graphicsDevice.createShaderImpl(this);
Expand Down
8 changes: 4 additions & 4 deletions src/scene/shader-lib/programs/basic.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ class ShaderGeneratorBasic extends ShaderGenerator {
if (options.diffuseMap) defines.set('DIFFUSE_MAP', true);

definitionOptions.vertexCode = vShader;
definitionOptions.vertexIncludesMap = includes;
definitionOptions.vertexDefinesMap = defines;
definitionOptions.vertexIncludes = includes;
definitionOptions.vertexDefines = defines;
}

createFragmentDefinition(definitionOptions, options, shaderPassInfo) {
Expand All @@ -189,8 +189,8 @@ class ShaderGeneratorBasic extends ShaderGenerator {
if (options.alphaTest) defines.set('ALPHA_TEST', true);

definitionOptions.fragmentCode = fShader;
definitionOptions.fragmentIncludesMap = includes;
definitionOptions.fragmentDefinesMap = defines;
definitionOptions.fragmentIncludes = includes;
definitionOptions.fragmentDefines = defines;
}

createShaderDefinition(device, options) {
Expand Down
42 changes: 20 additions & 22 deletions test/core/preprocessor.test.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ describe('Preprocessor', function () {
['inc2', 'block2']
]);

const defines = new Map();

const srcData = `
#define FEATURE1
Expand Down Expand Up @@ -78,82 +76,82 @@ describe('Preprocessor', function () {
`;

it('returns false for MORPH_A', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('MORPH_A')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('MORPH_A')).to.equal(false);
});

it('returns false for MORPH_B', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('MORPH_B')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('MORPH_B')).to.equal(false);
});

it('returns true for $', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('$')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('$')).to.equal(true);
});

it('returns true for TEST1', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST1')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST1')).to.equal(true);
});

it('returns true for TEST2', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST2')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST2')).to.equal(true);
});

it('returns true for TEST3', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST3')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST3')).to.equal(true);
});

it('returns true for TEST4', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST4')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST4')).to.equal(true);
});

it('returns false for TEST5', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST5')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST5')).to.equal(false);
});

it('returns true for TEST6', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST6')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST6')).to.equal(true);
});

it('returns false for TEST7', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST7')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST7')).to.equal(false);
});

it('returns false for TEST8', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST8')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST8')).to.equal(false);
});

it('returns false for TEST9', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST9')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST9')).to.equal(false);
});

it('returns true for TEST10', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST10')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST10')).to.equal(true);
});

it('returns false for TEST11', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST11')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST11')).to.equal(false);
});

it('returns false for TEST12', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST12')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST12')).to.equal(false);
});

it('returns true for TEST13', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST13')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('TEST13')).to.equal(true);
});

it('returns false for TEST14', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('TEST14')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('TEST14')).to.equal(false);
});

it('returns true for INC1', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('block1')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('block1')).to.equal(true);
});

it('returns false for INC2', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('block2')).to.equal(false);
expect(Preprocessor.run(srcData, includes).includes('block2')).to.equal(false);
});

it('returns true for nested', function () {
expect(Preprocessor.run(srcData, defines, includes).includes('nested')).to.equal(true);
expect(Preprocessor.run(srcData, includes).includes('nested')).to.equal(true);
});
});

0 comments on commit 2a136a0

Please sign in to comment.