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

sampleHeight fixes and cleanup #7174

Merged
merged 4 commits into from
Oct 23, 2018
Merged

sampleHeight fixes and cleanup #7174

merged 4 commits into from
Oct 23, 2018

Conversation

lilleyse
Copy link
Contributor

Fixes #7120 and #7129

Otherwise most of this is organizational changes from #7115 that will make that PR easier to review later.

@bagnell can you review?

@cesium-concierge
Copy link

Thanks for the pull request @lilleyse!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@@ -3112,10 +3112,19 @@ define([
this._lastRenderTime = JulianDate.clone(time, this._lastRenderTime);
this._renderRequested = false;
this._logDepthBufferDirty = false;
var frameNumber = CesiumMath.incrementWrap(frameState.frameNumber, 15000000.0, 1.0);
updateFrameNumber(this, frameNumber, time);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separated updateFrameState and updateFrameNumber so that the frame number/time can be updated before update. In #7115 update is where async tiles get loaded so the frame number needs to be correct for the tileset cache to work properly.

@lilleyse lilleyse force-pushed the offscreen-picking-prep branch from ebd65d9 to 4b3bfbf Compare October 19, 2018 21:46
@lilleyse lilleyse mentioned this pull request Oct 19, 2018
5 tasks
if (!defined(result)) {
return new Ray(ray.origin, ray.direction);
}
result.origin = ray.origin;
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and below should clone ray.origin and ray.direction.

@bagnell
Copy link
Contributor

bagnell commented Oct 22, 2018

Those are the only comments. Looks good to me.

@lilleyse
Copy link
Contributor Author

@bagnell Thanks, updated.

@bagnell bagnell merged commit 6757f83 into master Oct 23, 2018
@lilleyse lilleyse deleted the offscreen-picking-prep branch November 8, 2018 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants