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

Issue 12053- CesiumJS should not use push.apply #12176

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
refactor: replace push.apply with for loop
Tim-Quattrochi committed Aug 31, 2024
commit 3d40884371dbf8c23cf5c5d86155945281097124
24 changes: 18 additions & 6 deletions packages/engine/Source/Renderer/ShaderBuilder.js
Original file line number Diff line number Diff line change
@@ -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 {
11 changes: 6 additions & 5 deletions packages/engine/Source/Scene/Cesium3DTileBatchTable.js
Original file line number Diff line number Diff line change
@@ -373,13 +373,14 @@ Cesium3DTileBatchTable.prototype.getPropertyIds = function (batchId, results) {
results.length = 0;

const scratchPropertyIds = Object.keys(this._properties);
results.push.apply(results, scratchPropertyIds);
for (let i = 0; i < scratchPropertyIds.length; ++i) {
results.push(scratchPropertyIds[i]);
}

if (defined(this._batchTableHierarchy)) {
results.push.apply(
results,
this._batchTableHierarchy.getPropertyIds(batchId, scratchPropertyIds)
);
for (let i = 0; i < this._batchTableHierarchy._classes.length; ++i) {
results.push(this._batchTableHierarchy._classes[i].name);
}
}

return results;
12 changes: 9 additions & 3 deletions packages/engine/Source/Scene/Cesium3DTileStyle.js
Original file line number Diff line number Diff line change
@@ -1453,15 +1453,21 @@ Cesium3DTileStyle.prototype.getVariables = function () {
let variables = [];

if (defined(this.color) && defined(this.color.getVariables)) {
variables.push.apply(variables, this.color.getVariables());
for (let i = 0; i < this.color.getVariables().length; i++) {
variables.push(this.color.getVariables()[i]);
}
}

if (defined(this.show) && defined(this.show.getVariables)) {
variables.push.apply(variables, this.show.getVariables());
for (let i = 0; i < this.show.getVariables().length; i++) {
variables.push(this.show.getVariables()[i]);
}
}

if (defined(this.pointSize) && defined(this.pointSize.getVariables)) {
variables.push.apply(variables, this.pointSize.getVariables());
for (let i = 0; i < this.pointSize.getVariables().length; i++) {
variables.push(this.pointSize.getVariables()[i]);
}
}

// Remove duplicates
8 changes: 6 additions & 2 deletions packages/engine/Source/Scene/ConditionsExpression.js
Original file line number Diff line number Diff line change
@@ -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
18 changes: 16 additions & 2 deletions packages/engine/Source/Scene/GltfLoader.js
Original file line number Diff line number Diff line change
@@ -2683,12 +2683,26 @@ function parse(loader, frameState) {

// Gather promises and handle any errors
const readyPromises = [];
readyPromises.push.apply(readyPromises, loader._loaderPromises);
for (let i = 0; i < loader._loaderPromises.length; ++i) {
const promise = loader._loaderPromises[i];
promise.then(undefined, function (error) {
loader._state = GltfLoaderState.FAILED;
loader._error = error;
});
readyPromises.push(promise);
}

// 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);
for (let i = 0; i < loader._textureLoaders.length; ++i) {
const promise = loader._textureLoaders[i].readyPromise;
promise.then(undefined, function (error) {
loader._state = GltfLoaderState.FAILED;
loader._error = error;
});
readyPromises.push(promise);
}
}

return Promise.all(readyPromises);
19 changes: 9 additions & 10 deletions packages/engine/Source/Scene/MetadataTableProperty.js
Original file line number Diff line number Diff line change
@@ -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 {