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

Fixed clipping of Atmosphere #118

Closed
wants to merge 1 commit into from
Closed

Fixed clipping of Atmosphere #118

wants to merge 1 commit into from

Conversation

emxsys
Copy link
Contributor

@emxsys emxsys commented May 10, 2017

Adjusted the far clip plane to account for the atmosphere and above-ground placemarks beyond horizon.

Before the fix, notice black line on the horizon:
image

After the fix, atmosphere appears correct:
image

Sunrise/sunset effect works now:
image

Placemarks now visible beyond the horizon:
image

…here and above-ground placemarks beyond horizon.

- The constant (1.5e6 meters) is based on the distance to the horizon from the upper region of the atmosphere.
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.

@pdavidc
Copy link
Contributor

pdavidc commented Aug 5, 2017

Closed in favor of #172, which bring's Web WorldWind's far clip distance into parity with WorldWind Android.

@pdavidc pdavidc closed this Aug 5, 2017
@ghost ghost removed the needs review label Aug 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants