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 horizon culling issues with large root tiles #8487

Merged
merged 3 commits into from
Dec 20, 2019

Conversation

IanLilleyT
Copy link
Contributor

There were some horizon culling bugs with ArcGisTerrainProvider's root tile which cause some child tiles to be culled or the whole screen to go black altogether. The problem is the tile was creating a horizon culling point from a set of points that were on opposite sides of the globe. This has been a bug for a while but was made worse by #8475. The fix was to detect these backwards points in EllipsoidalOccluder and throw out the entire occlusion point if one of them is backwards. In practice this only happens for very large tiles with widths greater than half the globe. CesiumWorldTerrain never had this problem because it has two root tiles. ArcGisTerrainProvider only has one root tile so it was affected.

Before sandcastle
Screen Shot 2019-12-20 at 1 41 55 PM
After sandcastle (local)
Screen Shot 2019-12-20 at 1 42 03 PM

@IanLilleyT IanLilleyT requested a review from lilleyse December 20, 2019 18:54
@cesium-concierge
Copy link

Thanks for the pull request @IanLilleyT!

  • ✔️ Signed CLA found.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

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

@lilleyse
Copy link
Contributor

Thanks, this fixes the problem we were seeing last night.

I confirmed again that this doesn't change the number of tiles rendered each frame. Sandcastle with all the different terrain providers

@IanLilleyT
Copy link
Contributor Author

@lilleyse I added some new test cases. Ready for review.

@lilleyse
Copy link
Contributor

Thanks @IanLilleyT

@lilleyse lilleyse merged commit bf973bb into master Dec 20, 2019
@lilleyse lilleyse deleted the horizonCullingLargeTilesFix branch December 20, 2019 19:55
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