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.

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

Replace push.apply with dedicated function #12361

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions packages/engine/Source/Core/addAllToArray.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import defined from "./defined.js";

/**
* Adds all elements from the given source array to the given target array.
*
* If the <code>source</code> is <code>null</code>, <code>undefined</code>,
* or empty, then nothing will be done. Otherwise, this has the same
* semantics as<br>
* <code>for (const s of source) target.push(s);</code><br>
* but is usually more efficient than a <code>for</code>-loop, and does not
* put the elements of the source on the stack, as it would be done with the
* spread operator or when using <code>target.push.apply(source)</code>.
*
* @function
* @private
*
* @param {Array} target The target array
* @param {Array|undefined} source The source array
*
* @example
* const target = [ 0, 1, 2 ];
* const source = [ 3, 4, 5 ];
* Cesium.addAllToArray(target, source);
* // The target is now [ 0, 1, 2, 3, 4, 5 ]
*/
function addAllToArray(target, source) {
if (!defined(source)) {
return;
}
const sourceLength = source.length;
if (sourceLength === 0) {
return;
}
const targetLength = target.length;
target.length += sourceLength;
for (let i = 0; i < sourceLength; i++) {
target[targetLength + i] = source[i];
}
}
export default addAllToArray;
13 changes: 7 additions & 6 deletions packages/engine/Source/Renderer/ShaderBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ShaderProgram from "./ShaderProgram.js";
import ShaderSource from "./ShaderSource.js";
import ShaderStruct from "./ShaderStruct.js";
import ShaderFunction from "./ShaderFunction.js";
import addAllToArray from "../Core/addAllToArray.js";

/**
* An object that makes it easier to build the text of a {@link ShaderProgram}. This tracks GLSL code for both the vertex shader and the fragment shader.
Expand Down Expand Up @@ -414,7 +415,7 @@ ShaderBuilder.prototype.addVertexLines = function (lines) {

const vertexLines = this._vertexShaderParts.shaderLines;
if (Array.isArray(lines)) {
vertexLines.push.apply(vertexLines, lines);
addAllToArray(vertexLines, lines);
} else {
// Single string case
vertexLines.push(lines);
Expand Down Expand Up @@ -449,7 +450,7 @@ ShaderBuilder.prototype.addFragmentLines = function (lines) {

const fragmentLines = this._fragmentShaderParts.shaderLines;
if (Array.isArray(lines)) {
fragmentLines.push.apply(fragmentLines, lines);
addAllToArray(fragmentLines, lines);
} else {
// Single string case
fragmentLines.push(lines);
Expand Down Expand Up @@ -534,15 +535,15 @@ function generateStructLines(shaderBuilder) {
structId = structIds[i];
struct = shaderBuilder._structs[structId];
structLines = struct.generateGlslLines();
vertexLines.push.apply(vertexLines, structLines);
addAllToArray(vertexLines, structLines);
}

structIds = shaderBuilder._fragmentShaderParts.structIds;
for (i = 0; i < structIds.length; i++) {
structId = structIds[i];
struct = shaderBuilder._structs[structId];
structLines = struct.generateGlslLines();
fragmentLines.push.apply(fragmentLines, structLines);
addAllToArray(fragmentLines, structLines);
}

return {
Expand Down Expand Up @@ -577,15 +578,15 @@ function generateFunctionLines(shaderBuilder) {
functionId = functionIds[i];
func = shaderBuilder._functions[functionId];
functionLines = func.generateGlslLines();
vertexLines.push.apply(vertexLines, functionLines);
addAllToArray(vertexLines, functionLines);
}

functionIds = shaderBuilder._fragmentShaderParts.functionIds;
for (i = 0; i < functionIds.length; i++) {
functionId = functionIds[i];
func = shaderBuilder._functions[functionId];
functionLines = func.generateGlslLines();
fragmentLines.push.apply(fragmentLines, functionLines);
addAllToArray(fragmentLines, functionLines);
}

return {
Expand Down
10 changes: 6 additions & 4 deletions packages/engine/Source/Scene/Cesium3DTileBatchTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import getBinaryAccessor from "./getBinaryAccessor.js";
import StencilConstants from "./StencilConstants.js";
import StencilFunction from "./StencilFunction.js";
import StencilOperation from "./StencilOperation.js";
import addAllToArray from "../Core/addAllToArray.js";

const DEFAULT_COLOR_VALUE = BatchTexture.DEFAULT_COLOR_VALUE;
const DEFAULT_SHOW_VALUE = BatchTexture.DEFAULT_SHOW_VALUE;
Expand Down Expand Up @@ -372,13 +373,14 @@ Cesium3DTileBatchTable.prototype.getPropertyIds = function (batchId, results) {
results.length = 0;

const scratchPropertyIds = Object.keys(this._properties);
results.push.apply(results, scratchPropertyIds);
addAllToArray(results, scratchPropertyIds);

if (defined(this._batchTableHierarchy)) {
results.push.apply(
results,
this._batchTableHierarchy.getPropertyIds(batchId, scratchPropertyIds),
const propertyIds = this._batchTableHierarchy.getPropertyIds(
batchId,
scratchPropertyIds,
);
addAllToArray(results, propertyIds);
}

return results;
Expand Down
7 changes: 4 additions & 3 deletions packages/engine/Source/Scene/Cesium3DTileStyle.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import addAllToArray from "../Core/addAllToArray.js";
import clone from "../Core/clone.js";
import defaultValue from "../Core/defaultValue.js";
import defined from "../Core/defined.js";
Expand Down Expand Up @@ -1455,15 +1456,15 @@ Cesium3DTileStyle.prototype.getVariables = function () {
let variables = [];

if (defined(this.color) && defined(this.color.getVariables)) {
variables.push.apply(variables, this.color.getVariables());
addAllToArray(variables, this.color.getVariables());
}

if (defined(this.show) && defined(this.show.getVariables)) {
variables.push.apply(variables, this.show.getVariables());
addAllToArray(variables, this.show.getVariables());
}

if (defined(this.pointSize) && defined(this.pointSize.getVariables)) {
variables.push.apply(variables, this.pointSize.getVariables());
addAllToArray(variables, this.pointSize.getVariables());
}

// Remove duplicates
Expand Down
5 changes: 3 additions & 2 deletions packages/engine/Source/Scene/ConditionsExpression.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import addAllToArray from "../Core/addAllToArray.js";
import clone from "../Core/clone.js";
import defined from "../Core/defined.js";
import Expression from "./Expression.js";
Expand Down Expand Up @@ -203,8 +204,8 @@ ConditionsExpression.prototype.getVariables = function () {
const length = conditions.length;
for (let i = 0; i < length; ++i) {
const statement = conditions[i];
variables.push.apply(variables, statement.condition.getVariables());
variables.push.apply(variables, statement.expression.getVariables());
addAllToArray(variables, statement.condition.getVariables());
addAllToArray(variables, statement.expression.getVariables());
}

// Remove duplicates
Expand Down
5 changes: 3 additions & 2 deletions packages/engine/Source/Scene/GltfLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import VertexAttributeSemantic from "./VertexAttributeSemantic.js";
import GltfGpmLoader from "./Model/Extensions/Gpm/GltfGpmLoader.js";
import GltfMeshPrimitiveGpmLoader from "./Model/Extensions/Gpm/GltfMeshPrimitiveGpmLoader.js";
import oneTimeWarning from "../Core/oneTimeWarning.js";
import addAllToArray from "../Core/addAllToArray.js";

const {
Attribute,
Expand Down Expand Up @@ -2749,12 +2750,12 @@ function parse(loader, frameState) {

// Gather promises and handle any errors
const readyPromises = [];
readyPromises.push.apply(readyPromises, loader._loaderPromises);
addAllToArray(readyPromises, loader._loaderPromises);

// When incrementallyLoadTextures is true, the errors are caught and thrown individually
// since it doesn't affect the overall loader state
if (!loader._incrementallyLoadTextures) {
readyPromises.push.apply(readyPromises, loader._texturesPromises);
addAllToArray(readyPromises, loader._texturesPromises);
}

return Promise.all(readyPromises);
Expand Down
3 changes: 2 additions & 1 deletion packages/engine/Source/Scene/MetadataTableProperty.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import oneTimeWarning from "../Core/oneTimeWarning.js";
import MetadataComponentType from "./MetadataComponentType.js";
import MetadataClassProperty from "./MetadataClassProperty.js";
import MetadataType from "./MetadataType.js";
import addAllToArray from "../Core/addAllToArray.js";

/**
* A binary property in a {@MetadataTable}
Expand Down Expand Up @@ -388,7 +389,7 @@ function flatten(values) {
for (let i = 0; i < values.length; i++) {
const value = values[i];
if (Array.isArray(value)) {
result.push.apply(result, value);
addAllToArray(result, value);
} else {
result.push(value);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import addAllToArray from "../../Core/addAllToArray.js";
import BoundingSphere from "../../Core/BoundingSphere.js";
import Check from "../../Core/Check.js";
import defaultValue from "../../Core/defaultValue.js";
Expand Down Expand Up @@ -482,16 +483,16 @@ ClassificationModelDrawCommand.prototype.pushCommands = function (
const passes = frameState.passes;
if (passes.render) {
if (this._useDebugWireframe) {
result.push.apply(result, this._commandListDebugWireframe);
addAllToArray(result, this._commandListDebugWireframe);
return;
}

if (this._classifiesTerrain) {
result.push.apply(result, this._commandListTerrain);
addAllToArray(result, this._commandListTerrain);
}

if (this._classifies3DTiles) {
result.push.apply(result, this._commandList3DTiles);
addAllToArray(result, this._commandList3DTiles);
}

const useIgnoreShowCommands =
Expand All @@ -513,17 +514,17 @@ ClassificationModelDrawCommand.prototype.pushCommands = function (
);
}

result.push.apply(result, this._commandListIgnoreShow);
addAllToArray(result, this._commandListIgnoreShow);
}
}

if (passes.pick) {
if (this._classifiesTerrain) {
result.push.apply(result, this._commandListTerrainPicking);
addAllToArray(result, this._commandListTerrainPicking);
}

if (this._classifies3DTiles) {
result.push.apply(result, this._commandList3DTilesPicking);
addAllToArray(result, this._commandList3DTilesPicking);
}
}

Expand Down
4 changes: 3 additions & 1 deletion packages/engine/Source/Scene/Model/GeoJsonLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import StructuralMetadata from "../StructuralMetadata.js";
import VertexAttributeSemantic from "../VertexAttributeSemantic.js";
import Buffer from "../../Renderer/Buffer.js";
import BufferUsage from "../../Renderer/BufferUsage.js";
import addAllToArray from "../../Core/addAllToArray.js";

/**
* Loads a GeoJson model as part of the <code>MAXAR_content_geojson</code> extension with the following constraints:
Expand Down Expand Up @@ -166,7 +167,8 @@ function parseMultiPolygon(coordinates) {
const polygonsLength = coordinates.length;
const lines = [];
for (let i = 0; i < polygonsLength; i++) {
Array.prototype.push.apply(lines, parsePolygon(coordinates[i]));
const polygon = parsePolygon(coordinates[i]);
addAllToArray(lines, polygon);
}
return lines;
}
Expand Down
11 changes: 3 additions & 8 deletions packages/engine/Source/Scene/Model/InstancingPipelineStage.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import addAllToArray from "../../Core/addAllToArray.js";
import AttributeCompression from "../../Core/AttributeCompression.js";
import Cartesian3 from "../../Core/Cartesian3.js";
import clone from "../../Core/clone.js";
Expand Down Expand Up @@ -210,10 +211,7 @@ InstancingPipelineStage.process = function (renderResources, node, frameState) {
renderResources.uniformMap = combine(uniformMap, renderResources.uniformMap);

renderResources.instanceCount = count;
renderResources.attributes.push.apply(
renderResources.attributes,
instancingVertexAttributes,
);
addAllToArray(renderResources.attributes, instancingVertexAttributes);
};

const projectedTransformScratch = new Matrix4();
Expand Down Expand Up @@ -988,10 +986,7 @@ function processMatrixAttributes(
shaderBuilder.addAttribute("vec4", `a_instancing${attributeString}Row1`);
shaderBuilder.addAttribute("vec4", `a_instancing${attributeString}Row2`);

instancingVertexAttributes.push.apply(
instancingVertexAttributes,
matrixAttributes,
);
addAllToArray(instancingVertexAttributes, matrixAttributes);
}

function processVec3Attribute(
Expand Down
3 changes: 2 additions & 1 deletion packages/engine/Source/Scene/Model/ModelSceneGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import ModelType from "./ModelType.js";
import NodeRenderResources from "./NodeRenderResources.js";
import PrimitiveRenderResources from "./PrimitiveRenderResources.js";
import ModelDrawCommands from "./ModelDrawCommands.js";
import addAllToArray from "../../Core/addAllToArray.js";

/**
* An in memory representation of the scene graph for a {@link Model}
Expand Down Expand Up @@ -912,7 +913,7 @@ ModelSceneGraph.prototype.pushDrawCommands = function (frameState) {
pushDrawCommandOptions,
);

frameState.commandList.push.apply(frameState.commandList, silhouetteCommands);
addAllToArray(frameState.commandList, silhouetteCommands);
};

// Callback is defined here to avoid allocating a closure in the render loop
Expand Down
19 changes: 7 additions & 12 deletions packages/engine/Source/Scene/PropertyTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import defaultValue from "../Core/defaultValue.js";
import DeveloperError from "../Core/DeveloperError.js";
import defined from "../Core/defined.js";
import JsonMetadataTable from "./JsonMetadataTable.js";
import addAllToArray from "../Core/addAllToArray.js";

/**
* A property table for use with the <code>EXT_structural_metadata</code> extension or
Expand Down Expand Up @@ -296,24 +297,18 @@ PropertyTable.prototype.getPropertyIds = function (index, results) {

if (defined(this._metadataTable)) {
// concat in place to avoid unnecessary array allocation
results.push.apply(
results,
this._metadataTable.getPropertyIds(scratchResults),
);
const ids = this._metadataTable.getPropertyIds(scratchResults);
addAllToArray(results, ids);
}

if (defined(this._batchTableHierarchy)) {
results.push.apply(
results,
this._batchTableHierarchy.getPropertyIds(index, scratchResults),
);
const ids = this._batchTableHierarchy.getPropertyIds(index, scratchResults);
addAllToArray(results, ids);
}

if (defined(this._jsonMetadataTable)) {
results.push.apply(
results,
this._jsonMetadataTable.getPropertyIds(scratchResults),
);
const ids = this._jsonMetadataTable.getPropertyIds(scratchResults);
addAllToArray(results, ids);
}

return results;
Expand Down
30 changes: 30 additions & 0 deletions packages/engine/Specs/Core/addAllSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { addAllToArray } from "../../index.js";

describe("Core/addAllToArray", function () {
it("works for basic arrays", function () {
const source = [3, 4, 5];
const target = [0, 1, 2];
addAllToArray(target, source);
expect(target).toEqual([0, 1, 2, 3, 4, 5]);
});

it("works for null source", function () {
const target = [0, 1, 2];
addAllToArray(target, null);
expect(target).toEqual([0, 1, 2]);
});

it("works for undefined source", function () {
const target = [0, 1, 2];
addAllToArray(target, undefined);
expect(target).toEqual([0, 1, 2]);
});

it("works for large arrays", function () {
const source = Array(200000);
const target = Array(200000);
const result = Array(400000);
addAllToArray(target, source);
expect(target).toEqual(result);
});
});
Loading
Loading