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

Race condition in billboard clamping #10191

Open
mramato opened this issue Mar 10, 2022 · 4 comments
Open

Race condition in billboard clamping #10191

mramato opened this issue Mar 10, 2022 · 4 comments

Comments

@mramato
Copy link
Member

mramato commented Mar 10, 2022

I found this while testing out native promises, but confirmed it happens in master as well.

  1. Open Billboard Sandcastle: https://sandcastle.cesium.com/?src=Billboards.html
  2. Select "Add marker billboards" It will look like this: Screenshot_20220310_184309
  3. Focus should be on the combo box, so press arrow down to switch to "Disable depth test when clamped to ground.
  4. As soon as the billboard is visualized and while the terrain is still going, press arrow up to go back to "Add marker billboards" it will now look like this:
    Screenshot_20220310_184417

The billboard will remain in the wrong location for other examples as well. Something thinks this billboard is supposed to be clamped to ground when it's not.

P.S. You can use the mouse to switch between the demos as well, but the arrow key makes it easy

@yurichen17
Copy link
Contributor

Hi there, I would like to try and fix this bug! Could I be assigned to this?

@mramato
Copy link
Member Author

mramato commented Mar 27, 2022

Hi, @yurichen17, thanks for offering to help! We don't use of the "Assignee" field in GitHub. For bugs like this one there is no need to ask for permission and you can work on the issue and open a pull request for review when ready. If you have any questions, you can comment on the issue or on the PR itself if it's far enough along to open.

If you work on something larger in the future, like a new feature or something that requires a lot of code changes, then it's customary to comment on the issue or start a thread on our forum beforehand. While we love contributions, larger features or code changes usually need some coordination among the maintainers.

There's also a lot of standard documentation in https://github.com/CesiumGS/cesium/tree/main/Documentation/Contributors

Thanks, again!

@yurichen17
Copy link
Contributor

Hi @mramato , thank you so much for your quick response and the resources! I'm new to contributing so I appreciate the help in getting started. I'll be sure to go over the documentation and open a pull request for this issue. Thank you again!

@yurichen17
Copy link
Contributor

Just wanted to update on some progress (and maybe for some potential guidance as well)! I've been trying to find the origin of this bug and was able to recreate it on my Chrome browser. After some digging and playing around, it seems as though when the browser is thinner vertically (like making the browser only take up the top half of the computer screen) the billboards actually appear as they should, and are not clamped to the ground even after going into the "Disable depth test when clamped to ground" mode.

With the condensed browser, while it looks correct at first, once you zoom in on the map, the billboard goes back to being "clamped" and appears in the wrong location. This makes me think there must be some kind of update related to the zooming-in part of the camera view.

If anyone has any suggestions on where to look, please let me know! In the meantime, I'll keep looking through the code to work out what is happening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants