diff --git a/CHANGES.md b/CHANGES.md index 649b51a89f56..60a02408449a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -20,6 +20,7 @@ Change Log * Fixed a bug that caused large, nearby geometry to be clipped when using a logarithmic depth buffer, which is the default on most systems. [#8600](https://github.com/CesiumGS/cesium/pull/8600) * Fixed a bug where tiles would not load if the camera was tracking a moving tileset. [#8598](https://github.com/CesiumGS/cesium/pull/8598) * Fixed a bug where applying a new 3D Tiles style during a flight would not update all existing tiles. [#8622](https://github.com/CesiumGS/cesium/pull/8622) +* Fixed a bug where Cartesian vectors could not be packed to typed arrays [#8568](https://github.com/AnalyticalGraphicsInc/cesium/pull/8568) ### 1.66.0 - 2020-02-03 diff --git a/Source/Core/Cartesian2.js b/Source/Core/Cartesian2.js index 4ffcfcc81a9b..99401e23083f 100644 --- a/Source/Core/Cartesian2.js +++ b/Source/Core/Cartesian2.js @@ -149,7 +149,8 @@ import CesiumMath from './Math.js'; * Flattens an array of Cartesian2s into and array of components. * * @param {Cartesian2[]} array The array of cartesians to pack. - * @param {Number[]} [result] The array onto which to store the result. + * @param {Number[]} [result] The array onto which to store the result. If this is a typed array, it must have array.length * 2 components, else a {@link DeveloperError} will be thrown. If it is a regular array, it will be resized to have (array.length * 2) elements. + * @returns {Number[]} The packed array. */ Cartesian2.packArray = function(array, result) { @@ -158,10 +159,13 @@ import CesiumMath from './Math.js'; //>>includeEnd('debug'); var length = array.length; + var resultLength = length * 2; if (!defined(result)) { - result = new Array(length * 2); - } else { - result.length = length * 2; + result = new Array(resultLength); + } else if (!Array.isArray(result) && result.length !== resultLength) { + throw new DeveloperError('If result is a typed array, it must have exactly array.length * 2 elements'); + } else if (result.length !== resultLength) { + result.length = resultLength; } for (var i = 0; i < length; ++i) { @@ -180,6 +184,10 @@ import CesiumMath from './Math.js'; Cartesian2.unpackArray = function(array, result) { //>>includeStart('debug', pragmas.debug); Check.defined('array', array); + Check.typeOf.number.greaterThanOrEquals('array.length', array.length, 2); + if (array.length % 2 !== 0) { + throw new DeveloperError('array length must be a multiple of 2.'); + } //>>includeEnd('debug'); var length = array.length; diff --git a/Source/Core/Cartesian3.js b/Source/Core/Cartesian3.js index 9fbd998f683f..485d551b28d0 100644 --- a/Source/Core/Cartesian3.js +++ b/Source/Core/Cartesian3.js @@ -177,7 +177,7 @@ import CesiumMath from './Math.js'; * Flattens an array of Cartesian3s into an array of components. * * @param {Cartesian3[]} array The array of cartesians to pack. - * @param {Number[]} [result] The array onto which to store the result. + * @param {Number[]} [result] The array onto which to store the result. If this is a typed array, it must have array.length * 3 components, else a {@link DeveloperError} will be thrown. If it is a regular array, it will be resized to have (array.length * 3) elements. * @returns {Number[]} The packed array. */ Cartesian3.packArray = function(array, result) { @@ -186,10 +186,13 @@ import CesiumMath from './Math.js'; //>>includeEnd('debug'); var length = array.length; + var resultLength = length * 3; if (!defined(result)) { - result = new Array(length * 3); - } else { - result.length = length * 3; + result = new Array(resultLength); + } else if (!Array.isArray(result) && result.length !== resultLength) { + throw new DeveloperError('If result is a typed array, it must have exactly array.length * 3 elements'); + } else if (result.length !== resultLength) { + result.length = resultLength; } for (var i = 0; i < length; ++i) { diff --git a/Source/Core/Cartesian4.js b/Source/Core/Cartesian4.js index 6f3911fefad3..d0542d2f52fe 100644 --- a/Source/Core/Cartesian4.js +++ b/Source/Core/Cartesian4.js @@ -177,7 +177,8 @@ import CesiumMath from './Math.js'; * Flattens an array of Cartesian4s into and array of components. * * @param {Cartesian4[]} array The array of cartesians to pack. - * @param {Number[]} [result] The array onto which to store the result. + * @param {Number[]} [result] The array onto which to store the result. If this is a typed array, it must have array.length * 4 components, else a {@link DeveloperError} will be thrown. If it is a regular array, it will be resized to have (array.length * 4) elements. + * @returns {Number[]} The packed array. */ Cartesian4.packArray = function(array, result) { @@ -186,10 +187,13 @@ import CesiumMath from './Math.js'; //>>includeEnd('debug'); var length = array.length; + var resultLength = length * 4; if (!defined(result)) { - result = new Array(length * 4); - } else { - result.length = length * 4; + result = new Array(resultLength); + } else if (!Array.isArray(result) && result.length !== resultLength) { + throw new DeveloperError('If result is a typed array, it must have exactly array.length * 4 elements'); + } else if (result.length !== resultLength) { + result.length = resultLength; } for (var i = 0; i < length; ++i) { @@ -208,6 +212,10 @@ import CesiumMath from './Math.js'; Cartesian4.unpackArray = function(array, result) { //>>includeStart('debug', pragmas.debug); Check.defined('array', array); + Check.typeOf.number.greaterThanOrEquals('array.length', array.length, 4); + if (array.length % 4 !== 0) { + throw new DeveloperError('array length must be a multiple of 4.'); + } //>>includeEnd('debug'); var length = array.length; diff --git a/Specs/Core/Cartesian2Spec.js b/Specs/Core/Cartesian2Spec.js index 3389bbe056fa..b38140135daf 100644 --- a/Specs/Core/Cartesian2Spec.js +++ b/Specs/Core/Cartesian2Spec.js @@ -843,5 +843,5 @@ describe('Core/Cartesian2', function() { }); createPackableSpecs(Cartesian2, new Cartesian2(1, 2), [1, 2]); - createPackableArraySpecs(Cartesian2, [new Cartesian2(1, 2), new Cartesian2(3, 4)], [1, 2, 3, 4]); + createPackableArraySpecs(Cartesian2, [new Cartesian2(1, 2), new Cartesian2(3, 4)], [1, 2, 3, 4], 2); }); diff --git a/Specs/Core/Cartesian3Spec.js b/Specs/Core/Cartesian3Spec.js index fe96aad974ba..4dc614a6cbb2 100644 --- a/Specs/Core/Cartesian3Spec.js +++ b/Specs/Core/Cartesian3Spec.js @@ -54,35 +54,6 @@ describe('Core/Cartesian3', function() { }).toThrowDeveloperError(); }); - it('unpackArray works', function() { - var array = Cartesian3.unpackArray([0.0, 1.0, 2.0, 3.0, 0.0, 4.0]); - expect(array).toEqual([new Cartesian3(0.0, 1.0, 2.0), new Cartesian3(3.0, 0.0, 4.0)]); - }); - - it('unpackArray works with a result parameter', function() { - var array = []; - var result = Cartesian3.unpackArray([1.0, 2.0, 3.0], array); - expect(result).toBe(array); - expect(result).toEqual([new Cartesian3(1.0, 2.0, 3.0)]); - - array = [new Cartesian3(), new Cartesian3(), new Cartesian3()]; - result = Cartesian3.unpackArray([1.0, 2.0, 3.0], array); - expect(result).toBe(array); - expect(result).toEqual([new Cartesian3(1.0, 2.0, 3.0)]); - }); - - it('unpackArray throws with array less than 3 length', function() { - expect(function() { - Cartesian3.unpackArray([1.0]); - }).toThrowDeveloperError(); - }); - - it('unpackArray throws with array not multiple of 3', function() { - expect(function() { - Cartesian3.unpackArray([1.0, 2.0, 3.0, 4.0]); - }).toThrowDeveloperError(); - }); - it('clone with a result parameter', function() { var cartesian = new Cartesian3(1.0, 2.0, 3.0); var result = new Cartesian3(); @@ -1287,5 +1258,5 @@ describe('Core/Cartesian3', function() { }); createPackableSpecs(Cartesian3, new Cartesian3(1, 2, 3), [1, 2, 3]); - createPackableArraySpecs(Cartesian3, [new Cartesian3(1, 2, 3), new Cartesian3(4, 5, 6)], [1, 2, 3, 4, 5, 6]); + createPackableArraySpecs(Cartesian3, [new Cartesian3(1, 2, 3), new Cartesian3(4, 5, 6)], [1, 2, 3, 4, 5, 6], 3); }); diff --git a/Specs/Core/Cartesian4Spec.js b/Specs/Core/Cartesian4Spec.js index c4cf15242f94..abc37e44891a 100644 --- a/Specs/Core/Cartesian4Spec.js +++ b/Specs/Core/Cartesian4Spec.js @@ -956,5 +956,5 @@ describe('Core/Cartesian4', function() { }); createPackableSpecs(Cartesian4, new Cartesian4(1, 2, 3, 4), [1, 2, 3, 4]); - createPackableArraySpecs(Cartesian4, [new Cartesian4(1, 2, 3, 4), new Cartesian4(5, 6, 7, 8)], [1, 2, 3, 4, 5, 6, 7, 8]); + createPackableArraySpecs(Cartesian4, [new Cartesian4(1, 2, 3, 4), new Cartesian4(5, 6, 7, 8)], [1, 2, 3, 4, 5, 6, 7, 8], 4); }); diff --git a/Specs/createPackableArraySpecs.js b/Specs/createPackableArraySpecs.js index 402b0f7989e6..d6263c452bd1 100644 --- a/Specs/createPackableArraySpecs.js +++ b/Specs/createPackableArraySpecs.js @@ -1,6 +1,6 @@ import { defaultValue } from '../Source/Cesium.js'; - function createPackableArraySpecs(packable, unpackedArray, packedArray, namePrefix) { + function createPackableArraySpecs(packable, unpackedArray, packedArray, stride, namePrefix) { namePrefix = defaultValue(namePrefix, ''); it(namePrefix + ' can pack', function() { @@ -11,8 +11,7 @@ import { defaultValue } from '../Source/Cesium.js'; it(namePrefix + ' can roundtrip', function() { var actualPackedArray = packable.packArray(unpackedArray); - var result = packable.unpackArray(actualPackedArray); - expect(result).toEqual(unpackedArray); + var result = packable.unpackArray(actualPackedArray); expect(result).toEqual(unpackedArray); }); it(namePrefix + ' can unpack', function() { @@ -20,16 +19,78 @@ import { defaultValue } from '../Source/Cesium.js'; expect(result).toEqual(unpackedArray); }); + it(namePrefix + ' packArray works with typed arrays', function() { + var typedArray = new Float64Array(packedArray.length); + var result = packable.packArray(unpackedArray, typedArray); + expect(result).toEqual(new Float64Array(packedArray)); + }); + + it(namePrefix + ' packArray resizes arrays as needed', function() { + var emptyArray = []; + var result = packable.packArray(unpackedArray, emptyArray); + expect(result).toEqual(packedArray); + + var largerArray = new Array(packedArray.length + 1).fill(0.0); + result = packable.packArray(unpackedArray, largerArray); + expect(result).toEqual(packedArray); + }); + it(namePrefix + ' packArray throws with undefined array', function() { expect(function() { packable.packArray(undefined); }).toThrowDeveloperError(); }); + it(namePrefix + ' packArray throws for typed arrays of the wrong size', function() { + expect(function() { + var tooSmall = new Float64Array(0); + packable.packArray(unpackedArray, tooSmall); + }).toThrowDeveloperError(); + + expect(function() { + var tooBig = new Float64Array(10); + packable.packArray(unpackedArray, tooBig); + }).toThrowDeveloperError(); + }); + + it(namePrefix + ' unpackArray works for typed arrays', function() { + var array = packable.unpackArray(new Float64Array(packedArray)); + expect(array).toEqual(unpackedArray); + }); + it(namePrefix + ' unpackArray throws with undefined array', function() { expect(function() { packable.unpackArray(undefined); }).toThrowDeveloperError(); }); + + it(namePrefix + ' unpackArray works with a result parameter', function() { + var array = []; + var result = packable.unpackArray(packedArray, array); + expect(result).toBe(array); + expect(result).toEqual(unpackedArray); + + var PackableClass = packable; + array = new Array(unpackedArray.length); + for (var i = 0; i < unpackedArray.length; i++) { + array[i] = new PackableClass(); + } + + result = packable.unpackArray(packedArray, array); + expect(result).toBe(array); + expect(result).toEqual(unpackedArray); + }); + + it(namePrefix + ' unpackArray throws with array less than the minimum length', function() { + expect(function() { + packable.unpackArray([1.0]); + }).toThrowDeveloperError(); + }); + + it('unpackArray throws with array not multiple of stride', function() { + expect(function() { + packable.unpackArray(new Array(stride + 1).fill(1.0)); + }).toThrowDeveloperError(); + }); } export default createPackableArraySpecs;