Merge pull request #8568 from CesiumGS/fix-cartesian-pack-array

Make CartesianX.packArray() compatible with typed arrays

CHANGES.md

@@ -21,6 +21,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
 

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;

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

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;

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

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

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

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,25 +11,86 @@ 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() {
             var result = packable.unpackArray(packedArray);
             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;