Fix multifrustum seam artifacts #8205
Conversation
|
Thanks for the pull request @lilleyse!
Reviewers, don't forget to make sure that:
|
This reverts commit 9d46847.
|
I checked most of the Sandcastle examples with log-depth and depth-test-against-terrain set to false and the only one that had an issue was the VR example. I pushed a fix for that. @IanLilleyT ready to review |
|
You might notice this seam in the Sandcastle demo. I opened a separate issue for that #8216.
|
|
@IanLilleyT this is ready to review |
There was a problem hiding this comment.
Looks good @lilleyse . The approach to separate globe and primitives into their own framebuffers can be extended for future features like underground rendering, so I like it. I'm wondering if the naming should change from GlobeDepth.js to something else.
Also, usePrimitiveFramebuffer is slightly confusing inside Scene. Maybe separatePrimitiveFramebuffer to convey that globe and primitive rendering can be separate?
Source/Scene/GlobeDepth.js
Outdated
| @@ -17,13 +18,15 @@ define([ | |||
| '../Shaders/PostProcessStages/DepthViewPacked', | |||
There was a problem hiding this comment.
DepthViewPacked unused in this file
| '../Shaders/PostProcessStages/DepthViewPacked', |
Source/Scene/GlobeDepth.js
Outdated
| @@ -39,6 +42,7 @@ define([ | |||
| DepthViewPacked, | |||
There was a problem hiding this comment.
DepthViewPacked unused in this file
| DepthViewPacked, |
|
@IanLilleyT updated |
|
Looks good, thanks @lilleyse |
Fixes #4381
I decided not to go with the stencil approach mentioned in that issue because it would have gotten very complicated with derived commands.
The new idea is to keep two framebuffers, one for the globe and one for everything else. After all frustums have been rendered the "everything-else" framebuffer is overlayed above the globe framebuffer.
The new framebuffer is only created and used if
depthTestAgainstTerrain = falseand the number of frustums is greater than 1.#8202 needs to be merged first. I also want to do more testing.