-
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
Fix billboard on terrain and Globe.getHeight #4622
Merged
Merged
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
ead79f4
fix_billboardOnTerrain
duvifn 9f7d911
fix Globe.getHeight function
duvifn 156d5bb
compute origin point at the intersection of the surface normal with t…
duvifn 4d12c0e
add to CHANGES.md
duvifn 9f551ab
typos
duvifn 306c276
Merge branch 'master' into fix_billboardOnTerrain
duvifn 74a7de5
cartesian is computed on the ellipsoid surface
duvifn 30e73ff
fix typo
duvifn 7894d20
added Ellipsoid.getSurfaceNormalRayFromZAxis. no tested
duvifn edfd685
fix Ellipsoid.getSurfaceNormalIntersectionWithZAxis
duvifn 1b4f97d
Merge branch 'master' into fix_billboardOnTerrain
duvifn 23d2fea
factor out sqauredAOverSquaredB
duvifn dbd917d
add _sqauredAOverSquaredB to the Ellipsoid constructor
duvifn 0947e12
add check if radii.z===0
duvifn 244cc52
remove unused dependency, change sqauredAOverSquaredB to sqauredXOver…
duvifn 9ae322c
Whitespace
pjcozzi 056fc20
Whitespace
pjcozzi b394335
Style tweak
pjcozzi 1f325e5
Style tweak
pjcozzi 761eda4
Style tweak
pjcozzi d01196b
fix typo in spec title
duvifn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -240,7 +240,7 @@ define([ | |
QuadtreePrimitive.prototype.updateHeight = function(cartographic, callback) { | ||
var primitive = this; | ||
var object = { | ||
position : undefined, | ||
positionOnEllipsoidSurface : undefined, | ||
positionCartographic : cartographic, | ||
level : -1, | ||
callback : callback | ||
|
@@ -455,7 +455,7 @@ define([ | |
customDataRemoved.length = 0; | ||
} | ||
|
||
// Our goal with load ordering is to first load all of the tiles we need to | ||
// Our goal with load ordering is to first load all of the tiles we need to | ||
// render the current scene at full detail. Loading any other tiles is just | ||
// a form of prefetching, and we need not do it at all (other concerns aside). This | ||
// simple and obvious statement gets more complicated when we realize that, because | ||
|
@@ -761,13 +761,46 @@ define([ | |
var data = customData[i]; | ||
|
||
if (tile.level > data.level) { | ||
if (!defined(data.position)) { | ||
data.position = ellipsoid.cartographicToCartesian(data.positionCartographic); | ||
if (!defined(data.positionOnEllipsoidSurface)) { | ||
// cartesian has to be on the ellipsoid surface for `ellipsoid.geodeticSurfaceNormal` | ||
data.positionOnEllipsoidSurface = Cartesian3.fromRadians(data.positionCartographic.longitude, data.positionCartographic.latitude, 0.0, ellipsoid); | ||
} | ||
|
||
if (mode === SceneMode.SCENE3D) { | ||
Cartesian3.clone(Cartesian3.ZERO, scratchRay.origin); | ||
Cartesian3.normalize(data.position, scratchRay.direction); | ||
var surfaceNormal = ellipsoid.geodeticSurfaceNormal(data.positionOnEllipsoidSurface, scratchRay.direction); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this duplicate code belong in a utility function for |
||
|
||
// compute origin point | ||
|
||
// Try to find the intersection point between the surface normal and z-axis. | ||
// Ellipsoid is a surface of revolution, so surface normal intersects the rotation axis (z-axis) | ||
|
||
// compute the magnitude required to bring surface normal to x=0, y=0, from data.position | ||
var magnitude; | ||
|
||
// avoid dividing by zero | ||
if (Math.abs(surfaceNormal.x) > CesiumMath.EPSILON16){ | ||
magnitude = data.positionOnEllipsoidSurface.x / surfaceNormal.x; | ||
} else if (Math.abs(surfaceNormal.y) > CesiumMath.EPSILON16){ | ||
magnitude = data.positionOnEllipsoidSurface.y / surfaceNormal.y; | ||
} else if (Math.abs(surfaceNormal.z) > CesiumMath.EPSILON16){ //surface normal is (0,0,1) | (0,0,-1) | (0,0,0) | ||
magnitude = data.positionOnEllipsoidSurface.z / surfaceNormal.z; | ||
} else { //(0,0,0), just for case | ||
magnitude = 0; | ||
} | ||
|
||
var vectorToMinimumPoint = Cartesian3.multiplyByScalar(surfaceNormal, magnitude, scratchPosition); | ||
Cartesian3.subtract(data.positionOnEllipsoidSurface, vectorToMinimumPoint, scratchRay.origin); | ||
|
||
// Theoretically, the intersection point can be outside the ellipsoid, so we have to check if the result's 'z' is inside the ellipsoid (with some buffer) | ||
if (Math.abs(scratchRay.origin.z) >= ellipsoid.radii.z -11500.0){ | ||
// intersection point is outside the ellipsoid, try other value | ||
magnitude = Math.min(defaultValue(tile.data.minimumHeight, 0.0),-11500.0); | ||
|
||
// multiply by the *positive* value of the magnitude | ||
vectorToMinimumPoint = Cartesian3.multiplyByScalar(surfaceNormal, Math.abs(magnitude) + 1, scratchPosition); | ||
Cartesian3.subtract(data.positionOnEllipsoidSurface, vectorToMinimumPoint, scratchRay.origin); | ||
} | ||
|
||
} else { | ||
Cartographic.clone(data.positionCartographic, scratchCartographic); | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Instead of hard coding
-11500.0
here and below, can this use some percentage of ellipsoid.radii.maximumComponent? Or does the buffer need to be this big? Instead, can it be a much smaller epsilon?