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 12 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
Change addAll to addAllToArray, based on review
javagl committed Dec 13, 2024
commit 637288044b1c0c844f0bb13ad8f72b5fc8e8bb64
41 changes: 0 additions & 41 deletions packages/engine/Source/Core/addAll.js

This file was deleted.

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;
14 changes: 7 additions & 7 deletions packages/engine/Source/Renderer/ShaderBuilder.js
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import ShaderProgram from "./ShaderProgram.js";
import ShaderSource from "./ShaderSource.js";
import ShaderStruct from "./ShaderStruct.js";
import ShaderFunction from "./ShaderFunction.js";
import addAll from "../Core/addAll.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.
@@ -415,7 +415,7 @@ ShaderBuilder.prototype.addVertexLines = function (lines) {

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

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

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

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

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

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

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

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

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

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

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

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

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

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

// Remove duplicates
6 changes: 3 additions & 3 deletions packages/engine/Source/Scene/GltfLoader.js
Original file line number Diff line number Diff line change
@@ -32,7 +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 addAll from "../Core/addAll.js";
import addAllToArray from "../Core/addAllToArray.js";

const {
Attribute,
@@ -2750,12 +2750,12 @@ function parse(loader, frameState) {

// Gather promises and handle any errors
const readyPromises = [];
addAll(loader._loaderPromises, readyPromises);
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) {
addAll(loader._texturesPromises, readyPromises);
addAllToArray(readyPromises, loader._texturesPromises);
}

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

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

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

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

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

addAll(this._commandListIgnoreShow, result);
addAllToArray(result, this._commandListIgnoreShow);
}
}

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

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

4 changes: 2 additions & 2 deletions packages/engine/Source/Scene/Model/GeoJsonLoader.js
Original file line number Diff line number Diff line change
@@ -19,7 +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 addAll from "../../Core/addAll.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:
@@ -168,7 +168,7 @@ function parseMultiPolygon(coordinates) {
const lines = [];
for (let i = 0; i < polygonsLength; i++) {
const polygon = parsePolygon(coordinates[i]);
addAll(polygon, lines);
addAllToArray(lines, polygon);
}
return lines;
}
6 changes: 3 additions & 3 deletions packages/engine/Source/Scene/Model/InstancingPipelineStage.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import addAll from "../../Core/addAll.js";
import addAllToArray from "../../Core/addAllToArray.js";
import AttributeCompression from "../../Core/AttributeCompression.js";
import Cartesian3 from "../../Core/Cartesian3.js";
import clone from "../../Core/clone.js";
@@ -211,7 +211,7 @@ InstancingPipelineStage.process = function (renderResources, node, frameState) {
renderResources.uniformMap = combine(uniformMap, renderResources.uniformMap);

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

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

addAll(matrixAttributes, instancingVertexAttributes);
addAllToArray(instancingVertexAttributes, matrixAttributes);
}

function processVec3Attribute(
4 changes: 2 additions & 2 deletions packages/engine/Source/Scene/Model/ModelSceneGraph.js
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@ import ModelType from "./ModelType.js";
import NodeRenderResources from "./NodeRenderResources.js";
import PrimitiveRenderResources from "./PrimitiveRenderResources.js";
import ModelDrawCommands from "./ModelDrawCommands.js";
import addAll from "../../Core/addAll.js";
import addAllToArray from "../../Core/addAllToArray.js";

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

addAll(silhouetteCommands, frameState.commandList);
addAllToArray(frameState.commandList, silhouetteCommands);
};

// Callback is defined here to avoid allocating a closure in the render loop
8 changes: 4 additions & 4 deletions packages/engine/Source/Scene/PropertyTable.js
Original file line number Diff line number Diff line change
@@ -3,7 +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 addAll from "../Core/addAll.js";
import addAllToArray from "../Core/addAllToArray.js";

/**
* A property table for use with the <code>EXT_structural_metadata</code> extension or
@@ -298,17 +298,17 @@ PropertyTable.prototype.getPropertyIds = function (index, results) {
if (defined(this._metadataTable)) {
// concat in place to avoid unnecessary array allocation
const ids = this._metadataTable.getPropertyIds(scratchResults);
addAll(ids, results);
addAllToArray(results, ids);
}

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

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

return results;
Loading