diff --git a/packages/engine/Source/Core/addAllToArray.js b/packages/engine/Source/Core/addAllToArray.js new file mode 100644 index 000000000000..929e97fe0d30 --- /dev/null +++ b/packages/engine/Source/Core/addAllToArray.js @@ -0,0 +1,40 @@ +import defined from "./defined.js"; + +/** + * Adds all elements from the given source array to the given target array. + * + * If the source is null, undefined, + * or empty, 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} 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; diff --git a/packages/engine/Source/Renderer/ShaderBuilder.js b/packages/engine/Source/Renderer/ShaderBuilder.js index 95fd920d5769..48fb8cb2267a 100644 --- a/packages/engine/Source/Renderer/ShaderBuilder.js +++ b/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 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. @@ -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); @@ -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); @@ -534,7 +535,7 @@ 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; @@ -542,7 +543,7 @@ function generateStructLines(shaderBuilder) { structId = structIds[i]; struct = shaderBuilder._structs[structId]; structLines = struct.generateGlslLines(); - fragmentLines.push.apply(fragmentLines, structLines); + addAllToArray(fragmentLines, structLines); } return { @@ -577,7 +578,7 @@ 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; @@ -585,7 +586,7 @@ function generateFunctionLines(shaderBuilder) { functionId = functionIds[i]; func = shaderBuilder._functions[functionId]; functionLines = func.generateGlslLines(); - fragmentLines.push.apply(fragmentLines, functionLines); + addAllToArray(fragmentLines, functionLines); } return { diff --git a/packages/engine/Source/Scene/Cesium3DTileBatchTable.js b/packages/engine/Source/Scene/Cesium3DTileBatchTable.js index 5bb9f4f3ac42..4b758ea20ec5 100644 --- a/packages/engine/Source/Scene/Cesium3DTileBatchTable.js +++ b/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 addAllToArray from "../Core/addAllToArray.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); + 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; diff --git a/packages/engine/Source/Scene/Cesium3DTileStyle.js b/packages/engine/Source/Scene/Cesium3DTileStyle.js index 5ebe368e10a0..28d5f84323bf 100644 --- a/packages/engine/Source/Scene/Cesium3DTileStyle.js +++ b/packages/engine/Source/Scene/Cesium3DTileStyle.js @@ -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"; @@ -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 diff --git a/packages/engine/Source/Scene/ConditionsExpression.js b/packages/engine/Source/Scene/ConditionsExpression.js index b88e01cc2053..f2c16330d741 100644 --- a/packages/engine/Source/Scene/ConditionsExpression.js +++ b/packages/engine/Source/Scene/ConditionsExpression.js @@ -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"; @@ -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 diff --git a/packages/engine/Source/Scene/GltfLoader.js b/packages/engine/Source/Scene/GltfLoader.js index 35cdc7585012..0f8ef25fdf12 100644 --- a/packages/engine/Source/Scene/GltfLoader.js +++ b/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 addAllToArray from "../Core/addAllToArray.js"; const { Attribute, @@ -2764,12 +2765,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); diff --git a/packages/engine/Source/Scene/MetadataTableProperty.js b/packages/engine/Source/Scene/MetadataTableProperty.js index 3af58b4f559e..24dcf94fdb71 100644 --- a/packages/engine/Source/Scene/MetadataTableProperty.js +++ b/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 addAllToArray from "../Core/addAllToArray.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); + addAllToArray(result, value); } else { result.push(value); } diff --git a/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js b/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js index 1b8c3c91115c..ea051f0e4752 100644 --- a/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js +++ b/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js @@ -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"; @@ -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 = @@ -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); } } diff --git a/packages/engine/Source/Scene/Model/GeoJsonLoader.js b/packages/engine/Source/Scene/Model/GeoJsonLoader.js index 092962cd10f5..17b1e4cf1a3f 100644 --- a/packages/engine/Source/Scene/Model/GeoJsonLoader.js +++ b/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 addAllToArray from "../../Core/addAllToArray.js"; /** * Loads a GeoJson model as part of the MAXAR_content_geojson 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]); + addAllToArray(lines, polygon); } return lines; } diff --git a/packages/engine/Source/Scene/Model/InstancingPipelineStage.js b/packages/engine/Source/Scene/Model/InstancingPipelineStage.js index 682d2904cd5d..620f49016080 100644 --- a/packages/engine/Source/Scene/Model/InstancingPipelineStage.js +++ b/packages/engine/Source/Scene/Model/InstancingPipelineStage.js @@ -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"; @@ -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(); @@ -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( diff --git a/packages/engine/Source/Scene/Model/ModelSceneGraph.js b/packages/engine/Source/Scene/Model/ModelSceneGraph.js index 248ee80f3403..25e35fb62a60 100644 --- a/packages/engine/Source/Scene/Model/ModelSceneGraph.js +++ b/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 addAllToArray from "../../Core/addAllToArray.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); + addAllToArray(frameState.commandList, silhouetteCommands); }; // Callback is defined here to avoid allocating a closure in the render loop diff --git a/packages/engine/Source/Scene/PropertyTable.js b/packages/engine/Source/Scene/PropertyTable.js index cbb73b3cfd9b..35b1b1e20a2b 100644 --- a/packages/engine/Source/Scene/PropertyTable.js +++ b/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 addAllToArray from "../Core/addAllToArray.js"; /** * A property table for use with the EXT_structural_metadata 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); + 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; diff --git a/packages/engine/Specs/Core/addAllSpec.js b/packages/engine/Specs/Core/addAllSpec.js new file mode 100644 index 000000000000..15aa3e3f39b4 --- /dev/null +++ b/packages/engine/Specs/Core/addAllSpec.js @@ -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); + }); +}); diff --git a/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js b/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js index adfa295c7948..607076df9338 100644 --- a/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js +++ b/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js @@ -1,4 +1,5 @@ import { + addAllToArray, BlendingState, BoundingSphere, Cartesian3, @@ -814,8 +815,8 @@ describe( expect(drawCommand.modelMatrix).toBe(command.modelMatrix); const commandList = []; - commandList.push.apply(commandList, drawCommand._commandListTerrain); - commandList.push.apply(commandList, drawCommand._commandList3DTiles); + addAllToArray(commandList, drawCommand._commandListTerrain); + addAllToArray(commandList, drawCommand._commandList3DTiles); const length = commandList.length; expect(length).toEqual(12); diff --git a/packages/engine/Specs/Scene/Model/ClassificationPipelineStageSpec.js b/packages/engine/Specs/Scene/Model/ClassificationPipelineStageSpec.js index d62b7184c13c..c816f0fee252 100644 --- a/packages/engine/Specs/Scene/Model/ClassificationPipelineStageSpec.js +++ b/packages/engine/Specs/Scene/Model/ClassificationPipelineStageSpec.js @@ -1,4 +1,5 @@ import { + addAllToArray, ClassificationPipelineStage, PrimitiveType, RuntimeError, @@ -19,7 +20,7 @@ describe("Scene/Model/ClassificationPipelineStage", function () { const batchLength = batchLengths[id]; const batch = new Array(batchLength); batch.fill(id); - featureIds.push.apply(featureIds, batch); + addAllToArray(featureIds, batch); } return featureIds; diff --git a/packages/widgets/Source/Animation/AnimationViewModel.js b/packages/widgets/Source/Animation/AnimationViewModel.js index f00e762cd677..b74d0ba536eb 100644 --- a/packages/widgets/Source/Animation/AnimationViewModel.js +++ b/packages/widgets/Source/Animation/AnimationViewModel.js @@ -1,4 +1,5 @@ import { + addAllToArray, binarySearch, ClockRange, ClockStep, @@ -506,7 +507,7 @@ AnimationViewModel.prototype.setShuttleRingTicks = function (positiveTicks) { allTicks.push(-tick); } } - Array.prototype.push.apply(allTicks, sortedFilteredPositiveTicks); + addAllToArray(allTicks, sortedFilteredPositiveTicks); this._allShuttleRingTicks = allTicks; };