Ease of use for Non-WGS84 Ellipsoids #12000

ggetz · 2024-05-24T20:30:54Z

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 #4245
Fixes #3543
Fixes #11231

Testing plan

Testing Sandcastle

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

I have submitted a Contributor License Agreement
I have added my name to CONTRIBUTORS.md
I have updated CHANGES.md with a short summary of my changes
I have added or updated unit tests to ensure consistent code coverage
I have update the inline documentation, and included code examples where relevant
I have performed a self-review of my code

github-actions · 2024-05-24T20:31:39Z

Thank you for the pull request, @ggetz!

✅ We can confirm we have a CLA on file for you.

ggetz · 2024-05-24T20:31:47Z

@jjspace Could you please review?

Additionally, @angrycat9000 can you take a look from a usability standpoint?

ggetz · 2024-05-24T20:33:22Z

packages/engine/Source/Core/ArcGISTiledElevationTerrainProvider.js

@@ -446,7 +446,7 @@ ArcGISTiledElevationTerrainProvider.prototype.requestTileGeometry = function (
        encoding: that._encoding,
      });
    })
-    .catch(function (error) {
+    .catch(async function (error) {


Sorry, this is technically an unrelated change, but I was able to track down what I believe was causing #11231 while troubleshooting unit tests.

angrycat9000

Hi Gabby, thanks for working on this. I wasn't able to run the sandcastle.

TypeError: Simon1994PlanetaryPositions_default.computeSunPositionInMoonInertialFrame is not a function
at setSunAndMoonDirections (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:33500:54)
at UniformState.update (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:33578:3)
at render (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:235243:6)
at tryAndCatchError (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:235283:5)
at Scene4.render (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:235338:5)
at CesiumWidget.render (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:238457:17)
at render2 (https://ci-builds.cesium.com/cesium/ellipsoids/Build/CesiumUnminified/index.js:237868:18)

CHANGES.md

packages/engine/Source/Core/ArcGISTiledElevationTerrainProvider.js

packages/engine/Source/Core/CircleGeometry.js

packages/engine/Source/Core/Occluder.js

packages/engine/Source/Core/Cartesian3.js

ggetz · 2024-05-28T13:53:41Z

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.

angrycat9000 · 2024-05-28T15:09:28Z

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 # 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)?

ggetz · 2024-05-28T15:26:20Z

@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.

angrycat9000 · 2024-05-30T13:08:31Z

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?

angrycat9000 · 2024-05-30T14:14:47Z

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

ggetz · 2024-05-30T14:24:40Z

@angrycat9000 Gotcha. In the meantime I'll look into seeing if we have better ways of transitioning between the two.

angrycat9000 · 2024-05-30T15:19:17Z

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.

ggetz · 2024-05-30T15:22:30Z

@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 · 2024-05-30T15:41:33Z

@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.

malaretv · 2024-05-30T16:05:52Z

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.

jjspace

Other than a few small comments, some of which we've already discussed in person, I think this is looking good from a code perspective. from basic testing it seems ok too but i'm not confident I know enough to fully evaluate that but @angrycat9000 seems to have that covered

packages/engine/Source/Core/Cartesian3.js

packages/engine/Source/Core/CircleOutlineGeometry.js

packages/engine/Source/Scene/Globe.js

jjspace · 2024-05-30T18:40:04Z

packages/engine/Source/Scene/GltfLoader.js

@@ -2625,6 +2625,7 @@ function parse(loader, gltf, supportedImageFormats, frameState) {

  if (defined(cesiumRtcExtension)) {
    // CESIUM_RTC is almost always WGS84 coordinates so no axis conversion needed
+    // TODO: Ellipsoids


Double checked, no changes needed here.

packages/engine/Source/Scene/Scene.js

packages/engine/Source/Scene/ScreenSpaceCameraController.js

packages/engine/Source/Widget/CesiumWidget.js

angrycat9000 · 2024-06-03T15:49:06Z

Might also be a nice change to set the globe base color to a grey instead of blue when using the moon ellipsoid.

Co-authored-by: Josh <8007967+jjspace@users.noreply.github.com>

ggetz · 2024-06-10T14:12:14Z

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.

ggetz · 2024-06-10T14:28:51Z

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. 😆

ggetz · 2024-06-10T14:36:24Z

@jjspace This should be ready for another look.

jjspace

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?

angrycat9000 · 2024-06-11T17:31:54Z

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

ggetz · 2024-06-11T20:50:15Z

Thanks all!

ggetz added 6 commits May 14, 2024 16:29

Draft, moon sandcastle

efe698e

Updates for non-WGS84 ellipsoids

65a1951

Clean up API naming, specs

f221dbb

Cleanup geometries, baselayer picker

a541374

Cleanup geometries, baselayer picker

4387c7d

Cleanup

23bfc95

ggetz commented May 24, 2024

View reviewed changes

Cleanup

439ad49

angrycat9000 reviewed May 28, 2024

View reviewed changes

jjspace reviewed May 30, 2024

View reviewed changes

ggetz and others added 3 commits June 10, 2024 09:23

Merge branch 'main' into ellipsoids

eed6702

Update packages/engine/Source/Scene/Globe.js

35d8c63

Co-authored-by: Josh <8007967+jjspace@users.noreply.github.com>

Update inline docs

8f28afe

ggetz added 2 commits June 10, 2024 10:22

SkyBox helper function

189c80b

Remove extraneous comment

1376c9a

ggetz added 2 commits June 10, 2024 10:29

Merge branch 'main' into ellipsoids

8a09dff

Fix icon in baselayerpicker

66d8580

jjspace approved these changes Jun 11, 2024

View reviewed changes

ggetz merged commit cbd13ce into main Jun 11, 2024
9 checks passed

ggetz deleted the ellipsoids branch June 11, 2024 20:50

This was referenced Jun 26, 2024

Fixes #12036 - Types for SceneTransforms functions #12048

Merged

Remove SceneTransforms.wgs84To* functions in 1.121 #12059

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ease of use for Non-WGS84 Ellipsoids #12000

Ease of use for Non-WGS84 Ellipsoids #12000

ggetz commented May 24, 2024 •

edited

Loading

github-actions bot commented May 24, 2024

ggetz commented May 24, 2024

ggetz May 24, 2024

angrycat9000 left a comment

ggetz commented May 28, 2024

angrycat9000 commented May 28, 2024

ggetz commented May 28, 2024

angrycat9000 commented May 30, 2024

angrycat9000 commented May 30, 2024

ggetz commented May 30, 2024

angrycat9000 commented May 30, 2024

ggetz commented May 30, 2024 •

edited

Loading

angrycat9000 commented May 30, 2024

malaretv commented May 30, 2024

jjspace left a comment

jjspace May 30, 2024

ggetz Jun 10, 2024

angrycat9000 commented Jun 3, 2024

ggetz commented Jun 10, 2024

ggetz commented Jun 10, 2024

ggetz commented Jun 10, 2024

jjspace left a comment

angrycat9000 commented Jun 11, 2024

ggetz commented Jun 11, 2024

Ease of use for Non-WGS84 Ellipsoids #12000

Ease of use for Non-WGS84 Ellipsoids #12000

Conversation

ggetz commented May 24, 2024 • edited Loading

Description

Issue number and link

Testing plan

Author checklist

github-actions bot commented May 24, 2024

ggetz commented May 24, 2024

ggetz May 24, 2024

Choose a reason for hiding this comment

angrycat9000 left a comment

Choose a reason for hiding this comment

ggetz commented May 28, 2024

angrycat9000 commented May 28, 2024

ggetz commented May 28, 2024

angrycat9000 commented May 30, 2024

angrycat9000 commented May 30, 2024

ggetz commented May 30, 2024

angrycat9000 commented May 30, 2024

ggetz commented May 30, 2024 • edited Loading

angrycat9000 commented May 30, 2024

malaretv commented May 30, 2024

jjspace left a comment

Choose a reason for hiding this comment

jjspace May 30, 2024

Choose a reason for hiding this comment

ggetz Jun 10, 2024

Choose a reason for hiding this comment

angrycat9000 commented Jun 3, 2024

ggetz commented Jun 10, 2024

ggetz commented Jun 10, 2024

ggetz commented Jun 10, 2024

jjspace left a comment

Choose a reason for hiding this comment

angrycat9000 commented Jun 11, 2024

ggetz commented Jun 11, 2024

ggetz commented May 24, 2024 •

edited

Loading

ggetz commented May 30, 2024 •

edited

Loading