Ease of use for Non-WGS84 Ellipsoids
Description
This PR is designed to make the API for setting an alternative ellipsoid throughout the API more streamlined. There are some additional changes needed for 3D Tiles, but that is coming in a follow-up PR.
The main addition is Ellipsoid.default, which is now used throughout the API as the default ellipsoid rather than Ellipsoid.WGS84 directly. That means instead of having to do Cartographic.fromCartesian(0, 1, 3, myEllipsoid) everywhere, you can now set Ellipsoid.default = myEllipsoid once.
There were a few places which assumed WGS84 even before instantiation, so setting Ellipsoid.default modifies a few other classes. This is counterintuitive, but done to avoid circular dependencies (ie. Ellipsoid needs to depend on Cartesian3, so Cartesian3 can't depend on Ellipsoid).
Furthermore, we make a few Earth-centric assumptions throughout the API.We now disable them when the ellipsoid is set to something other than Ellipsoid.WGS84 or use a default proportionate to the selected ellipsoid. This includes:
- atmosphere (ground and sky)
- fog
- lighting and night imagery; fade in and fade out distances
- default camera position
- the star skybox
- the moon
In particular, I had to make some configuration changes to the base layer picker to 1) Ensure the WGS84 ellipsoid was always used when relevant, and 2) not override when you set the viewer's ellipsoid to something other than the WGS84 ellipsoid
Last, but not least, there were some instances of naming things with WGS84 when we just mean any arbitrary ellipsoid. These have been changed, and where needed functions have been renamed and the old versions deprecated.
Issue number and link
Fixes https://github.com/CesiumGS/cesium/issues/4245 Fixes https://github.com/CesiumGS/cesium/issues/3543 Fixes https://github.com/CesiumGS/cesium/issues/11231
Testing plan
Additionally to test this, I iterated through many Sandcastle examples, setting Cesium.Ellipsoid.default = Cesium.Ellipsoid.MOON; or another arbitrary ellipsoid as the first line.
Cesium.Ellipsoid.default = Cesium.Ellipsoid.UNIT_SPHERE; is also a possibility, and while nothing should crash, there is some weirdness purely due to the size disparity.
Author checklist
- [x] I have submitted a Contributor License Agreement
- [x] I have added my name to
CONTRIBUTORS.md - [x] I have updated
CHANGES.mdwith a short summary of my changes - [x] I have added or updated unit tests to ensure consistent code coverage
- [x] I have update the inline documentation, and included code examples where relevant
- [x] I have performed a self-review of my code
Thank you for the pull request, @ggetz!
:white_check_mark: We can confirm we have a CLA on file for you.
@jjspace Could you please review?
Additionally, @angrycat9000 can you take a look from a usability standpoint?
Hi Gabby, thanks for working on this. I wasn't able to run the sandcastle.
@angrycat9000 Apologies, I forgot to push up my last commit. Everything should be good to go now.
Thanks for the update @ggetz. I ran into some problems testing the use case for ion where we re-use the same preview window for different assets.
I modified your sandcastle to change back and forth between the earth and moon on left click. However this didn't seem to work as expected (the earth was sized the same as the moon after a click).
Just changing the default ellipsoid after the viewer was created did not work. I tried setting the ellipsoid properties individually however I got an error viewer.scene.ellipsoid = viewer.ellipsoid = Cesium.Ellipsoid.default = Cesium.Ellipsoid.WGS84;
Uncaught TypeError: Cannot set property ellipsoid of #<Viewer> which has only a getter (on line 104)
What would be the expected process to go back to the earth from the moon (and vise versa)?
@angrycat9000
Since you instantiated the earth options (terrain provider and imagery provider) while the default ellipsoid was set to the Moon, they were created with the Moon's ellipsoid. You can either manually pass in the WGS84 ellipsoid (which is actually a bit tricky when using IonImageryProvider since it relies on ion metadata to set all the configuration options), or wait to create the earth's terrain and imagery providers as in this tweaked example.
Hi @ggetz Thanks for that tweaked example. I'm running into some issue with the camera when switching back and forth.
See this example. Click to change to moon. Then try to zoom in. Notice it does not let you zoom in very far. Are there other things I would need to reset to change between moon and earth?
At this point I'm running into enough issues with stuff that was created before changing the default ellipsoid, I'm going to try just throwing out the whole viewer and recreating it if I need to change between Moon/Earth
@angrycat9000 Gotcha. In the meantime I'll look into seeing if we have better ways of transitioning between the two.
Is there a reason why the star skybox was removed? ion is using a grey background so this is more noticeable than in the sandcastle demo with the black background. Would prefer to have this left in place for the moon.
@angrycat9000 This was turned off by default since the star positions are not accurate when using the moon as the central body. It can be re-enabled.
@angrycat9000 This was turned off by default since the star positions are not accurate when using the moon as the central body. It can be re-enabled.
That makes sense. I can just update the background to black for the moon for now.
Not that it has to be in this PR, but curious about what would be the level of effort to get an accurate skybox suitable for use with lunar data? Is there any issue to track that?
It might also make sense to provide a method like Skybox.getEarthSkybox() to get the default skybox box that the viewer uses. That would enable consumers to turn the skybox on for the moon if they don't care about the accuracy of the star positions.
First off this is awesome! Very excited to have this land 😄
@angrycat9000 Gotcha. In the meantime I'll look into seeing if we have better ways of transitioning between the two.
Wayyyy out-of-scope for this issue, but just want to flag that transitioning between different planets will also need some way to dynamically load in different approximateTerrainHeights, once terrain and ground primitives are involved. We work with quite a few planetary bodies and I have yet found a way to seamlessly support viewing them all within one packaged application.
Might also be a nice change to set the globe base color to a grey instead of blue when using the moon ellipsoid.
Not that it has to be in this PR, but curious about what would be the level of effort to get an accurate skybox suitable for use with lunar data? Is there any issue to track that?
Not yet. I'll open an issue to track follow up changes. I'm not actually sure what all went into generating the default starmap.
Might also be a nice change to set the globe base color to a grey instead of blue when using the moon ellipsoid.
For now, I think it's best to leave this up to the app developer. We'll keep the blue default in the API, as I don't think realism was the driving factor for the color choice. 😆
@jjspace This should be ready for another look.
This looks good to me now, most of my comments were minor code and docs anyway. @angrycat9000 is there anything else you want to check or add from your testing?
This good from my end
Thanks all!