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

Fix translucent picking #7039

Merged
merged 5 commits into from
Sep 17, 2018
Merged

Fix translucent picking #7039

merged 5 commits into from
Sep 17, 2018

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Sep 13, 2018

Fixes #7032

This was a regression from #6957 at this commit 259c314. That turned off translucent object sorting during the pick pass. I must have been thinking pick commands wrote depth but they currently don't.

One fix is to revert that commit, the other is to write depth for pick commands. This PR does the second. I like the second way because it makes the pick pass more accurate for overlapping primitives, and allows pick functions to get a pick id and the pick position in the same pass, which I'm trying to do to optimize the pick-from-ray functions. It does break some PrimitiveCollection tests where it expects primitives added at the same spot to be picked based on the order they were added. I'm not sure if this is the test being too strict or actually important behavior to preserve. @bagnell do you know?

Only broken in master so no CHANGES.md update needed.
Edit: does happen in earlier versions of Cesium. Needs CHANGES.md update.

@cesium-concierge
Copy link

Thanks for the pull request @lilleyse!

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

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.

🌍 🌎 🌏

@lilleyse
Copy link
Contributor Author

To add to above, this will probably create a lot of z-fighting while picking overlapping primitives on the surface which will lead to inconsistent pick results. I might have to rethink this approach.

@lilleyse
Copy link
Contributor Author

lilleyse commented Sep 14, 2018

Actually ruled out z-fighting as a problem. However I still need to figure out why picking and rendering produce different results as to the primitive in front, since both passes write depth now.

Debugging demo: master and this branch

@lilleyse
Copy link
Contributor Author

lilleyse commented Sep 15, 2018

Ignoring the case where the primitives completely overlap, which has it's own sort of behavior, this is the problem with turning depth mask on for translucent primitives in the pick pass. Drawn as opaque rectangles, but imagine this is the pick pass:

picking-z-fighting

So the best change is to revert to the old behavior. I'll need to think of ways to work around this in #6934.

@lilleyse
Copy link
Contributor Author

lilleyse commented Sep 15, 2018

But then again master has it's own set of bugs because it doesn't write depth in the pick pass.

The green rectangle is 10000 meters above the red rectangle but the red gets picked if the red primitive's bounding sphere is closer to the camera. That almost seems worse than the z-fighting which can be remedied fairly easily by setting one rectangle's height slightly higher than the other.

pick

Demo 1
Demo 2

@lilleyse
Copy link
Contributor Author

@bagnell Updated CHANGES.md

@bagnell bagnell merged commit 0734aee into master Sep 17, 2018
@bagnell bagnell deleted the translucent-pick-fix branch September 17, 2018 23:16
lilleyse added a commit that referenced this pull request Sep 17, 2018
bagnell added a commit that referenced this pull request Sep 20, 2018
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.

Make Scene.pick work for translucent polygons
3 participants