diff --git a/CHANGES.md b/CHANGES.md index 003d3a93908f..9373c816377a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,13 @@ # Change Log +### 1.122 - 2024-08-30 + +#### @cesium/engine + +##### Fixes :wrench: + +- Replaced `Array.prototype.push.apply` with `for` loops to avoid exceeding the call stack limit. This change improves stability when handling large arrays. [#12053](https://github.com/CesiumGS/cesium/issues/12053) + ### 1.121 - 2024-09-01 #### @cesium/engine diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 5ffba76b8eba..1a99c3198644 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -405,3 +405,4 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for details on how to contribute to Cesiu - [Levi Montgomery](https://github.com/Levi-Montgomery) - [Brandon Berisford](https://github.com/BeyondBelief96) - [Adam Wirth](https://https://github.com/adamwirth) +- [Tim Quattrochi](https://github.com/Tim-Quattrochi) diff --git a/packages/engine/Source/Renderer/ShaderBuilder.js b/packages/engine/Source/Renderer/ShaderBuilder.js index 6fd79ab0098e..93d8df2b7850 100644 --- a/packages/engine/Source/Renderer/ShaderBuilder.js +++ b/packages/engine/Source/Renderer/ShaderBuilder.js @@ -414,7 +414,9 @@ ShaderBuilder.prototype.addVertexLines = function (lines) { const vertexLines = this._vertexShaderParts.shaderLines; if (Array.isArray(lines)) { - vertexLines.push.apply(vertexLines, lines); + for (let i = 0; i < lines.length; i++) { + vertexLines.push(lines[i]); + } } else { // Single string case vertexLines.push(lines); @@ -449,7 +451,9 @@ ShaderBuilder.prototype.addFragmentLines = function (lines) { const fragmentLines = this._fragmentShaderParts.shaderLines; if (Array.isArray(lines)) { - fragmentLines.push.apply(fragmentLines, lines); + for (let i = 0; i < lines.length; i++) { + fragmentLines.push(lines[i]); + } } else { // Single string case fragmentLines.push(lines); @@ -534,7 +538,9 @@ function generateStructLines(shaderBuilder) { structId = structIds[i]; struct = shaderBuilder._structs[structId]; structLines = struct.generateGlslLines(); - vertexLines.push.apply(vertexLines, structLines); + for (let j = 0; j < structLines.length; j++) { + vertexLines.push(structLines[j]); + } } structIds = shaderBuilder._fragmentShaderParts.structIds; @@ -542,7 +548,9 @@ function generateStructLines(shaderBuilder) { structId = structIds[i]; struct = shaderBuilder._structs[structId]; structLines = struct.generateGlslLines(); - fragmentLines.push.apply(fragmentLines, structLines); + for (let j = 0; j < structLines.length; j++) { + fragmentLines.push(structLines[j]); + } } return { @@ -577,7 +585,9 @@ function generateFunctionLines(shaderBuilder) { functionId = functionIds[i]; func = shaderBuilder._functions[functionId]; functionLines = func.generateGlslLines(); - vertexLines.push.apply(vertexLines, functionLines); + for (let j = 0; j < functionLines.length; j++) { + vertexLines.push(functionLines[j]); + } } functionIds = shaderBuilder._fragmentShaderParts.functionIds; @@ -585,7 +595,9 @@ function generateFunctionLines(shaderBuilder) { functionId = functionIds[i]; func = shaderBuilder._functions[functionId]; functionLines = func.generateGlslLines(); - fragmentLines.push.apply(fragmentLines, functionLines); + for (let j = 0; j < functionLines.length; j++) { + fragmentLines.push(functionLines[j]); + } } return { diff --git a/packages/engine/Source/Scene/Cesium3DTileBatchTable.js b/packages/engine/Source/Scene/Cesium3DTileBatchTable.js index b9a678cdd8c5..0f150cba341a 100644 --- a/packages/engine/Source/Scene/Cesium3DTileBatchTable.js +++ b/packages/engine/Source/Scene/Cesium3DTileBatchTable.js @@ -373,13 +373,21 @@ Cesium3DTileBatchTable.prototype.getPropertyIds = function (batchId, results) { results.length = 0; const scratchPropertyIds = Object.keys(this._properties); - results.push.apply(results, scratchPropertyIds); + const propertiesLength = scratchPropertyIds.length; + for (let i = 0; i < propertiesLength; ++i) { + results.push(scratchPropertyIds[i]); + } if (defined(this._batchTableHierarchy)) { - results.push.apply( - results, - this._batchTableHierarchy.getPropertyIds(batchId, scratchPropertyIds) + const hierarchyPropertyIds = this._batchTableHierarchy.getPropertyIds( + batchId, + scratchPropertyIds ); + + const hierarchyPropertyIdsLength = hierarchyPropertyIds.length; + for (let i = 0; i < hierarchyPropertyIdsLength; ++i) { + results.push(hierarchyPropertyIds[i]); + } } return results; diff --git a/packages/engine/Source/Scene/Cesium3DTileStyle.js b/packages/engine/Source/Scene/Cesium3DTileStyle.js index b5cd56c60d37..4280c4cae72e 100644 --- a/packages/engine/Source/Scene/Cesium3DTileStyle.js +++ b/packages/engine/Source/Scene/Cesium3DTileStyle.js @@ -1453,15 +1453,27 @@ Cesium3DTileStyle.prototype.getVariables = function () { let variables = []; if (defined(this.color) && defined(this.color.getVariables)) { - variables.push.apply(variables, this.color.getVariables()); + const colorVariables = this.color.getVariables(); + const n = colorVariables.length; + for (let i = 0; i < n; i++) { + variables.push(colorVariables[i]); + } } if (defined(this.show) && defined(this.show.getVariables)) { - variables.push.apply(variables, this.show.getVariables()); + const showVariables = this.show.getVariables(); + const n = showVariables.length; + for (let i = 0; i < n; i++) { + variables.push(showVariables[i]); + } } if (defined(this.pointSize) && defined(this.pointSize.getVariables)) { - variables.push.apply(variables, this.pointSize.getVariables()); + const pointSizeVariables = this.pointSize.getVariables(); + const n = pointSizeVariables.length; + for (let i = 0; i < n; i++) { + variables.push(pointSizeVariables[i]); + } } // Remove duplicates diff --git a/packages/engine/Source/Scene/ConditionsExpression.js b/packages/engine/Source/Scene/ConditionsExpression.js index 1bed27442ace..21ab50ba3d58 100644 --- a/packages/engine/Source/Scene/ConditionsExpression.js +++ b/packages/engine/Source/Scene/ConditionsExpression.js @@ -203,8 +203,12 @@ 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()); + for (const variable of statement.condition.getVariables()) { + variables.push(variable); + } + for (const variable of statement.expression.getVariables()) { + variables.push(variable); + } } // Remove duplicates diff --git a/packages/engine/Source/Scene/GltfLoader.js b/packages/engine/Source/Scene/GltfLoader.js index 18f825a6ae4e..1ad97f29530f 100644 --- a/packages/engine/Source/Scene/GltfLoader.js +++ b/packages/engine/Source/Scene/GltfLoader.js @@ -2683,12 +2683,18 @@ function parse(loader, frameState) { // Gather promises and handle any errors const readyPromises = []; - readyPromises.push.apply(readyPromises, loader._loaderPromises); + const n = loader._loaderPromises.length; + for (let i = 0; i < n; ++i) { + readyPromises.push(loader._loaderPromises[i]); + } // 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); + const texturesPromisesLength = loader._texturesPromises.length; + for (let i = 0; i < texturesPromisesLength; ++i) { + readyPromises.push(loader._texturesPromises[i]); + } } return Promise.all(readyPromises); diff --git a/packages/engine/Source/Scene/MetadataTableProperty.js b/packages/engine/Source/Scene/MetadataTableProperty.js index 8198e0151652..953b2853341e 100644 --- a/packages/engine/Source/Scene/MetadataTableProperty.js +++ b/packages/engine/Source/Scene/MetadataTableProperty.js @@ -388,7 +388,9 @@ function flatten(values) { for (let i = 0; i < values.length; i++) { const value = values[i]; if (Array.isArray(value)) { - result.push.apply(result, value); + for (let j = 0; j < value.length; j++) { + result.push(value[j]); + } } else { result.push(value); } @@ -562,7 +564,7 @@ function getInt64NumberFallback(index, values) { function getInt64BigIntFallback(index, values) { const dataView = values.dataView; const byteOffset = index * 8; - // eslint-disable-next-line no-undef + let value = BigInt(0); const isNegative = (dataView.getUint8(byteOffset + 7) & 0x80) > 0; let carrying = true; @@ -578,7 +580,7 @@ function getInt64BigIntFallback(index, values) { byte = ~byte & 0xff; } } - value += BigInt(byte) * (BigInt(1) << BigInt(i * 8)); // eslint-disable-line + value += BigInt(byte) * (BigInt(1) << BigInt(i * 8)); } if (isNegative) { value = -value; @@ -605,14 +607,13 @@ function getUint64BigIntFallback(index, values) { const byteOffset = index * 8; // Split 64-bit number into two 32-bit (4-byte) parts - // eslint-disable-next-line no-undef + const left = BigInt(dataView.getUint32(byteOffset, true)); - // eslint-disable-next-line no-undef const right = BigInt(dataView.getUint32(byteOffset + 4, true)); // Combine the two 32-bit values - // eslint-disable-next-line no-undef + const value = left + BigInt(4294967296) * right; return value; @@ -783,7 +784,6 @@ function BufferView(bufferView, componentType, length) { return getInt64BigIntFallback(index, that); }; } else { - // eslint-disable-next-line typedArray = new BigInt64Array( bufferView.buffer, bufferView.byteOffset, @@ -791,7 +791,7 @@ function BufferView(bufferView, componentType, length) { ); setFunction = function (index, value) { // Convert the number to a BigInt before setting the value in the typed array - that.typedArray[index] = BigInt(value); // eslint-disable-line + that.typedArray[index] = BigInt(value); }; } } else if (componentType === MetadataComponentType.UINT64) { @@ -817,7 +817,6 @@ function BufferView(bufferView, componentType, length) { return getUint64BigIntFallback(index, that); }; } else { - // eslint-disable-next-line typedArray = new BigUint64Array( bufferView.buffer, bufferView.byteOffset, @@ -825,7 +824,7 @@ function BufferView(bufferView, componentType, length) { ); setFunction = function (index, value) { // Convert the number to a BigInt before setting the value in the typed array - that.typedArray[index] = BigInt(value); // eslint-disable-line + that.typedArray[index] = BigInt(value); }; } } else { diff --git a/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js b/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js index 4d23fa50ef79..836192cddbf4 100644 --- a/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js +++ b/packages/engine/Source/Scene/Model/ClassificationModelDrawCommand.js @@ -482,16 +482,22 @@ ClassificationModelDrawCommand.prototype.pushCommands = function ( const passes = frameState.passes; if (passes.render) { if (this._useDebugWireframe) { - result.push.apply(result, this._commandListDebugWireframe); + for (let i = 0; i < this._commandListDebugWireframe.length; i++) { + result.push(this._commandListDebugWireframe[i]); + } return; } if (this._classifiesTerrain) { - result.push.apply(result, this._commandListTerrain); + for (let i = 0; i < this._commandListTerrain.length; i++) { + result.push(this._commandListTerrain[i]); + } } if (this._classifies3DTiles) { - result.push.apply(result, this._commandList3DTiles); + for (let i = 0; i < this._commandList3DTiles.length; i++) { + result.push(this._commandList3DTiles[i]); + } } const useIgnoreShowCommands = @@ -513,17 +519,23 @@ ClassificationModelDrawCommand.prototype.pushCommands = function ( ); } - result.push.apply(result, this._commandListIgnoreShow); + for (let i = 0; i < this._commandListIgnoreShow.length; i++) { + result.push(this._commandListIgnoreShow[i]); + } } } if (passes.pick) { if (this._classifiesTerrain) { - result.push.apply(result, this._commandListTerrainPicking); + for (let i = 0; i < this._commandListTerrainPicking.length; i++) { + result.push(this._commandListTerrainPicking[i]); + } } if (this._classifies3DTiles) { - result.push.apply(result, this._commandList3DTilesPicking); + for (let i = 0; i < this._commandList3DTilesPicking.length; i++) { + result.push(this._commandList3DTilesPicking[i]); + } } } diff --git a/packages/engine/Source/Scene/Model/GeoJsonLoader.js b/packages/engine/Source/Scene/Model/GeoJsonLoader.js index 5bc67d5ec9ca..7811132245b1 100644 --- a/packages/engine/Source/Scene/Model/GeoJsonLoader.js +++ b/packages/engine/Source/Scene/Model/GeoJsonLoader.js @@ -166,7 +166,11 @@ 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]); + const polygonLength = polygon.length; + for (let j = 0; j < polygonLength; j++) { + lines.push(polygon[j]); + } } return lines; } diff --git a/packages/engine/Source/Scene/Model/InstancingPipelineStage.js b/packages/engine/Source/Scene/Model/InstancingPipelineStage.js index f1c209285b93..8e7615b7efd3 100644 --- a/packages/engine/Source/Scene/Model/InstancingPipelineStage.js +++ b/packages/engine/Source/Scene/Model/InstancingPipelineStage.js @@ -210,10 +210,11 @@ InstancingPipelineStage.process = function (renderResources, node, frameState) { renderResources.uniformMap = combine(uniformMap, renderResources.uniformMap); renderResources.instanceCount = count; - renderResources.attributes.push.apply( - renderResources.attributes, - instancingVertexAttributes - ); + + const instancingVertexAttributesLength = instancingVertexAttributes.length; + for (let i = 0; i < instancingVertexAttributesLength; i++) { + renderResources.attributes.push(instancingVertexAttributes[i]); + } }; const projectedTransformScratch = new Matrix4(); @@ -988,10 +989,9 @@ function processMatrixAttributes( shaderBuilder.addAttribute("vec4", `a_instancing${attributeString}Row1`); shaderBuilder.addAttribute("vec4", `a_instancing${attributeString}Row2`); - instancingVertexAttributes.push.apply( - instancingVertexAttributes, - matrixAttributes - ); + for (let i = 0; i < matrixAttributes.length; i++) { + instancingVertexAttributes.push(matrixAttributes[i]); + } } function processVec3Attribute( diff --git a/packages/engine/Source/Scene/Model/ModelSceneGraph.js b/packages/engine/Source/Scene/Model/ModelSceneGraph.js index 954c63081acc..11f69636f59f 100644 --- a/packages/engine/Source/Scene/Model/ModelSceneGraph.js +++ b/packages/engine/Source/Scene/Model/ModelSceneGraph.js @@ -911,7 +911,10 @@ ModelSceneGraph.prototype.pushDrawCommands = function (frameState) { pushDrawCommandOptions ); - frameState.commandList.push.apply(frameState.commandList, silhouetteCommands); + for (let i = 0; i < silhouetteCommands.length; i++) { + const silhouetteCommand = silhouetteCommands[i]; + silhouetteCommand.execute(frameState); + } }; // 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 19860c82de18..521b365ba59f 100644 --- a/packages/engine/Source/Scene/PropertyTable.js +++ b/packages/engine/Source/Scene/PropertyTable.js @@ -296,24 +296,30 @@ 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 propertyIds = this._metadataTable.getPropertyIds(scratchResults); + const n = propertyIds.length; + for (let i = 0; i < n; ++i) { + results.push(propertyIds[i]); + } } if (defined(this._batchTableHierarchy)) { - results.push.apply( - results, - this._batchTableHierarchy.getPropertyIds(index, scratchResults) + const propertyIds = this._batchTableHierarchy.getPropertyIds( + index, + scratchResults ); + const n = propertyIds.length; + for (let i = 0; i < n; i++) { + results.push(propertyIds[i]); + } } if (defined(this._jsonMetadataTable)) { - results.push.apply( - results, - this._jsonMetadataTable.getPropertyIds(scratchResults) - ); + const propertyIds = this._jsonMetadataTable.getPropertyIds(scratchResults); + const n = propertyIds.length; + for (let i = 0; i < n; i++) { + results.push(propertyIds[i]); + } } return results; diff --git a/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js b/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js index 6df7bf6333e6..71cc2fc8031f 100644 --- a/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js +++ b/packages/engine/Specs/Scene/Model/ClassificationModelDrawCommandSpec.js @@ -814,8 +814,13 @@ describe( expect(drawCommand.modelMatrix).toBe(command.modelMatrix); const commandList = []; - commandList.push.apply(commandList, drawCommand._commandListTerrain); - commandList.push.apply(commandList, drawCommand._commandList3DTiles); + for (let i = 0; i < drawCommand._commandListTerrain.length; i++) { + commandList.push(drawCommand._commandListTerrain[i]); + } + + for (let i = 0; i < drawCommand._commandList3DTiles.length; i++) { + commandList.push(drawCommand._commandList3DTiles[i]); + } 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 dc30239df431..ca9f9236fc7e 100644 --- a/packages/engine/Specs/Scene/Model/ClassificationPipelineStageSpec.js +++ b/packages/engine/Specs/Scene/Model/ClassificationPipelineStageSpec.js @@ -19,7 +19,9 @@ describe("Scene/Model/ClassificationPipelineStage", function () { const batchLength = batchLengths[id]; const batch = new Array(batchLength); batch.fill(id); - featureIds.push.apply(featureIds, batch); + for (let j = 0; j < batch.length; j++) { + featureIds.push(batch[j]); + } } return featureIds; diff --git a/packages/widgets/Source/Animation/AnimationViewModel.js b/packages/widgets/Source/Animation/AnimationViewModel.js index 864b0bc51e6f..cf5e0b69c215 100644 --- a/packages/widgets/Source/Animation/AnimationViewModel.js +++ b/packages/widgets/Source/Animation/AnimationViewModel.js @@ -503,7 +503,10 @@ AnimationViewModel.prototype.setShuttleRingTicks = function (positiveTicks) { allTicks.push(-tick); } } - Array.prototype.push.apply(allTicks, sortedFilteredPositiveTicks); + for (i = 0, len = sortedFilteredPositiveTicks.length; i < len; ++i) { + tick = sortedFilteredPositiveTicks[i]; + allTicks.push(tick); + } this._allShuttleRingTicks = allTicks; };