Skip to content
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

Vertical exaggeration for 3D Tiles #11655

Merged
merged 38 commits into from
Dec 22, 2023
Merged

Vertical exaggeration for 3D Tiles #11655

merged 38 commits into from
Dec 22, 2023

Conversation

jjhembd
Copy link
Contributor

@jjhembd jjhembd commented Nov 29, 2023

This PR extends dynamic terrain exaggeration to 3D Tiles.

There are some differences in the approach for terrain vs. 3D Tiles. For on-the-fly exaggeration, we need 2 things at each vertex:

  1. Ellipsoid normal at the vertex position. This is the local "up" direction along which exaggeration will be performed.
  2. Height above the ellipsoid.

The current terrain exaggeration computes these 2 values on the CPU and uploads them as a separate vertex buffer. This works well for terrain, because it is (a) static and (b) relatively light-weight.

For 3D Tiles, this PR computes normals and heights for each vertex on-the-fly, in the vertex shader, using a local approximation to the Earth curvature. This reduces the load on GPU bandwidth, and simplifies handling of tilesets which are being dynamically positioned via a model matrix.

Here is a local Sandcastle link demonstrating exaggeration of the Google Photorealistic 3D Tiles.

Remaining tasks:

  • Update public API. Exaggeration is still controlled via globe.terrainExaggeration and globe.terrainExaggerationRelativeHeight. This should really be changed to scene.verticalExaggeration and scene.verticalExaggerationRelativeHeight.
  • Support exaggeration of TileBoundingSphere and TileOrientedBoundingBox
  • Update CHANGES.md

@cesium-concierge
Copy link

Thanks for the pull request @jjhembd!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.

@jjhembd
Copy link
Contributor Author

jjhembd commented Nov 29, 2023

@lilleyse can you take a first look? I am still working on a few peripheral things, but the main logic is ready.

@jjhembd
Copy link
Contributor Author

jjhembd commented Nov 29, 2023

I pushed an update to exaggerate bounding boxes and spheres. All bounding volume types are now supported.

Here is an updated local Sandcastle with bounding box QC.

image

packages/engine/Source/Scene/Cesium3DTile.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/Cesium3DTile.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/Cesium3DTile.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/Cesium3DTile.js Outdated Show resolved Hide resolved
packages/engine/Source/Renderer/UniformState.js Outdated Show resolved Hide resolved
packages/engine/Source/Scene/VoxelPrimitive.js Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does entity clamping (#11604) work on exaggerated 3D Tiles?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it does as-is, but it could be extended to support exaggerated tiles. #11604 depends on #11581. In that PR, the pickModel function has access to frameState, so it should be fairly straightforward to account for exaggeration there.
@ggetz am I correct here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjhembd What's the status of this thread? Is this something beyond the scope of this PR? if so, is there an issue for it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I linked to this thread in #11604 so we don't forget about this.

packages/engine/Source/Scene/Scene.js Outdated Show resolved Hide resolved
@jjhembd
Copy link
Contributor Author

jjhembd commented Dec 1, 2023

@lilleyse thanks again for the feedback! I think this is ready for another look.

@jjhembd
Copy link
Contributor Author

jjhembd commented Dec 6, 2023

Thanks again for the feedback @ggetz! I think I addressed everything you mentioned.

@ggetz
Copy link
Contributor

ggetz commented Dec 6, 2023

Sorry, @jjhembd, but one other thing I noticed while testing this out on other tilesets is that OSM Buildings...

const osmBuildingsTileset = await Cesium.createOsmBuildingsAsync();

...do not show up if the exaggeration is set to anything other than 1.0.

@ggetz
Copy link
Contributor

ggetz commented Dec 6, 2023

But other than OSM Buildings, exaggeration is working great with the other tilesets!

Copy link
Contributor

@ptrgags ptrgags left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjhembd I did a partial review pass focused on the updates to Model and automatic uniforms since these are relevant to #4196. Had a few comments there.

packages/engine/Source/Scene/Scene.js Show resolved Hide resolved
packages/engine/Source/Scene/Scene.js Show resolved Hide resolved
packages/engine/Source/Renderer/UniformState.js Outdated Show resolved Hide resolved
@jjhembd
Copy link
Contributor Author

jjhembd commented Dec 22, 2023

Sorry, @jjhembd, but one other thing I noticed while testing this out on other tilesets is that OSM Buildings...

const osmBuildingsTileset = await Cesium.createOsmBuildingsAsync();

...do not show up if the exaggeration is set to anything other than 1.0.

Thanks again for catching this @ggetz. A tile's bounding rectangle was being overwritten. This is fixed now, and OSM buildings exaggerate as expected:
image

@jjhembd
Copy link
Contributor Author

jjhembd commented Dec 22, 2023

Thanks for your feedback @ptrgags! I think I addressed everything you mentioned. Can you give it a final check, and merge if OK?

To verify both legacy and new behavior, you can check the Terrain Exaggeration and 3D Tiles Vertical Exaggeration Sandcastles.

@ptrgags
Copy link
Contributor

ptrgags commented Dec 22, 2023

@jjhembd I just noticed something, if you enable the Scene Mode Picker and switch the mode to CV, the globe doesn't render correctly. modified local Sandcastle with sceneModePicker

image

Expected result from hosted Sandcastle:
image

@ptrgags
Copy link
Contributor

ptrgags commented Dec 22, 2023

@jjhembd okay I've reviewed the sandcastles, the code, and read through the older comment threads (I had a couple questions there about the status of lose ends). I had a few questions.

@ptrgags
Copy link
Contributor

ptrgags commented Dec 22, 2023

The updates look good, thanks @jjhembd!

@syzdev
Copy link
Contributor

syzdev commented May 7, 2024

@jjhembd The code comments here are incorrect.

/**
* The custom shader pipeline stage takes GLSL callbacks from the
* {@link CustomShader} and inserts them into the overall shader code for the
* {@link Model}. The input to the callback is a struct with many
* properties that depend on the attributes of the primitive. This shader code
* is automatically generated by this stage.
*
* @namespace VerticalExaggerationPipelineStage
*
* @private
*/

@jjhembd
Copy link
Contributor Author

jjhembd commented May 7, 2024

@syzdev thanks for catching that! I submitted a fix in #11969.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants