-
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
Dynamic terrain exaggeration #9603
Conversation
Thanks for the pull request @IanLilleyT!
Reviewers, don't forget to make sure that:
|
@@ -805,23 +812,19 @@ function interpolateMeshHeight( | |||
northInteger = height - 1 - northInteger; | |||
|
|||
var southwestHeight = | |||
(encoding.decodeHeight(buffer, southInteger * width + westInteger) / | |||
exaggeration - | |||
(encoding.decodeHeight(buffer, southInteger * width + westInteger) - |
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.
Exaggeration used to be baked into the TerrainMesh
geometry and had to be undone here. Now there's no need for that.
|
||
var isGeographic = defaultValue(options.isGeographic, true); | ||
var ellipsoid = defaultValue(options.ellipsoid, Ellipsoid.WGS84); | ||
|
||
var oneOverGlobeSemimajorAxis = 1.0 / ellipsoid.maximumRadius; | ||
|
||
var nativeRectangle = options.nativeRectangle; | ||
var nativeRectangle = Rectangle.clone(options.nativeRectangle); |
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.
The worker strips the functions from these inputs, so clone to turn them into real Rectangle
again.
OrientedBoundingBox.clone(result.orientedBoundingBox), | ||
that._orientedBoundingBox | ||
); | ||
var boundingSphere = that._boundingSphere; |
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.
The create mesh worker doesn't return bounding sphere and obb any more. Now, GlobeSurfaceTileProvider
regenerates bounding volumes when exaggeration changes.
@@ -23,25 +24,32 @@ var SHIFT_LEFT_12 = Math.pow(2.0, 12.0); | |||
* @alias TerrainEncoding | |||
* @constructor | |||
* | |||
* @param {Cartesian3} center The center point of the vertices. |
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.
Before it used the AxisAlignedBoundingBox.center
as the RTC center, which is not necessarily the best possible value, and was also unclear.
this._calculateStrideAndOffsets(); | ||
var newStride = this.stride; | ||
|
||
for (var index = 0; index < vertexCount; index++) { |
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.
This code assumes geodeticSurfaceNormal
is the last vertex attribute. This could probably use a refactor when we add another vertex attribute after
@@ -45,7 +44,6 @@ function TerrainMesh( | |||
vertexStride, | |||
orientedBoundingBox, | |||
encoding, |
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.
TerrainMesh
doesn't need exaggeration any more. We send the un-exaggerated positions to the GPU and scale dynamically. In JS we would call TerrainEncoding.getExaggeratedPosition
* @type {Number} | ||
* @default 0.0 | ||
*/ | ||
this.terrainExaggerationRelativeHeight = 0.0; |
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.
FrameState
still needs exaggeration values because GlobeSurfaceTileProvider
doesn't have access to the Globe
, but does have access to FrameState
. This is used when checking if the exaggeration has changed between frames so that geodetic surface normals can be added or removed.
@@ -672,7 +692,7 @@ Globe.prototype.pickWorldCoordinates = function ( | |||
); | |||
} else if (defined(surfaceTile.renderedMesh)) { | |||
BoundingSphere.clone( | |||
surfaceTile.renderedMesh.boundingSphere3D, | |||
surfaceTile.tileBoundingRegion.boundingSphere, |
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.
The mesh's boundingSphere3D
is not exaggerated, so it can't be used. The TileBoundingRegion.boundingSphere
has the updated bounding volumes.
@@ -437,6 +511,7 @@ function processTerrainStateMachine( | |||
frameState, | |||
terrainProvider, | |||
imageryLayerCollection, | |||
vertexArraysToDestroy, |
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.
A missing param that was never caught....
@@ -1514,38 +1518,6 @@ function updateHeights(primitive, frameState) { | |||
data.callback(position); | |||
data.level = tile.level; | |||
} | |||
} else if (tile.level === data.level) { |
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.
This used to remove the height clamp callback from the object when the terrain tile beneath it was refined as far as possible, possibly as an optimization since the height was never going to get more accurate or change. This code has been deleted because terrain exaggeration can happen at any time, so the callback needs to stick around.
} | ||
} else { | ||
// this tile may come into view at a later time so keep the loop active | ||
continueProcessing = true; |
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.
What if the tile is never seen again? Does the loop happen every frame forever until the tile is unloaded?
In the sandcastle if I click "Remove Exaggeration" I see "here" get printed continuously.
quadtree.forEachLoadedTile(function (tile) {
console.log("here");
...
}
Should each tile have a dirty frame so that it knows to add or remove its geodetic surface normals?
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.
After offline discussion forEachRenderedTile
seems to work.
Source/Scene/Scene.js
Outdated
@@ -1432,15 +1438,32 @@ Object.defineProperties(Scene.prototype, { | |||
}, | |||
|
|||
/** | |||
* Gets the scalar used to exaggerate the terrain. | |||
* Gets or sets the scalar used to exaggerate the terrain. | |||
* @memberof Scene.prototype | |||
* @type {Number} | |||
* @readonly |
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.
No longer @readonly
(though it is going away so it doesn't really matter)
Fog culling is really severe with strong exaggeration. Out of scope for this PR. CC #8454 |
… unrendered straggler tiles
… going to be removed soon
@lilleyse This is ready for another review |
The updates look good. eslint and tests pass locally. |
dynamic_terrain_exaggeration.mov
Fixes #4342
This PR adds dynamic exaggeration to terrain, an enhancement over the fixed terrain exaggeration that CesiumJS has had since 2015. This property is controlled by
Globe.terrainExaggeration
which can be changed as often as you want. There's also a new property calledGlobe.terrainExaggerationRelativeHeight
which is the height relative to which terrain is exaggerated, meaning terrain above that height will stretch upwards, and terrain below that height will stretch downwards.Globe.terrainExaggeration
is1.0
by default andGlobe.terrainExaggerationRelativeHeight
is0.0
by default.The old ways of setting
terrainExaggeration
have been deprecated and will be removed in 1.85: #9626It works by storing a f32vec3 geodetic surface normal per-vertex and adding it to the relative-to-center position in the shader, which is about as good as we can get accuracy-wise without 64-bit emulation or some other slow path.
The geodetic surface normals are only stored when exaggeration is not
1.0
. If you go from1.0
to another number, they will be computed only the fly. If you go from not1.0
to1.0
, they will be removed. One downside to this PR is more memory is needed for dynamic terrain exaggeration compared to fixed terrain exaggeration. This is because fixed terrain exaggeration baked the exaggeration into the geometry, so it didn't need to store geodetic surface normals. To get a sense of how much extra memory is used, see the screenshots below, especially theTerrainMesh
category. Here is comes out to ~30% extra memory.1.0
exaggeration1.01
exaggerationAnd back to
1.0
exaggeration. This is the same amount ofTerrainMesh
memory as the first image.Also, this PR fixes #7580 - problem with exaggerated normals getting washed out. It looks better now:
Normals are exaggerated in the shader by scaling the "rejection" between the original vertex normal and the geodetic surface normal. So if the slope is originally 45 degrees and gets exaggerated by 100X, the exaggerated normal is going to be mostly perpendicular to the geodetic surface normal. It seems to work a lot better than the old way and is fairly cheap to calculate and is useful for lighting at the very least.
Finally, I whipped up a test to see how much floating point error is introduced by moving the exaggeration code to the shader instead of baking into the geometry. Baking has 64-bit precision and shader has 32-bit precision. The most error I recorded was under
10cm
. The sample code below came out with an error of0.063323
meters, with the inputs:RTC is the most important factor when determining error. When the RTC is forced to be between
-10,000
and+10,000
, the maximum error is0.001172m
See full results
I've tested with:
HeightReference.CLAMP_TO_GROUND
Globe.pick
ScreenSpaceCameraController
It's always possible I've missed a feature to test. Let me know if so.
This PR doesn't fix problems related to ground primitive shadow volumes not clamping to the ground properly: #8480. So if you exaggerate terrain there could still be problems there.