Added Cartographic.fromCartesian

[lilleyse]

For #3177

Added Cartographic.fromCartesian to 3d-tiles. Once this is merged I can also open this pull request into master.

[mramato]

This seems kind of like a weird function to add, since you have to specify the ellipsoid anyway (and the ellipsoid is the thing actually doing the conversion. Why not just call ellipsoid.cartesianToCartographic directly?

[lilleyse]

That's what was happening before. It might be useful as a utility function possibly. It would have been nice to let it use a default ellipsoid, but if I try using Ellipsoid.WGS84 I run into cyclical dependencies.

[mramato]

Yes, the circular dependency is exactly why we don't usually have from functions for these types of conversions. Cartesian3 gets around this by duplicating code, but that's because it's a pretty good win for usability. I'm not sure that's the case here.

[pjcozzi]

This is the analog of Cartesian.fromDegrees and is a nice usability improvement for picking, which returns a Cartesian, and then displaying the longitude/latitude/height without an explicit ellipsoid.

That reminds me, we should also update this example: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Apps/Sandcastle/gallery/Picking.html#L177

To allow the WGS84 default and avoid the dependency, we could duplicate the code or, perhaps better, put it in a @private helper function.

[denverpierce]

+1 for either doing this, or putting in a dummy function or documentation ("looking for .fromCartesian? click here") that points to ellipsoid. cartographic <-> cartesian is a super common operation, but getting a carto from a cart isn't very obvious.

[mramato]

I'm fine with this if we can cleanly factor out a common function (or duplicate a small amount of code) to enable it to default to WGS84

[pjcozzi]

@mramato agreed, I'm confident this can be done cleanly.

[lilleyse]

I started to look at this more. It looks like there will be a lot of duplicate code because of scaleToGeodeticSurface, which cartesianToCartographic calls. Maybe there's a way to condense it, I'm just not familiar with the math. Factoring into an outside function seems strange just to make this work...

Could there be some extra file that contains the WGS84 which Cartographic and Cartesian include?

[pjcozzi]

Could there be some extra file that contains the WGS84 which Cartographic and Cartesian include?

I'm not completely following this.

Why not introduce one new file with a @private scaleToGeodeticSurface function that both Ellipsoid and Cartographic include? Then just duplicate the fairly trivial implementation of cartesianToCartographic (with a comment) in Carographic. This code rarely changes so this approach should work well.

[lilleyse]

Okay I'll do that, I just wasn't sure if scaleToGeodeticSurface was appropriate as it's own file.

What I meant above was have a really simple file called EllispoidWGS84.js that has a single read-only variable which is the WGS84 ellipsoid. That variable can be initialized from within Ellipsoid. Then Cartographic and Cartesian can include EllispoidWGS84.js, get the WGS84 ellipsoid, and call the functions that they are currently duplicating.

Or you could do the same thing but put the WGS84 var in Cartographic and Cartesian separately, and initialize them in Ellispoid.

[pjcozzi]

Ah, I see now. Interesting, but I think this other approach is more straightforward and avoids potential problems by calling these functions during initialization before that WGS84 variable would be defined.

[lilleyse]

This is ready.

[pjcozzi]

Tweaked the picking example. Looks good.

[pjcozzi]

This is also ready for a pull request into master.

[pjcozzi]

I updated Cities.html to use it.