CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.85k stars 3.47k forks source link

KML parsing missing <kml> tag contents? #4343

Closed dwhipps closed 8 years ago

dwhipps commented 8 years ago

I have a valid KML file that opens fine in both GoogleEarth as well in WorldWind, but fails in Cesium with the error:

"KML - Unsupported feature: kml"

Broken KML - Sandcastle

The file is hosted here: https://earthdata.layeredearth.com/Layers/Earth/Climate%20Change/Global%20Warming/Scenario%20A2%20-%20global%20temperatures.kmz

I've been unable to figure out why Cesium isn't able to parse the contents of the tag.

dwhipps commented 8 years ago

Looks like the problem occurs in KmlDataSource.js:loadXmlFromZip when parser.parseFromString() is called.

DOMParser seems to be failing to properly parse the passed string (which itself looks like valid XML).

dwhipps commented 8 years ago

Note that the file linked in the example above has since been modified and now DOMParser works. The error lay in (valid, I think, but unnecessary) attributes within the tag itself:

<Document id="All Scenario" xsi:schemaLocation="http://www.opengis.net/kml/2.2 http://schemas.opengis.net/kml/2.2.0/ogckml22.xsd http://www.google.com/kml/ext/2.2 http://code.google.com/apis/kml/schema/kml22gx.xsd">

After removing the xsi:schemaLocation="..." attribute, DOMParser is fine with it.

I still think this is a bug in the (browser native) DOMParser, and it might make sense to use a 3rd party class for this.

twpayne commented 8 years ago

xsi:schemaLocation can be used as a hint by some XML parsers to validate the document. In this particular case, the KML schema specifies that (for example) latitudes must be in the range -90..90 but your document uses latitudes -108 to 108 (and also longitudes from -216 to 216). Therefore your document is not valid with respect to the schema you reference.

IMHO the native DOMParser is behaving reasonably in this case. Bringing in a third-party XML parser would add significantly to the download size of Cesium and likely perform worse than the native parser.

hpinkos commented 8 years ago

Yeah,we probably don't want to pull in a third-party XML parser. I agree with @twpayne, I think what Cesium is doing is acceptable. The KML file has out of bounds coordinates to make up for the fact that there's a bit of transparent padding baked into the images

image

Since GE and WW do support this, we may want to consider adding it eventually, but I wouldn't consider it a bug. I'm going to remove the bug label and keep this issue open so we can think about matching what GE does in the future

hpinkos commented 8 years ago

I changed my mind. I created #4463 to cover the issue so I could summarize the problem in the title/initial comment.

Thanks @dwhipps, @twpayne!

mramato commented 8 years ago

I'm not 100% against third-party XML parsers if they are better than the browser and have a small footprint, but I don't know of any that fit that bill. If switching to a third-party parser also cleaned up the code base, that would be useful too. I'd be happy to look at a pull request for that if someone wanted to work on it, but you should propose which library to use first, so that it can be evaluated up front.