Entities on terrain

[tfili]

Moved to cesium branch from fork PR #3406.

Bounding boxes seem correct but we are still seeing performance issues.

[tfili]

In addition to updating CHANGES, we need to update the Visualizing Geospatial Data tutorial to discuss all of the rules (and caveats) for the behavior; otherwise the forum is going to get flooded with questions because of all of the caveats. @tfili if you want to at least create a bullet list enumerating everything, that should get the ball rolling.

@mramato Forgot this one. Here is a starting list. Don't think I missed anything but update this if you think of anything else.

• Polylines aren't on terrain yet
• Material must be a ColorMaterialProperty
• height and extrudedHeight must be undefined
• For polygons, perPositionHeight must be false
• Outlines aren't drawn (and warning will be shown in the console)
• fill property must be true or a non-constant property

[mramato]

Merge in master, please.

[mramato]

For the default KML example, performance is still over 100% slower than master and it also very hitchy (I'm talking the default view with stats turned on).

Also, after playing with the default KML example for a while, then switching to the GDP one, Chrome just starting griding CPU for a full 30-45 seconds before the app became responsive and switched. (Seems reproducible on my machine).

[mramato]

Since this message is copied in 4 places, I would add a simple property to oneTimeWarning like oneTimeWarning.geometryOutlines with this string and use that instead.

Also, we should replace this message with Entity geometry outlines are unsupported on terrain. Outlines will be disabled. To enable outlines, disable geometry terrain clamping by explicitly setting height to 0. so that users actually know what they need to do to get the outline back if they want it.

[mramato]

I think those are my last comments. I realize this is probably going to be a disruptive change no matter what we do, but the performance issue does concern me.

[pjcozzi]

For the performance concerns, this is on by default, right? Instead, could we have users opt into it even if it is at the Viewer level? It would be easy to deprecate later.

[mramato]

GeoJSON is already opt-in and they can easily go back to the old behavior by setting an explicit height of 0 on any geometry, so I'm not sure adding another layer of checks is worth it. If we ever want to get to terrain on by default, we'll have the bite the bullet and start clamping by default, so I think now would be a good time for that. If we get negative feedback on the release, then we can put in an explicit flag for people that need it.

Unless anyone has any objections, or @pjcozzi feels strong about making the exclusively opt-in all around, I think this is ready. The travis deploy is occasionally failing, but this looks like a problem with deploys in general and not this branch.

[denverpierce]

If I get some free time, I can run this through it's paces and see how it affects my use case. I'm primarily worried about perf, and may have to very carefully tailor things to make it usable, or opt out completely.

What's the reasoning behind making geojson linestrings turn into corridors, and not KML linestrings? I use a fair amount of both, and having KML lines as an outlier is going to be a pain point. I understand that corridor conversion is a workaround at the moment, but it seems strange that it isn't consistent.

[tfili]

The reason it was done this way is because it is completely opt-in for Geojson.

For Kml, whether to clamp to terrain is part of the file itself. Therefore, it would break apps that load a kml document and then try to use Entity.polyline. Like you said, this is a workaround, so we would most likely break their app again at some point.

@pjcozzi Any opinion here?

[pjcozzi]

Do we have a table of before/after performance numbers?

Given that we expect to make terrain clamping 10x faster with 3D Tiles and that the current GroundPrimitive is basically opt-in, I still think this should be opt-in. Perhaps we can simplify the overall way in which opt-in happens if that will satisfy everyone's concerns.

[tfili]

@mramato @denverpierce I updated KmlDataSource and GeoJsonDataSource to both be opt-in for geometry (polygons and lines) and to use corridors for lines if they do opt-in. Both will follow their spec in regards to points, billboards, etc (For KML listen to altitudeMode and for GeoJson, clamp to ground if no height is specified). This should remove any performance concerns around using GroundPrimitives if the user doesn't care about terrain.

[pjcozzi]

@tfili can you please post a few choice screenshots of KML/GeoJSON on terrain here so we can use them in the 1.23 blog post, twitter, etc.?

[mramato]

As mentioned offline, update changes again and this is good to go.

[tfili]

@mramato CHANGES has been updated.

[tfili]

[pjcozzi]

@tfili we're looking for choice screenshots of KML/GeoJSON on terrain, not just of entities. Can you please create some?