Issue 12053- CesiumJS should not use push.apply

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

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)

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,15 +538,19 @@ 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;
   for (i = 0; i < structIds.length; i++) {
     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,15 +585,19 @@ 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;
   for (i = 0; i < functionIds.length; i++) {
     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 {

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;

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

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

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);

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,15 +784,14 @@ function BufferView(bufferView, componentType, length) {
         return getInt64BigIntFallback(index, that);
       };
     } else {
-      // eslint-disable-next-line
       typedArray = new BigInt64Array(
         bufferView.buffer,
         bufferView.byteOffset,
         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,15 +817,14 @@ function BufferView(bufferView, componentType, length) {
         return getUint64BigIntFallback(index, that);
       };
     } else {
-      // eslint-disable-next-line
       typedArray = new BigUint64Array(
         bufferView.buffer,
         bufferView.byteOffset,
         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 {

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]);
+      }
     }
   }
 

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;
 }

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(

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