Merge pull request #2526 from AnalyticalGraphicsInc/circle-outline-ge…

…ometry

Fix EllipseOutlineGeometry and CircleOutlineGeometry packing

CHANGES.md

@@ -22,6 +22,7 @@ Change Log
 * Fixed incorrect ellipse texture coordinates. [#2363](https://github.com/AnalyticalGraphicsInc/cesium/issues/2363) and [#2465](https://github.com/AnalyticalGraphicsInc/cesium/issues/2465)
 * Fixed a bug that would cause incorrect geometry for long Corridors and Polyline Volumes. [#2513](https://github.com/AnalyticalGraphicsInc/cesium/issues/2513)
 * Fixed a bug in imagery loading that could cause some or all of the globe to be missing when using an imagery layer that does not cover the entire globe.
+* Fixed a bug that caused `ElipseOutlineGeometry` and `CircleOutlineGeometry` to be extruded to the ground when they should have instead been drawn at height. [#2499](https://github.com/AnalyticalGraphicsInc/cesium/issues/2499).
 * Fixed some styling issues with `InfoBox` and `BaseLayerPicker` caused by using Bootstrap with Cesium. [#2487](https://github.com/AnalyticalGraphicsInc/cesium/issues/2479)
 * Added support for rendering a water effect on Quantized-Mesh terrain tiles.
 * Added `pack` and `unpack` functions to `Matrix2` and `Matrix3`.

Source/Core/EllipseOutlineGeometry.js

@@ -201,7 +201,7 @@ define([
         this._rotation = defaultValue(options.rotation, 0.0);
         this._height = height;
         this._granularity = granularity;
-        this._extrudedHeight = defaultValue(extrudedHeight, 0.0);
+        this._extrudedHeight = extrudedHeight;
         this._extrude = extrude;
         this._numberOfVerticalLines = Math.max(defaultValue(options.numberOfVerticalLines, 16), 0);
         this._workerName = 'createEllipseOutlineGeometry';
@@ -211,7 +211,7 @@ define([
      * The number of elements used to pack the object into an array.
      * @type {Number}
      */
-    EllipseOutlineGeometry.packedLength = Cartesian3.packedLength + Ellipsoid.packedLength + 8;
+    EllipseOutlineGeometry.packedLength = Cartesian3.packedLength + Ellipsoid.packedLength + 9;
 
     /**
      * Stores the provided instance into the provided array.
@@ -244,7 +244,8 @@ define([
         array[startingIndex++] = value._rotation;
         array[startingIndex++] = value._height;
         array[startingIndex++] = value._granularity;
-        array[startingIndex++] = value._extrudedHeight;
+        array[startingIndex++] = defined(value._extrudedHeight) ? 1.0 : 0.0;
+        array[startingIndex++] = defaultValue(value._extrudedHeight, 0.0);
         array[startingIndex++] = value._extrude ? 1.0 : 0.0;
         array[startingIndex]   = value._numberOfVerticalLines;
     };
@@ -290,13 +291,14 @@ define([
         var rotation = array[startingIndex++];
         var height = array[startingIndex++];
         var granularity = array[startingIndex++];
+        var hasExtrudedHeight = array[startingIndex++];
         var extrudedHeight = array[startingIndex++];
         var extrude = array[startingIndex++] === 1.0;
         var numberOfVerticalLines = array[startingIndex];
 
         if (!defined(result)) {
             scratchOptions.height = height;
-            scratchOptions.extrudedHeight = extrudedHeight;
+            scratchOptions.extrudedHeight = hasExtrudedHeight ? extrudedHeight : undefined;
             scratchOptions.granularity = granularity;
             scratchOptions.rotation = rotation;
             scratchOptions.semiMajorAxis = semiMajorAxis;
@@ -312,7 +314,7 @@ define([
         result._rotation = rotation;
         result._height = height;
         result._granularity = granularity;
-        result._extrudedHeight = extrudedHeight;
+        result._extrudedHeight = hasExtrudedHeight ? extrudedHeight : undefined;
         result._extrude = extrude;
         result._numberOfVerticalLines = numberOfVerticalLines;
 

Specs/Core/CircleOutlineGeometrySpec.js

@@ -87,15 +87,29 @@ defineSuite([
         expect(m.indices.length).toEqual(2 * 6 * 2);
     });
 
-    var center = Cartesian3.fromDegrees(0,0);
-    var ellipsoid = Ellipsoid.WGS84;
+    var center = new Cartesian3(8, 9, 10);
+    var ellipsoid = new Ellipsoid(11, 12, 13);
     var packableInstance = new CircleOutlineGeometry({
         ellipsoid : ellipsoid,
         center : center,
-        granularity : 0.1,
-        radius : 1.0,
-        numberOfVerticalLines : 0
+        granularity : 1,
+        radius : 2,
+        numberOfVerticalLines : 4,
+        height : 5,
+        extrudedHeight : 7
     });
-    var packedInstance = [center.x, center.y, center.z, ellipsoid.radii.x, ellipsoid.radii.y, ellipsoid.radii.z, 1.0, 1.0, 0.0, 0.0, 0.1, 0.0, 0.0, 0.0];
-    createPackableSpecs(CircleOutlineGeometry, packableInstance, packedInstance);
+    var packedInstance = [center.x, center.y, center.z, ellipsoid.radii.x, ellipsoid.radii.y, ellipsoid.radii.z, 2, 2, 0, 5, 1, 1, 7, 1, 4];
+    createPackableSpecs(CircleOutlineGeometry, packableInstance, packedInstance, 'extruded');
+
+    //Because extrudedHeight is optional and has to be taken into account when packing, we have a second test without it.
+    packableInstance = new CircleOutlineGeometry({
+        ellipsoid : ellipsoid,
+        center : center,
+        granularity : 1,
+        radius : 2,
+        numberOfVerticalLines : 4,
+        height : 5
+    });
+    packedInstance = [center.x, center.y, center.z, ellipsoid.radii.x, ellipsoid.radii.y, ellipsoid.radii.z, 2, 2, 0, 5, 1, 0, 0, 0, 4];
+    createPackableSpecs(CircleOutlineGeometry, packableInstance, packedInstance, 'at height');
 });

Specs/Core/EllipseOutlineGeometrySpec.js

@@ -113,16 +113,33 @@ defineSuite([
         expect(m.indices.length).toEqual(2 * 6 * 2);
     });
 
-    var center = Cartesian3.fromDegrees(0,0);
-    var ellipsoid = Ellipsoid.WGS84;
+    var center = new Cartesian3(8, 9, 10);
+    var ellipsoid = new Ellipsoid(11, 12, 13);
     var packableInstance = new EllipseOutlineGeometry({
         ellipsoid : ellipsoid,
         center : center,
-        granularity : 0.1,
-        semiMajorAxis : 1.0,
-        semiMinorAxis : 1.0,
-        numberOfVerticalLines : 0
+        granularity : 1,
+        semiMinorAxis : 2,
+        semiMajorAxis : 3,
+        numberOfVerticalLines : 4,
+        height : 5,
+        rotation : 6,
+        extrudedHeight : 7
     });
-    var packedInstance = [center.x, center.y, center.z, ellipsoid.radii.x, ellipsoid.radii.y, ellipsoid.radii.z, 1.0, 1.0, 0.0, 0.0, 0.1, 0.0, 0.0, 0.0];
-    createPackableSpecs(EllipseOutlineGeometry, packableInstance, packedInstance);
+    var packedInstance = [center.x, center.y, center.z, ellipsoid.radii.x, ellipsoid.radii.y, ellipsoid.radii.z, 3, 2, 6, 5, 1, 1, 7, 1, 4];
+    createPackableSpecs(EllipseOutlineGeometry, packableInstance, packedInstance, 'extruded');
+
+    //Because extrudedHeight is optional and has to be taken into account when packing, we have a second test without it.
+    packableInstance = new EllipseOutlineGeometry({
+        ellipsoid : ellipsoid,
+        center : center,
+        granularity : 1,
+        semiMinorAxis : 2,
+        semiMajorAxis : 3,
+        numberOfVerticalLines : 4,
+        height : 5,
+        rotation : 6
+    });
+    packedInstance = [center.x, center.y, center.z, ellipsoid.radii.x, ellipsoid.radii.y, ellipsoid.radii.z, 3, 2, 6, 5, 1, 0, 0, 0, 4];
+    createPackableSpecs(EllipseOutlineGeometry, packableInstance, packedInstance, 'at height');
 });

Specs/createPackableSpecs.js

@@ -1,62 +1,65 @@
 /*global define*/
 define([
+        'Core/defaultValue',
         'Core/defined'
     ], function(
+        defaultValue,
         defined) {
     "use strict";
     /*global jasmine,describe,xdescribe,it,xit,expect,beforeEach,afterEach,beforeAll,afterAll,spyOn,runs,waits,waitsFor*/
 
-    function createPackableSpecs(packable, instance, packedInstance) {
+    function createPackableSpecs(packable, instance, packedInstance, namePrefix) {
         instance = JSON.parse(JSON.stringify(instance));
         packedInstance = JSON.parse(JSON.stringify(packedInstance));
+        namePrefix = defaultValue(namePrefix, '');
 
-        it('can pack', function() {
+        it(namePrefix + ' can pack', function() {
             var packedArray = [];
             packable.pack(instance, packedArray);
             var packedLength = defined(packable.packedLength) ? packable.packedLength : instance.packedLength;
             expect(packedArray.length).toEqual(packedLength);
             expect(packedArray).toEqual(packedInstance);
         });
 
-        it('can roundtrip', function() {
+        it(namePrefix + ' can roundtrip', function() {
             var packedArray = [];
             packable.pack(instance, packedArray);
             var result = packable.unpack(packedArray);
             expect(instance).toEqual(result);
         });
 
-        it('can unpack', function() {
+        it(namePrefix + ' can unpack', function() {
             var result = packable.unpack(packedInstance);
             expect(result).toEqual(instance);
         });
 
-        it('can pack with startingIndex', function() {
+        it(namePrefix + ' can pack with startingIndex', function() {
             var packedArray = [0];
             var expected = packedArray.concat(packedInstance);
             packable.pack(instance, packedArray, 1);
             expect(packedArray).toEqual(expected);
         });
 
-        it('can unpack with startingIndex', function() {
+        it(namePrefix + ' can unpack with startingIndex', function() {
             var packedArray = [0].concat(packedInstance);
             var result = packable.unpack(packedArray, 1);
             expect(instance).toEqual(result);
         });
 
-        it('pack throws with undefined value', function() {
+        it(namePrefix + ' pack throws with undefined value', function() {
             var array = [];
             expect(function() {
                 packable.pack(undefined, array);
             }).toThrowDeveloperError();
         });
 
-        it('pack throws with undefined array', function() {
+        it(namePrefix + ' pack throws with undefined array', function() {
             expect(function() {
                 packable.pack(instance, undefined);
             }).toThrowDeveloperError();
         });
 
-        it('unpack throws with undefined array', function() {
+        it(namePrefix + ' unpack throws with undefined array', function() {
             expect(function() {
                 packable.unpack(undefined);
             }).toThrowDeveloperError();