Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Shader generators are converted to modules #5552

Merged
merged 2 commits into from
Aug 17, 2023
Merged

Shader generators are converted to modules #5552

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8489635
Shader generators are converted to modules
Aug 4, 2023
cdcf8c5
jsdocs + cleanup
Aug 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/deprecated/deprecated.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
TEXTURETYPE_DEFAULT, TEXTURETYPE_RGBM, TEXTURETYPE_SWIZZLEGGGR,
TYPE_INT8, TYPE_UINT8, TYPE_INT16, TYPE_UINT16, TYPE_INT32, TYPE_UINT32, TYPE_FLOAT32
} from '../platform/graphics/constants.js';
import { begin, end, fogCode, gammaCode, skinCode, tonemapCode } from '../scene/shader-lib/programs/common.js';
import { ShaderGenerator } from '../scene/shader-lib/programs/shader-generator.js';
import { drawQuadWithShader } from '../scene/graphics/quad-render-utils.js';
import { shaderChunks } from '../scene/shader-lib/chunks/chunks.js';
import { GraphicsDevice } from '../platform/graphics/graphics-device.js';
Expand Down Expand Up @@ -369,14 +369,14 @@ export function ContextCreationError(message) {
ContextCreationError.prototype = Error.prototype;

export const programlib = {
begin: begin,
begin: ShaderGenerator.begin,
dummyFragmentCode: ShaderUtils.dummyFragmentCode,
end: end,
fogCode: fogCode,
gammaCode: gammaCode,
end: ShaderGenerator.end,
fogCode: ShaderGenerator.fogCode,
gammaCode: ShaderGenerator.gammaCode,
precisionCode: ShaderUtils.precisionCode,
skinCode: skinCode,
tonemapCode: tonemapCode,
skinCode: ShaderGenerator.skinCode,
tonemapCode: ShaderGenerator.tonemapCode,
versionCode: ShaderUtils.versionCode
};

Expand Down
33 changes: 24 additions & 9 deletions src/scene/shader-lib/program-library.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,19 @@ class ProgramLibrary {
/**
* A cache of shader definitions before processing.
*
* @type {Map<string, object>}
* @type {Map<number, object>}
*/
definitionsCache = new Map();

/**
* Named shader generators.
*
* @type {Map<string, import('./programs/shader-generator.js').ShaderGenerator>}
*/
_generators = new Map();

constructor(device, standardMaterial) {
this._device = device;
this._generators = {};
this._isClearingCache = false;
this._precached = false;

Expand All @@ -57,22 +63,31 @@ class ProgramLibrary {
}

register(name, generator) {
if (!this.isRegistered(name)) {
this._generators[name] = generator;
if (!this._generators.has(name)) {
this._generators.set(name, generator);
}
}

unregister(name) {
if (this.isRegistered(name)) {
delete this._generators[name];
if (this._generators.has(name)) {
this._generators.delete(name);
}
}

isRegistered(name) {
const generator = this._generators[name];
return (generator !== undefined);
return this._generators.has(name);
}

/**
* Returns a generated shader definition for the specified options. They key is used to cache the
* shader definition.
* @param {import('./programs/shader-generator.js').ShaderGenerator} generator - The generator
* to use.
* @param {string} name - The unique name of the shader generator.
* @param {number} key - A unique key representing the shader options.
* @param {object} options - The shader options.
* @returns {object} - The shader definition.
*/
generateShaderDefinition(generator, name, key, options) {
let def = this.definitionsCache.get(key);
if (!def) {
Expand Down Expand Up @@ -112,7 +127,7 @@ class ProgramLibrary {
}

getProgram(name, options, processingOptions, userMaterialId) {
const generator = this._generators[name];
const generator = this._generators.get(name);
if (!generator) {
Debug.warn(`ProgramLibrary#getProgram: No program library functions registered for: ${name}`);
return null;
Expand Down
26 changes: 14 additions & 12 deletions src/scene/shader-lib/programs/basic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import {
} from '../../constants.js';
import { ShaderPass } from '../../shader-pass.js';

import { begin, end, fogCode, skinCode } from './common.js';
import { ShaderGenerator } from './shader-generator.js';

const basic = {
generateKey: function (options) {
class ShaderGeneratorBasic extends ShaderGenerator {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this extends?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To give all those classes base class / shared type information.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see that used anywhere?

The structure of this whole class hierarchy is very non-standard (and for me confusing).

ShaderGenerator isn't a real base class because it defines no base functionality. Rather it is a set of static helper functions.

Then ShaderGeneratorBasic and the rest implement what looks like a consistent set of functions (which presumably program-libraray expects, but never defines).

Ultimately the structure/interface of ShaderGenerator isn't defined anywhere and is left as implicit.

I don't think there needs to be any inheritance here and probably program-library should define what structure a 'generator' takes. Each implementation of generator just implements that interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point I'm going to add type information to the program-library, and this is the type of the generator. Ideally this would be an interface, but unfortunately JS does not have a concept of this. But this is the only way to assign a single (base) type to multiple classes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If program-library just uses duck typing on generators, why do generators need a class hierarchy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some JSDocs to the program-libray that use the base class type - type safety is where we're going to with the engine changes.

generateKey(options) {
let key = 'basic';
if (options.fog) key += '_fog';
if (options.alphaTest) key += '_atst';
Expand All @@ -28,9 +28,9 @@ const basic = {

key += '_' + options.pass;
return key;
},
}

createShaderDefinition: function (device, options) {
createShaderDefinition(device, options) {
// GENERATE ATTRIBUTES
const attributes = {
vertex_position: SEMANTIC_POSITION
Expand All @@ -56,7 +56,7 @@ const basic = {
vshader += shaderChunks.transformDeclVS;

if (options.skin) {
vshader += skinCode(device);
vshader += ShaderGenerator.skinCode(device);
vshader += shaderChunks.transformSkinnedVS;
} else {
vshader += shaderChunks.transformVS;
Expand Down Expand Up @@ -84,7 +84,7 @@ const basic = {
}

// VERTEX SHADER BODY
vshader += begin();
vshader += ShaderGenerator.begin();

vshader += " gl_Position = getPosition();\n";

Expand All @@ -99,7 +99,7 @@ const basic = {
vshader += ' vUv0 = vertex_texCoord0;\n';
}

vshader += end();
vshader += ShaderGenerator.end();

// GENERATE FRAGMENT SHADER
let fshader = shaderPassDefines;
Expand All @@ -115,7 +115,7 @@ const basic = {
fshader += 'uniform sampler2D texture_diffuseMap;\n';
}
if (options.fog) {
fshader += fogCode(options.fog);
fshader += ShaderGenerator.fogCode(options.fog);
}
if (options.alphaTest) {
fshader += shaderChunks.alphaTestPS;
Expand All @@ -128,7 +128,7 @@ const basic = {
}

// FRAGMENT SHADER BODY
fshader += begin();
fshader += ShaderGenerator.begin();

// Read the map texels that the shader needs
if (options.vertexColors) {
Expand Down Expand Up @@ -156,7 +156,7 @@ const basic = {
}
}

fshader += end();
fshader += ShaderGenerator.end();

return ShaderUtils.createDefinition(device, {
name: 'BasicShader',
Expand All @@ -165,6 +165,8 @@ const basic = {
fragmentCode: fshader
});
}
};
}

const basic = new ShaderGeneratorBasic();

export { basic };
62 changes: 0 additions & 62 deletions src/scene/shader-lib/programs/common.js
View file
Open in desktop

This file was deleted.

24 changes: 12 additions & 12 deletions src/scene/shader-lib/programs/lit-shader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import {
import { LightsBuffer } from '../../lighting/lights-buffer.js';
import { ShaderPass } from '../../shader-pass.js';

import { begin, end, fogCode, gammaCode, skinCode, tonemapCode } from './common.js';
import { validateUserChunks } from '../chunks/chunk-validation.js';
import { ShaderUtils } from '../../../platform/graphics/shader-utils.js';
import { ChunkBuilder } from '../chunk-builder.js';
import { ShaderGenerator } from './shader-generator.js';

const builtinAttributes = {
vertex_normal: SEMANTIC_NORMAL,
Expand Down Expand Up @@ -363,7 +363,7 @@ class LitShader {
if (options.skin) {
this.attributes.vertex_boneWeights = SEMANTIC_BLENDWEIGHT;
this.attributes.vertex_boneIndices = SEMANTIC_BLENDINDICES;
code += skinCode(device, chunks);
code += ShaderGenerator.skinCode(device, chunks);
code += "#define SKIN\n";
} else if (options.useInstancing) {
code += "#define INSTANCING\n";
Expand Down Expand Up @@ -417,10 +417,10 @@ class LitShader {
code += this.varyingDefines;
code += this.frontendDecl;
code += this.frontendCode;
code += begin();
code += ShaderGenerator.begin();
code += this.frontendFunc;
code += " gl_FragColor = uColor;\n";
code += end();
code += ShaderGenerator.end();
return code;
}

Expand All @@ -435,10 +435,10 @@ class LitShader {
code += chunks.packDepthPS;
code += this.frontendDecl;
code += this.frontendCode;
code += begin();
code += ShaderGenerator.begin();
code += this.frontendFunc;
code += " gl_FragColor = packFloat(vDepth);\n";
code += end();
code += ShaderGenerator.end();

return code;
}
Expand Down Expand Up @@ -503,7 +503,7 @@ class LitShader {
code += shaderChunks.linearizeDepthPS;
}

code += begin();
code += ShaderGenerator.begin();

code += this.frontendFunc;

Expand Down Expand Up @@ -552,7 +552,7 @@ class LitShader {
code += chunks.storeEVSMPS;
}

code += end();
code += ShaderGenerator.end();

return code;
}
Expand Down Expand Up @@ -737,9 +737,9 @@ class LitShader {
// FIXME: only add these when needed
func.append(chunks.sphericalPS);
func.append(chunks.decodePS);
func.append(gammaCode(options.gamma, chunks));
func.append(tonemapCode(options.toneMap, chunks));
func.append(fogCode(options.fog, chunks));
func.append(ShaderGenerator.gammaCode(options.gamma, chunks));
func.append(ShaderGenerator.tonemapCode(options.toneMap, chunks));
func.append(ShaderGenerator.fogCode(options.fog, chunks));

// frontend
func.append(this.frontendCode);
Expand Down Expand Up @@ -1504,7 +1504,7 @@ class LitShader {

code.append(" evaluateBackend();");

code.append(end());
code.append(ShaderGenerator.end());

const mergedCode = decl.code + func.code + code.code;

Expand Down
18 changes: 8 additions & 10 deletions src/scene/shader-lib/programs/lit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { ChunkBuilder } from '../chunk-builder.js';
import { LitShader } from './lit-shader.js';
import { LitOptionsUtils } from './lit-options-utils.js';
import { ShaderGenerator } from './shader-generator.js';

const dummyUvs = [0, 1, 2, 3, 4, 5, 6, 7];

/**
* @ignore
*/
const lit = {

/** @type {Function} */
generateKey: function (options) {
class ShaderGeneratorLit extends ShaderGenerator {
generateKey(options) {
const key = "lit" +
dummyUvs.map((dummy, index) => {
return options.usedUvs[index] ? "1" : "0";
Expand All @@ -19,7 +15,7 @@ const lit = {
LitOptionsUtils.generateKey(options.litOptions);

return key;
},
}

/**
* @param {import('../../../platform/graphics/graphics-device.js').GraphicsDevice} device - The
Expand All @@ -28,7 +24,7 @@ const lit = {
* @returns {object} Returns the created shader definition.
* @ignore
*/
createShaderDefinition: function (device, options) {
createShaderDefinition(device, options) {
const litShader = new LitShader(device, options.litOptions);

const decl = new ChunkBuilder();
Expand All @@ -50,6 +46,8 @@ const lit = {

return litShader.getDefinition();
}
};
}

const lit = new ShaderGeneratorLit();

export { lit };