-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Conversation
Thanks for the pull request @jjhembd!
Reviewers, don't forget to make sure that:
|
@lilleyse can you take a first look? I am still working on a few peripheral things, but the main logic is ready. |
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. |
packages/engine/Source/Shaders/Model/VerticalExaggerationStageVS.glsl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
@lilleyse thanks again for the feedback! I think this is ready for another look. |
Thanks again for the feedback @ggetz! I think I addressed everything you mentioned. |
Sorry, @jjhembd, but one other thing I noticed while testing this out on other tilesets is that OSM Buildings...
...do not show up if the exaggeration is set to anything other than |
But other than OSM Buildings, exaggeration is working great with the other tilesets! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages/engine/Source/Scene/Model/VerticalExaggerationPipelineStage.js
Outdated
Show resolved
Hide resolved
Thanks again for catching this @ggetz. A tile's bounding rectangle was being overwritten. This is fixed now, and OSM buildings exaggerate as expected: |
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. |
@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 Expected result from hosted Sandcastle: |
packages/engine/Specs/Scene/Model/VerticalExaggerationPipelineStageSpec.js
Outdated
Show resolved
Hide resolved
@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. |
The updates look good, thanks @jjhembd! |
@jjhembd The code comments here are incorrect. cesium/packages/engine/Source/Scene/Model/VerticalExaggerationPipelineStage.js Lines 5 to 15 in 0734229
|
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:
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:
globe.terrainExaggeration
andglobe.terrainExaggerationRelativeHeight
. This should really be changed toscene.verticalExaggeration
andscene.verticalExaggerationRelativeHeight
.TileBoundingSphere
andTileOrientedBoundingBox