-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds KmlExporter class for exporting an EntityCollection as KML #7921
Conversation
…ls. Fixed polygons. Fixed handling of default intervals
Thanks for the pull request @tfili!
Reviewers, don't forget to make sure that:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is super super cool! This is a GeoJSON pulled into Cesium, that's extruded, and then exported as a KML and viewed in Google Earth pro:
So you can use CesiumJS, the visualization library, as a neat pipeline of sorts of almost.
I do have some suggestions about the usability here:
1) I think modelCallback and textureCallback are confusing
I almost feel like modelCallback should be required if a model is present, since there's no scenario where it will work as exported directly, since you at least need to replace the glTF with Collada.
It's also confusing because textureCallback takes in 3 possible different types of arguments, and it's not clear what the data types are (it's also not clear that it passes in a time parameter, which I only discovered from reading the specs).
I think you'll either need to fix the doc, and make this easier with some code examples, or design it so it's a function that takes in the URL and modifies it, instead of taking in the source object. In this design, they would be called imageUrlCallback
and modelUrlCallback
. The imageUrl callback would essentially run on the output of:
function defaultTextureCallback(texture) {
if (typeof texture === 'string') {
return texture;
}
if (texture instanceof Resource) {
return texture.url;
}
if (texture instanceof HTMLCanvasElement) {
return texture.toDataURL();
}
return '';
}
So all you need to do is modify the correct path to the URL, and same thing for the model.
2) Why can't I specify a global start/stop time for sampling the entities to export?
It looks like defaultAvailability
gets overridden by the entities availability. But this means I can't take the CZML satellites example and export a full rotation of the satellites. I feel like this is a simpler way to think about it and gives the user more control? Here's what I was trying to do:
var timeInterval = new Cesium.TimeInterval({
start : Cesium.JulianDate.fromIso8601('2012-03-15T10:27:12.7559055118181277Z'),
stop : Cesium.JulianDate.fromIso8601("2012-03-16T09:45:15.5905511811142787Z"),
isStartIncluded : true,
isStopIncluded : true,
});
var exporter = new Cesium.KmlExporter(dataSource.entities, {
defaultAvailability: timeInterval
});
var kml = exporter.toString();
download('test.kml', kml);
But I only get just a small slice of the full rotation:
3) Time dynamic lines are transparent instead of disappearing?
Is this intentional? It's more obvious on the green line in the GIF above. I can see all the instances of the line, the ones that are not "in this frame" are slightly transparent. It's kind of cool but perhaps you might not want to see this?
4) Using data URIs for images
I think converting images to dataURIs would be nice in that it would be one more step to make it work out of the box, but then there would be no way to have external image files as is common in KML. I think I'd rather go with the imageUrlCallback
and the user can then decide what they want to do with that URL.
5) Some missing coverage spots
Overall code coverage looks pretty good! Looks like it's missing a test for labelGraphics, multi geometry, and entity.path.
Here's the full coverage for KmlExporter to save you some time:
Specs/DataSources/KmlExporterSpec.js
Outdated
var kml = exporter.toString(); | ||
download('test.kml', kml); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this test here for just debug? Should this be removed? Similarly with the download
function here.
Source/DataSources/KmlExporter.js
Outdated
* @param {Function} [options.modelCallback] A callback that will be called with a ModelGraphics instance and should return the URI to use in the KML. By default it will just return the model's uri property directly. | ||
* @param {JulianDate} [options.time=entities.computeAvailability().start] The time value to use to get properties that are not time varying in KML. | ||
* @param {TimeInterval} [options.defaultAvailability=entities.computeAvailability()] The interval that will be sampled if an entity doesn't have an availability. | ||
* @param {Number} [options.sampleDuration=60] The number of seconds to sample properties that are varying in KML. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have a code example here if we're not creating a Sandcastle example.
You also might want to explain some context here about how time dynamic entities are handled or the caveats about this class (that there isn't a 1:1 mapping so don't expect it to look exactly the same etc?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with this comment. There should be a long-form paragraph before the params with everything @OmarShehata mentioned.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I added an example and a paragraph about how we basically handle everything.
…as a collection of blobs and we now handle inertial positions.
Thanks @OmarShehata. @mramato you want to take a look now that the first pass comments have been addressed. |
Some todos here:
|
I actually haven't finished reviewing this yet. Going to look now. |
Source/DataSources/KmlExporter.js
Outdated
* }); | ||
* | ||
*/ | ||
function KmlExporter(entities, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a class instead of a standalone function? Why not just exportKml(options)
where options.entities
is required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I had bigger plans. I was thinking they could work with the DOM object before exporting. I'll switch it.
Source/DataSources/KmlExporter.js
Outdated
styleCache.save(kmlDocumentElement); | ||
} | ||
|
||
KmlExporter.prototype.export = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming there's a good reason to keep this as a class, this would need doc.
Source/DataSources/KmlExporter.js
Outdated
* | ||
* @param {EntityCollection} entities The EntityCollection to export as KML | ||
* @param {Object} options An object with the following properties: | ||
* @param {Function} [options.ellipsoid=Ellipsoid.WGS84] The ellipsoid for the output file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be Ellipsoid
not Function
.
Source/DataSources/KmlExporter.js
Outdated
* @param {EntityCollection} entities The EntityCollection to export as KML | ||
* @param {Object} options An object with the following properties: | ||
* @param {Function} [options.ellipsoid=Ellipsoid.WGS84] The ellipsoid for the output file | ||
* @param {Function} [options.modelCallback] A callback that will be called with a ModelGraphics instance and should return the URI to use in the KML. This will throw a RuntimeError if there is a model and this is undefined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can actually define the specific callback as a type instead of just Function
so its crystal clear what parameters are expected. Search for TimeInterval~MergeCallback
as an example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. I think that's what @OmarShehata was referring to and not textureCallback. I'll add the callback doc and that should keep things straight.
@OmarShehata Unless I'm mistaken, textureCallback is gone. All files are now returned as both hrefs and blobs. |
Source/DataSources/KmlExporter.js
Outdated
|
||
processMaterial(that, polylineGraphics.material, lineStyle); | ||
|
||
// TODO: <gx:physicalWidth>? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this TODO (write up an issue if it's something we still need to handle)
Source/DataSources/KmlExporter.js
Outdated
/** | ||
* An object with filename and blobs for external files | ||
* @memberof ExternalFileHandler.prototype | ||
* @type {ObjectId} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, does this even need doc? Same as the promise before it, isn't this an internal class?
Please run @OmarShehata this is also something that's good to remember to do when reviewing PRs that include branch new API/classes as well. It's really easy to write JSDoc that doesn't error but doesn't turn out quite right either. |
Functionality itself looks great, we just need to polish it up from the end user's point of view. There's probably no reason for me to look at this again unless something new comes up. @OmarShehata please merge once both our comments are addressed and you're happy. |
@mramato @OmarShehata I think this should be good now. |
Added KMZ support. Just specify function downloadBlob(filename, blob) {
if (window.navigator.msSaveOrOpenBlob) {
window.navigator.msSaveBlob(blob, filename);
} else {
var elem = window.document.createElement('a');
elem.href = window.URL.createObjectURL(blob);
elem.download = filename;
document.body.appendChild(elem);
elem.click();
document.body.removeChild(elem);
}
}
viewer.dataSources.add(Cesium.CzmlDataSource.load('../../SampleData/Vehicle.czml'))
.then(function(dataSource) {
return Cesium.exportKml({
entities: dataSource.entities,
kmz: true
});
})
.then(function(result) {
downloadBlob('vehicle.kmz', result.kmz);
}); |
@tfili this is all working great! The KMZ output is super useful, I imagine most people will want to use it that way. Mostly doc/wording comments below.
I made some tweaks to the top paragraph doc, adding code ticks and doc @links, give it a read and if it's accurate you can just use it:
|
Thanks @OmarShehata. I updated the doc, so this should be ready to go. |
if czml contains model(gltf contians .bin),does it can be exported?I try it,but failed |
It can't be opened in Sandcastle Demo named KML,what is exported by the Sandcastle Demo named Export KML for Model in the dropdown list |
@liyangis KML only supports COLLADA models and Cesium only supports glTF models. For that reason, when exporting a KML document you must specify a Unfortunately, KML loading in Cesium doesn't support model features because there isn't support for COLLADA in Cesium. For this reason you won't be able to export a CZML with models as a KML and reimport it into Cesium. |
Thank you! |
CZML or the Entity API don't have a one to one mapping into KML, but we try our best. This handles the following:
Point
geometrygx:Track
geometryLineString
geometryPolygon
geometryModel
geometryOptions to
exportKml
.entities
:EntityCollection
to export.ellipsoid
: Since KML uses degrees, we can control whatEllipsoid
to use to convert from cartesian to cartographic. Defaults toWGS84
.modelCallback
: Textures and Models have callbacks that are specified so the user can say how to handle the external files. I may actually just return the textures as blobs instead, but I'm not sure.Since KML isn't truly dynamic, so we take some parameters to deal with it
time
: The time to sample properties that aren't time dynamic in KML at all (eg color, etc).defaultAvailability
: For properties that are time dynamic without samples, we will sample across their availability. If an entity has an non-infinite availability we will use it. Otherwise we will use this. Defaults to thecomputeAvailability()
of the entity collection.sampleDuration
: The frequency to the sampling in seconds. Defaults to 60.There are tests for all these. You can test using the CZML Sandcastle examples and just adding the following code
TODO:
GlobeOverlay
s