Replace push.apply with dedicated function

packages/engine/Source/Core/addAll.js

@@ -0,0 +1,41 @@
+import defined from "./defined";
+
+/**
+ * Adds all elements from the given source array to the given target array.
+ *
+ * If the `source` is `undefined`, then nothing will be done. Otherwise,
+ * this has the same semantics as
+ * ```
+ * for (const s of source) target.push(s);
+ * ```
+ * but is usually more efficient than a `for`-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 `target.push.apply(source)`.
+ *
+ * @function
+ * @private
+ *
+ * @param {Array|undefined} source The source array
+ * @param {Array} target The target array
+ *
+ * @example
+ * const target = [ 0, 1, 2 ];
+ * const source = [ 3, 4, 5 ];
+ * Cesium.addAll(source, target);
+ * // The target is now [ 0, 1, 2, 3, 4, 5 ]
+ */
+function addAll(source, target) {
+  if (!defined(source)) {
+    return;
+  }
+  const s = source.length;
+  if (s === 0) {
+    return;
+  }
+  const t = target.length;
+  target.length += s;
+  for (let i = 0; i < s; i++) {
+    target[t + i] = source[i];
+  }
+}
+export default addAll;

packages/engine/Source/Renderer/ShaderBuilder.js

@@ -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 addAll from "../Core/addAll.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.
@@ -414,7 +415,7 @@ ShaderBuilder.prototype.addVertexLines = function (lines) {
 
   const vertexLines = this._vertexShaderParts.shaderLines;
   if (Array.isArray(lines)) {
-    vertexLines.push.apply(vertexLines, lines);
+    addAll(lines, vertexLines);
   } else {
     // Single string case
     vertexLines.push(lines);
@@ -449,7 +450,7 @@ ShaderBuilder.prototype.addFragmentLines = function (lines) {
 
   const fragmentLines = this._fragmentShaderParts.shaderLines;
   if (Array.isArray(lines)) {
-    fragmentLines.push.apply(fragmentLines, lines);
+    addAll(lines, fragmentLines);
   } else {
     // Single string case
     fragmentLines.push(lines);
@@ -534,15 +535,15 @@ function generateStructLines(shaderBuilder) {
     structId = structIds[i];
     struct = shaderBuilder._structs[structId];
     structLines = struct.generateGlslLines();
-    vertexLines.push.apply(vertexLines, structLines);
+    addAll(structLines, vertexLines);
   }
 
   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);
+    addAll(structLines, fragmentLines);
   }
 
   return {
@@ -577,15 +578,15 @@ function generateFunctionLines(shaderBuilder) {
     functionId = functionIds[i];
     func = shaderBuilder._functions[functionId];
     functionLines = func.generateGlslLines();
-    vertexLines.push.apply(vertexLines, functionLines);
+    addAll(functionLines, vertexLines);
   }
 
   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);
+    addAll(functionLines, fragmentLines);
   }
 
   return {

packages/engine/Source/Scene/Cesium3DTileBatchTable.js

@@ -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 addAll from "../Core/addAll.js";
 
 const DEFAULT_COLOR_VALUE = BatchTexture.DEFAULT_COLOR_VALUE;
 const DEFAULT_SHOW_VALUE = BatchTexture.DEFAULT_SHOW_VALUE;
@@ -372,13 +373,14 @@ Cesium3DTileBatchTable.prototype.getPropertyIds = function (batchId, results) {
   results.length = 0;
 
   const scratchPropertyIds = Object.keys(this._properties);
-  results.push.apply(results, scratchPropertyIds);
+  addAll(scratchPropertyIds, results);
 
   if (defined(this._batchTableHierarchy)) {
-    results.push.apply(
-      results,
-      this._batchTableHierarchy.getPropertyIds(batchId, scratchPropertyIds),
+    const propertyIds = this._batchTableHierarchy.getPropertyIds(
+      batchId,
+      scratchPropertyIds,
     );
+    addAll(propertyIds, results);
   }
 
   return results;

packages/engine/Source/Scene/Cesium3DTileStyle.js

@@ -1,3 +1,4 @@
+import addAll from "../Core/addAll.js";
 import clone from "../Core/clone.js";
 import defaultValue from "../Core/defaultValue.js";
 import defined from "../Core/defined.js";
@@ -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());
+    addAll(this.color.getVariables(), variables);
   }
 
   if (defined(this.show) && defined(this.show.getVariables)) {
-    variables.push.apply(variables, this.show.getVariables());
+    addAll(this.show.getVariables(), variables);
   }
 
   if (defined(this.pointSize) && defined(this.pointSize.getVariables)) {
-    variables.push.apply(variables, this.pointSize.getVariables());
+    addAll(this.pointSize.getVariables(), variables);
   }
 
   // Remove duplicates

packages/engine/Source/Scene/ConditionsExpression.js

@@ -1,3 +1,4 @@
+import addAll from "../Core/addAll.js";
 import clone from "../Core/clone.js";
 import defined from "../Core/defined.js";
 import Expression from "./Expression.js";
@@ -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());
+    addAll(statement.condition.getVariables(), variables);
+    addAll(statement.expression.getVariables(), variables);
   }
 
   // Remove duplicates

packages/engine/Source/Scene/GltfLoader.js

