Skip to content

Commit 79fcbd1

Browse files
j9liuj9liu
authored
Merge pull request #10719 from CesiumGS/shader-builder-single-line
Minimize temporary arrays when using ShaderBuilder
2 parents b6399a8 + 68a2f6b commit 79fcbd1

23 files changed

+169
-54
lines changed

‎Source/Renderer/ShaderBuilder.js

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ ShaderBuilder.prototype.addFunction = function (
234234
/**
235235
* Add lines to a dynamically-generated function
236236
* @param {String} functionName The name of the function. This must be created beforehand using {@link ShaderBuilder#addFunction}
237-
* @param {String[]} lines An array of lines of GLSL code to add to the function body. Do not include any preceding or ending whitespace, but do include the semicolon for each line.
237+
* @param {String|String[]} lines One or more lines of GLSL code to add to the function body. Do not include any preceding or ending whitespace, but do include the semicolon for each line.
238238
*
239239
* @example
240240
* // generates the following function in the vertex shader
@@ -252,7 +252,11 @@ ShaderBuilder.prototype.addFunction = function (
252252
ShaderBuilder.prototype.addFunctionLines = function (functionName, lines) {
253253
//>>includeStart('debug', pragmas.debug);
254254
Check.typeOf.string("functionName", functionName);
255-
Check.typeOf.object("lines", lines);
255+
if (typeof lines !== "string" && !Array.isArray(lines)) {
256+
throw new DeveloperError(
257+
`Expected lines to be a string or an array of strings, actual value was ${lines}`
258+
);
259+
}
256260
//>>includeEnd('debug');
257261
this._functions[functionName].addLines(lines);
258262
};
@@ -384,7 +388,7 @@ ShaderBuilder.prototype.addVarying = function (type, identifier) {
384388
/**
385389
* Appends lines of GLSL code to the vertex shader
386390
*
387-
* @param {String[]} lines The lines to add to the end of the vertex shader source
391+
* @param {String|String[]} lines One or more lines to add to the end of the vertex shader source
388392
*
389393
* @example
390394
* shaderBuilder.addVertexLines([
@@ -397,10 +401,20 @@ ShaderBuilder.prototype.addVarying = function (type, identifier) {
397401
*/
398402
ShaderBuilder.prototype.addVertexLines = function (lines) {
399403
//>>includeStart('debug', pragmas.debug);
400-
Check.typeOf.object("lines", lines);
404+
if (typeof lines !== "string" && !Array.isArray(lines)) {
405+
throw new DeveloperError(
406+
`Expected lines to be a string or an array of strings, actual value was ${lines}`
407+
);
408+
}
401409
//>>includeEnd('debug');
402410

403-
Array.prototype.push.apply(this._vertexShaderParts.shaderLines, lines);
411+
const vertexLines = this._vertexShaderParts.shaderLines;
412+
if (Array.isArray(lines)) {
413+
vertexLines.push.apply(vertexLines, lines);
414+
} else {
415+
// Single string case
416+
vertexLines.push(lines);
417+
}
404418
};
405419

406420
/**
@@ -422,9 +436,20 @@ ShaderBuilder.prototype.addVertexLines = function (lines) {
422436
*/
423437
ShaderBuilder.prototype.addFragmentLines = function (lines) {
424438
//>>includeStart('debug', pragmas.debug);
425-
Check.typeOf.object("lines", lines);
439+
if (typeof lines !== "string" && !Array.isArray(lines)) {
440+
throw new DeveloperError(
441+
`Expected lines to be a string or an array of strings, actual value was ${lines}`
442+
);
443+
}
426444
//>>includeEnd('debug');
427-
Array.prototype.push.apply(this._fragmentShaderParts.shaderLines, lines);
445+
446+
const fragmentLines = this._fragmentShaderParts.shaderLines;
447+
if (Array.isArray(lines)) {
448+
fragmentLines.push.apply(fragmentLines, lines);
449+
} else {
450+
// Single string case
451+
fragmentLines.push(lines);
452+
}
428453
};
429454

430455
/**

‎Source/Renderer/ShaderFunction.js

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import DeveloperError from "../Core/DeveloperError.js";
2+
13
/**
24
* A utility for dynamically-generating a GLSL function
35
*
@@ -28,14 +30,29 @@ function ShaderFunction(signature) {
2830
}
2931

3032
/**
31-
* Add an array of lines to the body of the function
32-
* @param {String[]} lines An array of lines of GLSL code to add to the function body. Do not include any preceding or ending whitespace, but do include the semicolon for each line.
33+
* Adds one or more lines to the body of the function
34+
* @param {String|String[]} lines One or more lines of GLSL code to add to the function body. Do not include any preceding or ending whitespace, but do include the semicolon for each line.
3335
*/
3436
ShaderFunction.prototype.addLines = function (lines) {
35-
const paddedLines = lines.map(function (line) {
36-
return ` ${line}`;
37-
});
38-
Array.prototype.push.apply(this.body, paddedLines);
37+
//>>includeStart('debug', pragmas.debug);
38+
if (typeof lines !== "string" && !Array.isArray(lines)) {
39+
throw new DeveloperError(
40+
`Expected lines to be a string or an array of strings, actual value was ${lines}`
41+
);
42+
}
43+
//>>includeEnd('debug');
44+
const body = this.body;
45+
46+
// Indent the body of the function by 4 spaces
47+
if (Array.isArray(lines)) {
48+
const length = lines.length;
49+
for (let i = 0; i < length; i++) {
50+
body.push(` ${lines[i]}`);
51+
}
52+
} else {
53+
// Single string case
54+
body.push(` ${lines}`);
55+
}
3956
};
4057

4158
/**

‎Source/Scene/Model/CPUStylingPipelineStage.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ CPUStylingPipelineStage.process = function (
4040
const model = renderResources.model;
4141
const shaderBuilder = renderResources.shaderBuilder;
4242

43-
shaderBuilder.addVertexLines([CPUStylingStageVS]);
44-
shaderBuilder.addFragmentLines([CPUStylingStageFS]);
43+
shaderBuilder.addVertexLines(CPUStylingStageVS);
44+
shaderBuilder.addFragmentLines(CPUStylingStageFS);
4545
shaderBuilder.addDefine("USE_CPU_STYLING", undefined, ShaderDestination.BOTH);
4646

4747
// These uniforms may have already been added by the ModelColorStage if a static

‎Source/Scene/Model/CustomShaderPipelineStage.js

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -603,27 +603,37 @@ function addFragmentLinesToShader(shaderBuilder, fragmentLines) {
603603
shaderBuilder.addFunctionLines(functionId, initializationLines);
604604
}
605605

606+
const scratchShaderLines = [];
607+
606608
function addLinesToShader(shaderBuilder, customShader, generatedCode) {
607609
const vertexLines = generatedCode.vertexLines;
610+
const shaderLines = scratchShaderLines;
611+
608612
if (vertexLines.enabled) {
609613
addVertexLinesToShader(shaderBuilder, vertexLines);
610614

611-
shaderBuilder.addVertexLines([
615+
shaderLines.length = 0;
616+
shaderLines.push(
612617
"#line 0",
613618
customShader.vertexShaderText,
614-
CustomShaderStageVS,
615-
]);
619+
CustomShaderStageVS
620+
);
621+
622+
shaderBuilder.addVertexLines(shaderLines);
616623
}
617624

618625
const fragmentLines = generatedCode.fragmentLines;
619626
if (fragmentLines.enabled) {
620627
addFragmentLinesToShader(shaderBuilder, fragmentLines);
621628

622-
shaderBuilder.addFragmentLines([
629+
shaderLines.length = 0;
630+
shaderLines.push(
623631
"#line 0",
624632
customShader.fragmentShaderText,
625-
CustomShaderStageFS,
626-
]);
633+
CustomShaderStageFS
634+
);
635+
636+
shaderBuilder.addFragmentLines(shaderLines);
627637
}
628638
}
629639

‎Source/Scene/Model/FeatureIdPipelineStage.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ FeatureIdPipelineStage.process = function (
6868
}
6969
processPrimitiveFeatureIds(renderResources, primitive, frameState);
7070

71-
shaderBuilder.addVertexLines([FeatureIdStageVS]);
72-
shaderBuilder.addFragmentLines([FeatureIdStageFS]);
71+
shaderBuilder.addVertexLines(FeatureIdStageVS);
72+
shaderBuilder.addFragmentLines(FeatureIdStageFS);
7373
};
7474

7575
function declareStructsAndFunctions(shaderBuilder) {

‎Source/Scene/Model/GeometryPipelineStage.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ GeometryPipelineStage.process = function (
196196
shaderBuilder.addDefine("PRIMITIVE_TYPE_POINTS");
197197
}
198198

199-
shaderBuilder.addVertexLines([GeometryStageVS]);
200-
shaderBuilder.addFragmentLines([GeometryStageFS]);
199+
shaderBuilder.addVertexLines(GeometryStageVS);
200+
shaderBuilder.addFragmentLines(GeometryStageFS);
201201
};
202202

203203
function processAttribute(

‎Source/Scene/Model/ImageBasedLightingPipelineStage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ ImageBasedLightingPipelineStage.process = function (
114114
);
115115
}
116116

117-
shaderBuilder.addFragmentLines([ImageBasedLightingStageFS]);
117+
shaderBuilder.addFragmentLines(ImageBasedLightingStageFS);
118118

119119
const uniformMap = {
120120
model_iblFactor: function () {

‎Source/Scene/Model/InstancingPipelineStage.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ InstancingPipelineStage.process = function (renderResources, node, frameState) {
5757

5858
const shaderBuilder = renderResources.shaderBuilder;
5959
shaderBuilder.addDefine("HAS_INSTANCING");
60-
shaderBuilder.addVertexLines([InstancingStageCommon]);
60+
shaderBuilder.addVertexLines(InstancingStageCommon);
6161

6262
const model = renderResources.model;
6363
const sceneGraph = model.sceneGraph;
@@ -168,9 +168,9 @@ InstancingPipelineStage.process = function (renderResources, node, frameState) {
168168
);
169169
};
170170

171-
shaderBuilder.addVertexLines([LegacyInstancingStageVS]);
171+
shaderBuilder.addVertexLines(LegacyInstancingStageVS);
172172
} else {
173-
shaderBuilder.addVertexLines([InstancingStageVS]);
173+
shaderBuilder.addVertexLines(InstancingStageVS);
174174
}
175175

176176
if (use2D) {

‎Source/Scene/Model/LightingPipelineStage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ LightingPipelineStage.process = function (renderResources, primitive) {
6868
);
6969
}
7070

71-
shaderBuilder.addFragmentLines([LightingStageFS]);
71+
shaderBuilder.addFragmentLines(LightingStageFS);
7272
};
7373

7474
export default LightingPipelineStage;

‎Source/Scene/Model/MaterialPipelineStage.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ MaterialPipelineStage.process = function (
122122
alphaOptions.alphaCutoff = material.alphaCutoff;
123123
}
124124

125-
shaderBuilder.addFragmentLines([MaterialStageFS]);
125+
shaderBuilder.addFragmentLines(MaterialStageFS);
126126

127127
if (material.doubleSided) {
128128
shaderBuilder.addDefine(

0 commit comments

Comments
 (0)