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

Improved point cloud eye dome lighting at varying resolutions #8257

Merged
merged 3 commits into from
Oct 16, 2019

Conversation

IanLilleyT
Copy link
Contributor

For #8113

Point cloud eye dome lighting works by comparing a pixel's depth with neighbor depth values to create a silhouette. Currently the depth texture uses nearest filtering which is a problem when the EDL radius is less than 1, because it will only sample depths within the center pixel. This is a problem with smaller pixel ratios.

I switched it to linear sampling which introduces a small border between point clouds and the background. For example, the gray lines in #8216 become black. This is a pretty uncommon setup, as explained in the issue.

Old:
65531576-37a69400-dec8-11e9-8972-f3fa9818672a
New:
Screen Shot 2019-10-04 at 3 21 40 PM copy
0.25x res
Screen Shot 2019-10-04 at 3 21 46 PM

Old 1.0x res
Screen Shot 2019-10-04 at 3 22 58 PM
New 1.0x res
Screen Shot 2019-10-04 at 3 22 26 PM
Old 0.25x res
Screen Shot 2019-10-04 at 3 23 02 PM
New 0.25x res
Screen Shot 2019-10-04 at 3 22 32 PM

Old 1.0x res
Screen Shot 2019-10-04 at 3 22 44 PM
New 1.0x res
Screen Shot 2019-10-04 at 3 22 00 PM
Old 0.25x res
Screen Shot 2019-10-04 at 3 22 50 PM
New 0.25x res
Screen Shot 2019-10-04 at 3 22 15 PM

@IanLilleyT IanLilleyT requested a review from lilleyse October 4, 2019 22:02
@cesium-concierge
Copy link

Thanks for the pull request @IanLilleyT!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
  • ❔ 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.

@IanLilleyT
Copy link
Contributor Author

IanLilleyT commented Oct 8, 2019

Made a new version of the eye dome lighting shader which uses Nearest sampling and manually interpolates the depth values inside the shader. This way the shader can check for out-of-bounds depth values and still get a clean interpolated depth, best of both worlds.

The main downside is it requires 10 textures reads per pixel vs 6 from before. This might be ok since the extra reads are from adjacent pixels. The image quality is exactly the same as pre-PR but now works at different resolution scales and non-integer EDL radius.

Ready @lilleyse

@lilleyse
Copy link
Contributor

lilleyse commented Oct 15, 2019

The 10 vs. 6 pixels seems to not hurt the fps. I set the view to look just at EDL pixels and cranked resolution scale to 4x (roughly 15360x8640 on my monitor). Both this branch and master rendered at 8fps consistently. Sandcastle

I also like that the new approach doesn't have the black border artifacts that the first approach had.

@lilleyse lilleyse merged commit ec7295c into master Oct 16, 2019
@lilleyse lilleyse deleted the CSS_EyeDomeFix branch October 16, 2019 17:42
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