Log Depth View Fix

[bkuster]

Issue

After #8850 we had some erratic picking failures (especially for billboards). I broke it down to these lines:

    } else {
      curNear = Math.max(near, Math.pow(farToNearRatio, m) * near);
-      curFar = farToNearRatio * curNear;
-      if (!logDepth) {
-        curFar = Math.min(far, curFar);
-      }
+      curFar = Math.min(far, farToNearRatio * curNear);
    }

Solution

This MR trys to fix this by clamping the far plane to the next log10 range.

  } else {
      curNear = Math.max(near, Math.pow(farToNearRatio, m) * near);
      curFar = Math.min(far, farToNearRatio * curNear);
+      if (scene.logarithmicDepthBuffer) {
+       curFar = Math.pow(10, Math.ceil(Math.log(curFar) / Math.log(10)));
+      }
  }

Since my grasp on the subject is slight at best, maybe @IanLilleyT you could pitch in here, thanks.

[cesium-concierge]

Thanks for the pull request @bkuster!

• ✔️ 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.

[ebogo1]

Thanks for opening a PR @bkuster! I'm currently working on other parts of the code but this is on our radar 👍

[bkuster]

Thanks for the feedback @ebogo1. You want me to spruce this up with some specs? or are you going to fix it in your PR? and is said PR already open? We can close this in favor of your development, as soon as your ready with it.

[ebogo1]

Ah sorry if I was unclear - for something tricky like log depth we'd want to take a closer look later when we have more bandwidth to tackle changes to the code. Just wanted to let you know that we will return to your PR in the future 👍

[cesium-concierge]

Thanks again for your contribution @bkuster!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[cesium-concierge]

Thanks again for your contribution @bkuster!

No one has commented on this pull request in 90 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 90 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

[lilleyse]

    } else {
      curNear = Math.max(near, Math.pow(farToNearRatio, m) * near);
-      curFar = farToNearRatio * curNear;
-      if (!logDepth) {
-        curFar = Math.min(far, curFar);
-      }
+      curFar = Math.min(far, farToNearRatio * curNear);
    }

This code in #8850 clamps the far plane to the actual far plane as determined by the furthest draw command's bounding volume. Before, log depth would extend to the next multiple of the logarithmicDepthFarToNearRatio.

It would be good to know why this caused picking failures for billboards, and why increasing the far plane helps.

@bkuster could you provide a minimal sandcastle example? If you don't have a sandcastle could you provide the options you use to create billboards? Also just to clarify - are you referring to scene.pick or scene.pickPosition?

[ggetz]

I'm going to close this PR to keep things tidy.

@bkuster, if you want to discuss further, I would suggest opening an issue. If there are further updates here, feel free to reopen. Thanks!