@@ -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 addAll from "../Core/addAll.js";
 
 const {
   Attribute,
@@ -2749,12 +2750,12 @@ function parse(loader, frameState) {
 
   // Gather promises and handle any errors
   const readyPromises = [];
-  readyPromises.push.apply(readyPromises, loader._loaderPromises);
+  addAll(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);
+    addAll(readyPromises, loader._texturesPromises);
   }
 
   return Promise.all(readyPromises);

packages/engine/Source/Scene/MetadataTableProperty.js

@@ -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 addAll from "../Core/addAll.js";
 
 /**
  * A binary property in a {@MetadataTable}
@@ -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);
+      addAll(value, result);
     } else {
       result.push(value);
     }

packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js

@@ -1,3 +1,4 @@
+import addAll from "../../Core/addAll.js";
 import BoundingSphere from "../../Core/BoundingSphere.js";
 import Check from "../../Core/Check.js";
 import defaultValue from "../../Core/defaultValue.js";
@@ -482,16 +483,16 @@ ClassificationModelDrawCommand.prototype.pushCommands = function (
   const passes = frameState.passes;
   if (passes.render) {
     if (this._useDebugWireframe) {
-      result.push.apply(result, this._commandListDebugWireframe);
+      addAll(this._commandListDebugWireframe, result);
       return;
     }
 
     if (this._classifiesTerrain) {
-      result.push.apply(result, this._commandListTerrain);
+      addAll(this._commandListTerrain, result);
     }
 
     if (this._classifies3DTiles) {
-      result.push.apply(result, this._commandList3DTiles);
+      addAll(this._commandList3DTiles, result);
     }
 
     const useIgnoreShowCommands =
@@ -513,17 +514,17 @@ ClassificationModelDrawCommand.prototype.pushCommands = function (
         );
       }
 
-      result.push.apply(result, this._commandListIgnoreShow);
+      addAll(this._commandListIgnoreShow, result);
     }
   }
 
   if (passes.pick) {
     if (this._classifiesTerrain) {
-      result.push.apply(result, this._commandListTerrainPicking);
+      addAll(this._commandListTerrainPicking, result);
     }
 
     if (this._classifies3DTiles) {
-      result.push.apply(result, this._commandList3DTilesPicking);
+      addAll(this._commandList3DTilesPicking, result);
     }
   }
 

packages/engine/Source/Scene/Model/GeoJsonLoader.js

@@ -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 addAll from "../../Core/addAll.js";
 
 /**
  * Loads a GeoJson model as part of the <code>MAXAR_content_geojson</code> extension with the following constraints:
@@ -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]);
+    addAll(polygon, lines);
   }
   return lines;
 }

packages/engine/Source/Scene/Model/InstancingPipelineStage.js

@@ -1,3 +1,4 @@
+import addAll from "../../Core/addAll.js";
 import AttributeCompression from "../../Core/AttributeCompression.js";
 import Cartesian3 from "../../Core/Cartesian3.js";
 import clone from "../../Core/clone.js";
@@ -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,
-  );
+  addAll(instancingVertexAttributes, renderResources.attributes);
 };
 
 const projectedTransformScratch = new Matrix4();
@@ -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,
-  );
+  addAll(matrixAttributes, instancingVertexAttributes);
 }
 
 function processVec3Attribute(

packages/engine/Source/Scene/Model/ModelSceneGraph.js

@@ -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 addAll from "../../Core/addAll.js";
 
 /**
  * An in memory representation of the scene graph for a {@link Model}
@@ -912,7 +913,7 @@ ModelSceneGraph.prototype.pushDrawCommands = function (frameState) {
     pushDrawCommandOptions,
   );
 
-  frameState.commandList.push.apply(frameState.commandList, silhouetteCommands);
+  addAll(silhouetteCommands, frameState.commandList);
 };
 
 // Callback is defined here to avoid allocating a closure in the render loop

packages/engine/Source/Scene/PropertyTable.js

@@ -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 addAll from "../Core/addAll.js";
 
 /**
  * A property table for use with the <code>EXT_structural_metadata</code> extension or
@@ -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);
+    addAll(ids, results);
   }
 
   if (defined(this._batchTableHierarchy)) {
-    results.push.apply(
-      results,
-      this._batchTableHierarchy.getPropertyIds(index, scratchResults),
-    );
+    const ids = this._batchTableHierarchy.getPropertyIds(index, scratchResults);
+    addAll(ids, results);
   }
 
   if (defined(this._jsonMetadataTable)) {
-    results.push.apply(
-      results,
-      this._jsonMetadataTable.getPropertyIds(scratchResults),
-    );
+    const ids = this._jsonMetadataTable.getPropertyIds(scratchResults);
+    addAll(ids, results);
   }
 
   return results;

packages/engine/Specs/Core/addAllSpec.js

@@ -0,0 +1,30 @@
+import { addAll } from "../../index.js";
+
+describe("Core/addAll", function () {
+  it("works for basic arrays", function () {
+    const source = [3, 4, 5];
+    const target = [0, 1, 2];
+    addAll(source, target);
+    expect(target).toEqual([0, 1, 2, 3, 4, 5]);
+  });
+
+  it("works for null source", function () {
+    const target = [0, 1, 2];
+    addAll(null, target);
+    expect(target).toEqual([0, 1, 2]);
+  });
+
+  it("works for undefined source", function () {
+    const target = [0, 1, 2];
+    addAll(undefined, target);
+    expect(target).toEqual([0, 1, 2]);
+  });
+
+  it("works for large arrays", function () {
+    const source = Array(200000);
+    const target = Array(200000);
+    const result = Array(400000);
+    addAll(source, target);
+    expect(target).toEqual(result);
+  });
+});