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.

Sign up for GitHub

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

Jump to bottom

Fixed clipping of Atmosphere #118

Closed
wants to merge 1 commit into from
Closed

Fixed clipping of Atmosphere #118

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/navigate/Navigator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,7 @@ define([
// Compute the far clip distance based on the current eye altitude. This must be done after computing the
// modelview matrix and before computing the near clip distance. The far clip distance depends on the
// modelview matrix, and the near clip distance depends on the far clip distance.
this.farDistance = WWMath.horizonDistanceForGlobeRadius(globeRadius, eyePos.altitude);
if (this.farDistance < 1e3)
this.farDistance = 1e3;
this.farDistance = WWMath.horizonDistanceForGlobeRadius(globeRadius, eyePos.altitude) + 1.5e6;
Copy link
Contributor

Choose a reason for hiding this comment

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

@emxsys Instead of the fixed 1.5e6, WWA asks for the horizon distance of the atmosphere height. Would this be better here? See https://github.com/NASAWorldWind/WorldWindAndroid/blob/develop/worldwind/src/main/java/gov/nasa/worldwind/WorldWindow.java#L1041

Copy link
Contributor

Choose a reason for hiding this comment

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

The precomputed horizon distance in the original pull request is correct, but it has two issues: it's specific to Earth, and it's inconsistent with the pattern in Android. It seems to me that we could take this change if it were in line with what's done in Android. I suspect that @emxsys will agree that it'll be easier to keep the two SDKs synchronized if we avoid arbitrary differences.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

If you have time to make the necessary changes, we can approve this PR. Otherwise, I'd be happy to open a new PR from an in-repository branch.


// Compute the near clip distance in order to achieve a desired depth resolution at the far clip distance.
// This computed distance is limited such that it does not intersect the terrain when possible and is never
Expand Down