Be more lenient when processing KML coordinates

Our code in master is pretty strict whenprocessing KML, but it turns out Google will let you pass almost anything and just use zeros (in the case of single coordinates) or create an object without an geometry (in the case of linear rings). This change makes Cesium match that behavior, event though the spec itself says it's invalid. I also noticed the Viewer specs had a bug because Viewer itself was listed in the list of requires (the way our specs work, the thing being tested has to be in the list of requires and has to be first). In this case, because all creation is deferred to the `createViewer` helper, it wasn't being included. To test this just run/review the unit tests, they are updated to reflect the change in functionality.
CesiumGS · Oct 23, 2015 · a772110 · a772110
1 parent 6552144
commit a772110
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 37 deletions.
diff --git a/Source/DataSources/KmlDataSource.js b/Source/DataSources/KmlDataSource.js
@@ -9,6 +9,7 @@ define([
         '../Core/createGuid',
         '../Core/defaultValue',
         '../Core/defined',
+        '../Core/definedNotNull',
         '../Core/defineProperties',
         '../Core/DeveloperError',
         '../Core/Ellipsoid',
@@ -60,6 +61,7 @@ define([
         createGuid,
         defaultValue,
         defined,
+        definedNotNull,
         defineProperties,
         DeveloperError,
         Ellipsoid,
@@ -276,14 +278,14 @@ define([
     }
 
     function readCoordinate(value) {
+        //Google Earth treats empty or missing coordinates as 0.
         if (!defined(value)) {
-            return undefined;
+            return Cartesian3.fromDegrees(0, 0, 0);
         }
 
         var digits = value.match(/[^\s,\n]+/g);
-        if (digits.length !== 2 && digits.length !== 3) {
-            window.console.log('KML - Invalid coordinates: ' + value);
-            return undefined;
+        if (!definedNotNull(digits)) {
+            return Cartesian3.fromDegrees(0, 0, 0);
         }
 
         var longitude = parseFloat(digits[0]);
@@ -303,6 +305,10 @@ define([
         }
 
         var tuples = element.textContent.match(/[^\s\n]+/g);
+        if (!definedNotNull(tuples)) {
+            return undefined;
+        }
+
         var length = tuples.length;
         var result = new Array(length);
         var resultIndex = 0;
@@ -438,12 +444,13 @@ define([
     }
 
     var colorOptions = {};
+
     function parseColorString(value, isRandom) {
         if (!defined(value)) {
             return undefined;
         }
 
-        if(value[0] === '#'){
+        if (value[0] === '#') {
             value = value.substring(1);
         }
 
@@ -843,7 +850,7 @@ define([
         }
 
         if ((defined(altitudeMode) && altitudeMode !== 'clampToGround') || //
-           (defined(gxAltitudeMode) && gxAltitudeMode !== 'clampToSeaFloor')) {
+            (defined(gxAltitudeMode) && gxAltitudeMode !== 'clampToSeaFloor')) {
             window.console.log('KML - Unknown altitudeMode: ' + defaultValue(altitudeMode, gxAltitudeMode));
         }
 
@@ -927,9 +934,6 @@ define([
         var extrude = queryBooleanValue(geometryNode, 'extrude', namespaces.kml);
 
         var position = readCoordinate(coordinatesString);
-        if (!defined(position)) {
-            return;
-        }
 
         entity.position = createPositionPropertyFromAltitudeMode(new ConstantPositionProperty(position), altitudeMode, gxAltitudeMode);
         processPositionGraphics(dataSource, entity, styleEntity);
@@ -1028,12 +1032,9 @@ define([
         var coordinates = [];
         var times = [];
         for (var i = 0; i < length; i++) {
-            //An empty position is OK according to the spec
             var position = readCoordinate(coordNodes[i].textContent);
-            if (defined(position)) {
-                coordinates.push(position);
-                times.push(JulianDate.fromIso8601(timeNodes[i].textContent));
-            }
+            coordinates.push(position);
+            times.push(JulianDate.fromIso8601(timeNodes[i].textContent));
         }
         var property = new SampledPositionProperty();
         property.addSamples(times, coordinates);
@@ -1114,12 +1115,9 @@ define([
             var positions = [];
             times = [];
             for (var x = 0; x < length; x++) {
-                //An empty position is OK according to the spec
                 var position = readCoordinate(coordNodes[x].textContent);
-                if (defined(position)) {
-                    positions.push(position);
-                    times.push(JulianDate.fromIso8601(timeNodes[x].textContent));
-                }
+                positions.push(position);
+                times.push(JulianDate.fromIso8601(timeNodes[x].textContent));
             }
 
             if (interpolate) {
@@ -1191,6 +1189,7 @@ define([
     }
 
     var scratchDiv = document.createElement('div');
+
     function processDescription(node, entity, styleEntity, uriResolver) {
         var i;
         var key;

diff --git a/Specs/DataSources/KmlDataSourceSpec.js b/Specs/DataSources/KmlDataSourceSpec.js
@@ -1704,21 +1704,21 @@ defineSuite([
             expect(label.eyeOffset).toBeUndefined();
             expect(label.pixelOffsetScaleByDistance).toBeUndefined();
 
-        expect(label.text).toBeUndefined();
-        expect(label.fillColor).toBeUndefined();
-        expect(label.outlineColor).toBeUndefined();
-        expect(label.outlineWidth).toBeUndefined();
-        expect(label.show).toBeUndefined();
-        expect(label.scale).toBeUndefined();
-        expect(label.verticalOrigin).toBeUndefined();
-        expect(label.eyeOffset).toBeUndefined();
-        expect(label.pixelOffsetScaleByDistance).toBeUndefined();
+            expect(label.text).toBeUndefined();
+            expect(label.fillColor).toBeUndefined();
+            expect(label.outlineColor).toBeUndefined();
+            expect(label.outlineWidth).toBeUndefined();
+            expect(label.show).toBeUndefined();
+            expect(label.scale).toBeUndefined();
+            expect(label.verticalOrigin).toBeUndefined();
+            expect(label.eyeOffset).toBeUndefined();
+            expect(label.pixelOffsetScaleByDistance).toBeUndefined();
 
-        expect(label.font.getValue()).toEqual('16px sans-serif');
-        expect(label.style.getValue()).toEqual(LabelStyle.FILL_AND_OUTLINE);
-        expect(label.horizontalOrigin.getValue()).toEqual(HorizontalOrigin.LEFT);
-        expect(label.pixelOffset.getValue()).toEqual(new Cartesian2(17, 0));
-        expect(label.translucencyByDistance.getValue()).toEqual(new NearFarScalar(3000000, 1.0, 5000000, 0.0));
+            expect(label.font.getValue()).toEqual('16px sans-serif');
+            expect(label.style.getValue()).toEqual(LabelStyle.FILL_AND_OUTLINE);
+            expect(label.horizontalOrigin.getValue()).toEqual(HorizontalOrigin.LEFT);
+            expect(label.pixelOffset.getValue()).toEqual(new Cartesian2(17, 0));
+            expect(label.translucencyByDistance.getValue()).toEqual(new NearFarScalar(3000000, 1.0, 5000000, 0.0));
         });
     });
 
@@ -1961,7 +1961,7 @@ defineSuite([
         return KmlDataSource.load(parser.parseFromString(kml, "text/xml")).then(function(dataSource) {
             var entities = dataSource.entities.values;
             expect(entities.length).toEqual(1);
-            expect(entities[0].position).toBeUndefined();
+            expect(entities[0].position.getValue(Iso8601.MINIMUM_VALUE)).toEqual(Cartesian3.fromDegrees(0, 0, 0));
             expect(entities[0].polyline).toBeUndefined();
         });
     });
@@ -1970,14 +1970,31 @@ defineSuite([
         var kml = '<?xml version="1.0" encoding="UTF-8"?>\
           <Placemark>\
             <Point>\
+            <altitudeMode>absolute</altitudeMode>\
             <coordinates>1,2,3,4</coordinates>\
             </Point>\
           </Placemark>';
 
         return KmlDataSource.load(parser.parseFromString(kml, "text/xml")).then(function(dataSource) {
             var entities = dataSource.entities.values;
             expect(entities.length).toEqual(1);
-            expect(entities[0].position).toBeUndefined();
+            expect(entities[0].position.getValue(Iso8601.MINIMUM_VALUE)).toEqual(Cartesian3.fromDegrees(1, 2, 3));
+            expect(entities[0].polyline).toBeUndefined();
+        });
+    });
+
+    it('Geometry Point: handles empty coordinates', function() {
+        var kml = '<?xml version="1.0" encoding="UTF-8"?>\
+          <Placemark>\
+            <Point>\
+            <coordinates></coordinates>\
+            </Point>\
+          </Placemark>';
+
+        return KmlDataSource.load(parser.parseFromString(kml, "text/xml")).then(function(dataSource) {
+            var entities = dataSource.entities.values;
+            expect(entities.length).toEqual(1);
+            expect(entities[0].position.getValue(Iso8601.MINIMUM_VALUE)).toEqual(Cartesian3.fromDegrees(0, 0, 0));
             expect(entities[0].polyline).toBeUndefined();
         });
     });
@@ -2133,6 +2150,25 @@ defineSuite([
         });
     });
 
+    it('Geometry Polygon: handles empty coordinates', function() {
+        var kml = '<?xml version="1.0" encoding="UTF-8"?>\
+          <Placemark>\
+            <Polygon>\
+              <outerBoundaryIs>\
+                <LinearRing>\
+                  <coordinates>\
+                 </coordinates>\
+                </LinearRing>\
+              </outerBoundaryIs>\
+            </Polygon>\
+          </Placemark>';
+
+        return KmlDataSource.load(parser.parseFromString(kml, "text/xml")).then(function(dataSource) {
+            var entity = dataSource.entities.values[0];
+            expect(entity.polygon.hierarchy).toBeUndefined();
+        });
+    });
+
     it('Geometry Polygon: without holes', function() {
         var kml = '<?xml version="1.0" encoding="UTF-8"?>\
           <Placemark>\
@@ -2851,4 +2887,4 @@ defineSuite([
             expect(entity.label.text.getValue()).toBe('bob');
         });
     });
-});
+});
diff --git a/Specs/Widgets/Viewer/ViewerSpec.js b/Specs/Widgets/Viewer/ViewerSpec.js
@@ -1,5 +1,6 @@
 /*global defineSuite*/
 defineSuite([
+        'Widgets/Viewer/Viewer',
         'Core/Cartesian3',
         'Core/ClockRange',
         'Core/ClockStep',
@@ -33,6 +34,7 @@ defineSuite([
         'Widgets/SelectionIndicator/SelectionIndicator',
         'Widgets/Timeline/Timeline'
     ], function(
+        Viewer,
         Cartesian3,
         ClockRange,
         ClockStep,
@@ -938,4 +940,4 @@ defineSuite([
         expect(preMixinDataSource.entities.collectionChanged._listeners.length).not.toEqual(preMixinListenerCount);
         expect(postMixinDataSource.entities.collectionChanged._listeners.length).not.toEqual(postMixinListenerCount);
     });
-}, 'WebGL');
+}, 'WebGL');